MWRender refactor

Everything about development and the OpenMW source code.
Locked
User avatar
lgromanowski
Site Admin
Posts: 1193
Joined: 05 Aug 2011, 22:21
Location: Wroclaw, Poland
Contact:

MWRender refactor

Post by lgromanowski » 13 Aug 2011, 21:37

gus wrote: I think MWRender should be refactored. Latly, I tried to find a memory leak, but it's very difficult to know who create/delete what in the MWRender namespace.
I think this is due to the coupling between the World, Scene, and CellRenderer Classes. The World class should deal with only one class IMO (so we should choose between the CellRenderer class or the Scene class to interact with the World class).

And the CelleRenderer and the Scene classes are too coupled.

There should be some clear rules about which class is responsible for deleting objects. For now, the Scene class delete and create the physic part, and the CellRenderer class create and delete the rendering part. I think (as we decided to completely couple physic and rendering) that the same class should do both, making it easier to track memory leaks and bugs.

And it should be the same for moving objects. It should be either the CellRenderer class which move objects, either the Scene class, but not both.

Do you agree?
Zini wrote: See, what I added to the tracker a while back:

http://bugs.openmw.org/issues/144

;)
Zini wrote: To elaborate a bit more on it: The whole CellRenderer stuff was Nico's design. I wouldn't have added such a class at all.
But when I had to add some of the functionality currently found in CellRenderer, I was pretty new to the project. Going to an (at that time) unknown project leader and telling him "a substantial part of your design is crap, remove it" didn't seem to be a wise move.
So I kept the CellRenderer class (that really didn't make any sense to me) and tried to work around it, which resulted in the much too complex design and all the inconsistencies we have now.
jhooks1 wrote: I agree with this change, it was annoying to have to place identical code for npc rendering in the exterior and interior classes.
Zini wrote: Right. The two renderer classes are something, that I can't blame Nico for. That was my idea.

Originally, I wasn't sure, if exteriors use the same coordinate system as interiors. Unfortunately they do, which is a major reason for the precision issues projects like TR have.
I have kept both classes, because post 1.0 we may decide to offer an ESM/ESP format modification, that uses cell-coordinates for exteriors instead of world-coordinates. But I guess we can hack that back in later. I am totally okay with removing two different interfaces for interiors and exteriors.
gus wrote:
See, what I added to the tracker a while back:

http://bugs.openmw.org/issues/144
I should check the tracker more often 8-)
To elaborate a bit more on it: The whole CellRenderer stuff was Nico's design. I wouldn't have added such a class at all.
That's right, completly removing the CellRenderer class seems fine. The only thing i'm a little worried about is that if we do that, the scene class might get quiet big. But i guess there is no helping it.

Edit: maybe it's the whole scene class which should be different for interior/exterior uses?
Zini wrote:
maybe it's the whole scene class which should be different for interior/exterior uses?
Don't think so. Exteriors and interiors are almost identical (at least from a rendering perspective). The only important differences are the terrain and the possible future coordinate system changes.
best regards,
Lukasz

Locked