PDA

View Full Version : use inherited abstract procedure with typecasting?



noeska
19-05-2007, 01:28 PM
System: Windows XP
Compiler/IDE: Turbo Delphi for Win32
API: OpenGL

As the gl3ds became too large i decided to split it up. But i am running into some problems with that.

The idea is that i have a generic model class named TModel and for Loading i have an inherited class T3dsModel and an TMsaModel and even more file formats are possible this way. For rendering i have a TglModel. A TdxModel could come along too.
In TModel Render and LoadFromFile are virual and abstract.

useage is like this:

mesh1:=TModel.Create(nil);
TMsaModel(mesh1).LoadFromFile('models\hog2.txt');

and rendering like:
TglModel(mesh1).Render;

Things are working just fine, althoug i get a lot of warnings from turbo delphi like:
[Pascal Warning] Model3ds.pas(40): W1010 Method 'LoadFromFile' hides virtual method of base type 'TModel'
and
[Pascal Warning] OpenGL15_MainForm.pas(197): W1020 Constructing instance of 'TModel' containing abstract method 'TModel.LoadFromFile'
and more like those 2 warnings.

How do i fix these? I do like the concept of having a TModel class and use that with typecasting for loading and rendering. Thanks for your ideas in advance.

You can download the complete source here: http://www.noeska.com/downloads/FrameWork3d.zip

Setharian
19-05-2007, 01:41 PM
the warnings are the outcome of what you did....
mesh1 := TModel.Create(nil);
is not correct and it should be
mesh1 := TMsaModel.Create(nil);

where mesh1 is of type TModel....by constructing TModel and typecasting it to TMsaModel and then using it could result in some very nasty AVs (in case TMsaModel has some new fields and you invoke a method which uses them)
now because mesh1 actually contains a reference to TMsaModel, not to just TModel which hasn't got LoadFromFile implemented, you can safely use
mesh1.LoadFromFile('mymodel.txt');

the first warning makes me assume that TMsaModel.LoadFromFile is not tagged as an override, instead it's declared as a normal method and "hides" the inherited virtual abstract method from TModel....

Edit:
looked at the source - I'll give you an advice, use try..finally blocks when constructing local objects like string lists, lists, streams, etc. or when an exception happens, you leak memory...

noeska
19-05-2007, 02:32 PM
Setharian:
-you are right that a TMsaModel.Create shoudl be the right way, but (1) .
Also this still gives:
[Pascal Warning] OpenGL15_MainForm.pas(197): W1020 Constructing instance of 'T3dsModel' containing abstract method 'TModel.Render'
so even if i do not use render i need to implement it?
-I did not mark the procedures override or reimplement because the application does not work anymore then.
-The try finaly are on the TODO list.

(1):
But in the current implementation i can do

T3dsModel(mesh1).LoadFromFile('models\tulip.3ds');
TMsaModel(mesh1).SaveToFile('tulip.txt');

What i am trying to accomplish is something like an interface, but without the com part (in the end everything should also be compileable with fpc).
I would like to keep that possible. Thinking about it again i might have to use a TBaseModel and inherit TModel from that and also inherig T3dsModel from that. I don't know if that works.

noeska
19-05-2007, 03:49 PM
I split up some more:

TBaseModel
-TModel
-TRenderModel
--TglModel
-TLoadSaveModel
--TMsaModel
--T3dsModel

The abstract funtions are in TRenderModel and TLoadSaveModel
In TglModel/TMsaModel/T3dsModel i use reintroduce;

Now i can still use:
mesh1:=TModel.Create(nil);
TMsaModel(mesh1).LoadFromFile('models\hog2.txt');

and rendering like:
TglModel(mesh1).Render;

Without a lot of warnings....

Complet Source is for downloat at: http://www.noeska.com/downloads/FrameWork3d-2.zip

Now i need to find out if i can prevent direct usage of TBaseModel but keep shared code there.

Setharian
19-05-2007, 04:08 PM
I think you are not going the right way...I'd say this...

TModel - base
methods .Load/SaveFromStream/File(ModelFormat: TModelFormat; ...);
TModelFormat = (mf3ds, mfMsa, ...);

the specialized render model classes are then fine....I'd say that you want to achieve that you can load the model from any resource and continue working with it in the same way so no need to declare special classes for loading/saving if the run-time data representing the model is the same (that is T3dsModel or TMsaModel has no extra data)....

noeska
19-05-2007, 05:57 PM
How is TModelFormat = (mf3ds, mfMsa, ...); related to the TModel? The way i have it now i can access its proctected variables easily. Also i do not want to reintrocude file format specific code to the TModel again, or in any other class for that matter. Explain further ...

Setharian
19-05-2007, 06:37 PM
why do you want a seperate class for every model format type with only some methods overriden and no new fields or methods added....makes no sense, like I said I presume you want to work with TModel (or TGlModel, etc.) without caring from which file format it has been loaded....in that case it's perfectly fine to include all loading methods inside TModel itself and not make seperate classes which has just the loading methods overriden....

for example you have
type
TMsaModel = class(TLoadSaveModel)
public
procedure LoadFromFile(AFileName: string); reintroduce;
procedure LoadFromStream(stream: Tstream); reintroduce;
procedure SaveToFile(AFileName: string); reintroduce;
procedure SaveToStream(stream: TStream); reintroduce;
end;

if TMsaModel contains no new methods and no new fields, it's a redundant class....you could add that functionality directly to TModel and have it everywhere...that way you wouldn't need typecasts to invoke the overriden methods (which is a very bad practive and rest assured can't be put into production code without your team leader having firing you after a conversation with the boss)....the same is with T3dsModel class - it also only overrides 2 methods and adds internal helper method, but that's where it ends....

with
TModel.LoadFromFile(ModelFormat: TModelFormat; const FileName: string);
TModel.SaveToFile(ModelFormat: TModelFormat; const FileName: string);
you could do just
Model.LoadFromFile(mf3ds, 'mymodel.3ds');
Model.SaveToFile(mfMsa, 'mymodelout.txt');
to load model in 3DS format and saving it in Msa instead of
T3dsModel(Model).LoadFromFile('mymodel.3ds');
TMsaModel(Model).SaveToFile('mymodelout.txt');

the method with typecasts is just wrong (why you reintroduce those methods in the implementation is another question, should be overriden instead)...

so the main question is if you need T3dsModel/TMsaModel/etc. to have Data which is not in TModel.....if no then scrap modelformat-specific classes....if yes, then you must do seperate classes per model-format and instead make the rendering architecture different something like...
TModel.Render(Renderer: TRenderer); virtual; abstract;
which is not possibly the best thing due to the fact that most of the rendering code would be duplicated by the different model types...

suffice to say, you must write how you imagine it working and what fearures it should have, then I can help you more....

noeska
19-05-2007, 08:48 PM
I gues that i am using classes to do things that are normaly done with interfaces is making things clouded. And me implementing it 'badly' as i cannot to multiple inheritance from multiple classes, the additional ones have to be an real interface.

It is al intended to make things more maintainable e.g. seperating msa loading from 3ds loading and seperating loading from rendering. That gives me seperate projects that are easier to maintain. Having multiple file formats in one class makes it cluttered especially after more fileformats are added. Adding obj funtionality with the new classes would be just making an inherited version TLoadSaveModel.

The typecasts are not nice, but are they as bad as you say? I do like to hear some more opinions on this matter.

Tomorow i try to lose the reintroduce and replace them with override, see what is does.

The overiding of TModel from TBaseModel without addtional functions is also a thing i am not proud of. I just needed it as i want to use a generic way of representing a model without it being fileformat specific. And i feel a TBaseClass should not be used directly.

technomage
20-05-2007, 07:27 AM
I've been thinking about waht you want to do and how about this.

Yuor TModel is really just a class which stores the data needed to render a model on the screen. So you, so you have a TModel which contains data such as vertex arrays, texture info etc. You could then derive you TGLModel or a TDXModel to handle rendering (athough I would say that you just need to implement that in a TRenderer and have on TRenderer for each API).

To load model formats make use of a TModelReader class, which takes a Stream or filename as a constructor parameter so you get


var Model: TModel;
var Reader: TModelReader;
begin

Model := TModel.Create;
Reader := T3DSModelReader.Create('Filename.3ds');
try
Model.LoadFromReader(Reader);
finally
Reader.Free;
end;



You can then also have a TModelWriter as well to save the model data to a file



var Writer: TModelWriter;
begin

Writer := TOBJModelWriter.Create('Filename.obj');
try
Model.SaveToWriter(Writer);
finally
Writer.Free;
end;



The TModelWriter and TModelReaders would have overloaded constructors to take filenames, or a Stream. You then have one public Load / Save method that you override in the descendent classes.


type

TModelReader = class
private
FData: TStream;
protected
property Data: TStream read FData;
public
// implement the create from file to call , createFromStream
constructor CreateFromFile(const AFilename: string);
// createfromStream simply sets the FData field to the incomming stream
constructor CreateFromStream(const AStream: TStream);

// override this method in descendent classes
function LoadModel: TModel; virtual; abstract;

end;



So you end up with the TModel that has no loading or saving functionality, and the TModelReader and Writers that are specialists in loading or saving particular model formats.

You can also see how to can merge the Reader and Writers into one class that can handle both loading and savng


type

TModelReadWriter = class
private
FData: TStream;
protected
property Data: TStream read FData;
public
// implement the create from file to call , createFromStream
constructor CreateFromFile(const AFilename: string);
// createfromStream simply sets the FData field to the incomming stream
constructor CreateFromStream(const AStream: TStream);

// override this method in descendent classes
function LoadModel: TModel; virtual; abstract;
procedure SaveModel(const AModel: TModel); virtual; abstract;

end;



In this case youd need to ensure that in the CreateFromFile when you created a TFileStream to pass it to CreateFromStream that you open the file for both read and write operations.

Just a few thoughts for you anyway. There are 101 ways to handle that problem you have.

savage
20-05-2007, 12:08 PM
Now i can still use:
mesh1:=TModel.Create(nil);
TMsaModel(mesh1).LoadFromFile('models\hog2.txt');


as Setharian said, this is strictly not the correct use of OO. The above code would work better as ...
var
mesh1:=TModel;
begin
mesh1:=TMsaModel.Create(nil);
mesh1.LoadFromFile('models\hog2.txt');
end;

if you plan to use it that way.

I disagree with Setharian on the suggestion of using enumerated types though. I think a better way and more OO way of achieving that, would be through the use of class factories. If you would like me to explain that idea further or provide a small example, let me know.

noeska
20-05-2007, 12:16 PM
@savage: please do explain and an example would also be nice.


Looking into class factories i do see benefits for loading meshes, but not yet for saving a mesh into a different format as it is loaded in. Hmm maybe via cloning or something like that?

savage
20-05-2007, 02:14 PM
type
TModelLoaderClass = class of TGameInterface;

function TModelLoadingManager.LoadModel( aModelLoader : TModelLoaderClass; string : aFileName ) : TModelLoader;
begin
if aModelLoader <> nil do
begin
result := aModelLoader .Create( aFileName );
end;
end;


which can then be called as


var
ModelToLoad : TModelLoader;
begin
ModelToLoad := MyModelLoadingManager.LoadModel( T3DSModel, 'c:\somewhere.3ds' );
ModelToLoad.DoStuffWithModel;
.
.
.
ModelToLoad := MyModelLoadingManager.LoadModel( TDirectXModel, 'c:\somewhere.x' );
ModelToLoad.DoOtherStuffWithModel;
end;


I'm using this sort of factory technique in the SoAoS port to decide what interface to load and display next. It looks like this...


type
TGameInterfaceClass = class of TGameInterface;
var
CurrentGameInterface : TGameInterfaceClass = TGameIntro;
GameWindow : TGameInterface;

begin
while CurrentGameInterface <> nil do
begin
GameWindow := CurrentGameInterface.Create( Application );
GameWindow.LoadSurfaces;

Application.Show;
CurrentGameInterface := GameWindow.NextGameInterface;

if ( GameWindow <> nil ) then
FreeAndNil( GameWindow );
end;
end.


This way I can add X new screens/interfaces without the need to update any enumeration types. I just have to make sure that when I want to display an interface that I populate the NextGameInterface variable with a game interface class type and it just works.

I'm not saying this doesn't have it's drawbacks, but I think is less of a maintenance headache than updating an enumerated list everytime I think of adding something new. Also no case statements required either. Though they can be very usefull at times.

To do something that would allow you to load one model and then save it as some other format, you will need to have one common way of holding the data in your base class or similar, then each derived class would use this stored data to convert it to what format it is familiar with. Look through the VCL and look at how TGraphic/TPicture work to see how this can be implemented.

I hope this helps.

noeska
20-05-2007, 02:53 PM
Do class factories also work with free pascal?

So in the baseclass i need to implement the assign funtion and i am there.
Then i can create a new object from the class i want to use for saving and use something like
var ModelToSave: TMsaModel;
ModelToSave := TMsaModel.Create(nil);
ModelToSave.Assign(ModelToLoad);
ModelToSave.SaveToFile('test.txt');
ModelToSave.Free;
Or i could place the above code in TModelLoadingManager as an safetoformat function.

So i have to make sure i clean up the copies as otherwise i could end up with a lot copies of the same object.

Next is Rendering. Assigning the model to an TGlModel for each render is not working nice. What could work is doing it once after loading and free the original meshes.
OR
Use the suggested seperate render class. The render class needs to read out details for the models. That way i could also optimize rendering by sending out all geometry at once and other things like that....

Thankt to you all...

cairnswm
21-05-2007, 05:42 AM
The way I get around these problems is to not use the Abstract keyword


TModel = Class
Procedure Render; Virtual;
Procedure LoadFromFile(FileName : String); Virtual;
End;

Procedure TModel.Render;
Begin
// Empty Method, override to do something useful
End;

Procedure TModel.LoadFromFile(FileName : String);
Begin
// Empty Method, override to do something usefull
End;


Then the child classes do what they need


TmsaModel = Class(TModel)
Procedure Render; Override;
Procedure LoadFromFile(FileName : String); Override;
End;

Procedure TmsaModel.Render;
Begin
Inherited;
..
Do Something
..
End;

Procedure TmsaModel.LoadFromFile(FileName : String);
Begin
Inherited;
..
Do Something
..
End;


Then when creating a msa model, but making it compatible with all other models I use the base class and let polymorphism take care of what gets called.


Var
MyModel : TModel;
Begin
// Create it as the child class type I need to use
MyModel := TmsaModel.Create;
MyModel.LoadFromFile('Model.msatype');

//To render
MyModel.Render;
End;


Because the methods are virtual in the base class but the created type is TmsaModel the TMsaModel methods will be called and not the base methods.

Of course this means that at the base level you need to create stubs for all the methods you will be using later. but remember you can also do this


If MyModel is TmsaModel then
TmsaModel(MyModel).msaMethod;


To call specific methods for the various types.