MWRender Refactoring

Everything about development and the OpenMW source code.
jhooks1
Posts: 780
Joined: 06 Aug 2011, 21:34

Re: MWRender Refactoring

Post by jhooks1 » 17 Nov 2011, 23:01

Yes the old code created it just like it does now. I passed in the mwRoot and it works now.

jhooks1
Posts: 780
Joined: 06 Aug 2011, 21:34

Re: MWRender Refactoring

Post by jhooks1 » 17 Nov 2011, 23:12

So far:
Interior cells work from startup
Exterior cells do not work
Changing cells does not work
Physics does not work

jhooks1
Posts: 780
Joined: 06 Aug 2011, 21:34

Re: MWRender Refactoring

Post by jhooks1 » 18 Nov 2011, 00:08

Exterior cells are working. Cell changing still not working.

EDIT: Physics are working now

User avatar
Rhys
Posts: 113
Joined: 06 Aug 2011, 01:51
Location: Australia

Re: MWRender Refactoring

Post by Rhys » 18 Nov 2011, 08:52

Wow, go Jason! :D

User avatar
Zini
Posts: 5537
Joined: 06 Aug 2011, 15:16

Re: MWRender Refactoring

Post by Zini » 18 Nov 2011, 09:55

The use of the cellstore variable in Scene::changeCell looks odd. Totally redundant, in fact. You don't need it when removing cells and when trying to find the current cell, you can use the iterator directly.

However that does not explain a crash (I assume you are still getting crashes).

There are also a lot of functions in RenderingManager that don't have an implementation yet (or only a dummy implementation). What definitely should not be working yet is removing unloaded cells from the scene. But I don't see right now how that could result in a crash.

btw. you were right about the mwRoot. Turning the OGRE root node would have resulted in problems with other rendering subsystems (sky, terrain).

jhooks1
Posts: 780
Joined: 06 Aug 2011, 21:34

Re: MWRender Refactoring

Post by jhooks1 » 19 Nov 2011, 05:02

I can change cells now, there was something wrong with the way foreach works with handles (called in unloadCells()). Now the next problem: if you change cells, when you toggle collision you get a crash.

jhooks1
Posts: 780
Joined: 06 Aug 2011, 21:34

Re: MWRender Refactoring

Post by jhooks1 » 19 Nov 2011, 06:55

Sweet! Fixed the physics bug mentioned. Also fixed a bug where when you switch cells the previous cell would be shown along with the new cell. Both these bugs were hard to track down, one was a case of a missing & in mwclass.

EDIT: Code committed. There is one more bug to fix, if you press the spacebar and you do not have an object in front of you openmw will crash. Not sure where this problem would lie.

User avatar
Zini
Posts: 5537
Joined: 06 Aug 2011, 15:16

Re: MWRender Refactoring

Post by Zini » 19 Nov 2011, 09:03

one was a case of a missing & in mwclass.
Usually, if you have a class that can't be copied or should not be copied, you declare copy constructor and assignment operator privately and don't provide an implementation. This will make the linker complain, if you accidentally try to copy an object of this class. Unfortunately this practice has been neglected in the OpenMW source a lot.

The call to configureAmbient in loadCell looks wrong. Ambient is scene-wide, so changing it for the last cell loaded will not provide correct results in exteriors. Instead you should call this function on a cell change.

The rest looks good though. If you can get rid of this last final bug and also fill in a couple of missing RenderingManager functions, we should be done with the refactoring.

User avatar
Zini
Posts: 5537
Joined: 06 Aug 2011, 15:16

Re: MWRender Refactoring

Post by Zini » 19 Nov 2011, 09:14

The call to configureAmbient in loadCell looks wrong. Ambient is scene-wide, so changing it for the last cell loaded will not provide correct results in exteriors. Instead you should call this function on a cell change.
Oops. I guess I wasn't fully awake. Exteriors don't have ambient settings. You can keep the call, but limit it to interiors (check the Interior flag in ESM::Cell).

jhooks1
Posts: 780
Joined: 06 Aug 2011, 21:34

Re: MWRender Refactoring

Post by jhooks1 » 19 Nov 2011, 09:23

Any ideas on how to fix the last bug? I thought we had this problem before once.

Post Reply