MWRender Refactoring

[gus]

I also made an interesting observation. After toggling the physics debug rendering on, the player's physics shape moves away from the camera when you are trying to move. That explains a lot of what we are seeing.

I remember that we had exactly this problem once before. Looks like you resurrected an old bug ( bug necromancy -> bad ). Don't remember any details thought. There might be something about it in the old threads about physics. gus would be the right person to ask about it. He handled the problem last time. But I am not sure if he is currently around.

I've just come back. I will have a look at it. But from what you describe, the problem is simply that the camera position is not correctly modified when the physic shape of the player moves. Did you changed the camera scene node (or the root scene node)?

[Zini]

No idea what was changed. That kinda is the problem, because nothing should have been changed at all. So far the whole thing was just an exercise in moving stuff around. But apparently something went wrong.

btw. you aren't registered on the new tracker yet.

[gus]

How the Scene::doPhysic function was replaced?
Anyway, in the old code, this function and to be precise this:

Code: Select all

    for(std::map<std::string,OEngine::Physic::PhysicActor*>::iterator it = eng->PhysicActorMap.begin(); it != eng->PhysicActorMap.end();it++)
    {
        OEngine::Physic::PhysicActor* act = it->second;
        btVector3 newPos = act->getPosition();
        MWWorld::Ptr ptr = world.getPtrViaHandle (it->first);
        world.moveObject (ptr, newPos.x(), newPos.y(), newPos.z());
    }

was executed each frame, but I can't find it in the new code. If you have forgotten to add it in the new code, just add it, and it *should* solve the bug.

Edit: my bad, i found it in the new code.
But is moving impossible? Or is it just that it's too slow?

Edit2: looks like I found the bug. It seems that the function World::doPhysics doesn't update sceneNodes position/orientations. ( should moveObjectImp update the position of the sceneNode?)

Edit3: while i'm at it, shouldn't most of the physic code be moved to the World::Scene class as the only objects that are "in" the physic engine are those that are in the scene class (as far as i understand).

[Zini]

I made some changes today, but apparently they did not fix anything.

Edit: my bad, i found it in the new code.
But is moving impossible? Or is it just that it's too slow?

Interior works. Exterior doesn't work at all and hangs sometimes.

Edit2: looks like I found the bug. It seems that the function World::doPhysics doesn't update sceneNodes position/orientations. ( should moveObjectImp update the position of the sceneNode?)

There was an update, just at the wrong spot. But that should not change much.

Scene node manipulation should happen in the new mwrender subsystem. But it has not been written yet, so we do it in MWWorld::World for now.

Edit3: while i'm at it, shouldn't most of the physic code be moved to the World::Scene class as the only objects that are "in" the physic engine are those that are in the scene class (as far as i understand).

I think it is better to have the physics in a separate class, since physics is only one element of the scene (the other being the rendering).

[Zini]

Narrowed it down a bit. During the physics loop OpenMW fails to find a matching object for an OGRE handle. This throws an exception and interrupts the update loop midway, which in turn leads to moving the physics representation of the player without moving the OGRE representation (currently the camera).

But the big question is now why OpenMW can't find an object for the handle.

[Zini]

Found it. A container was returned by value instead of by reference, which in turn broke iterations when there was more than one object in the container.

Give me another day to do a bit more cleanup and then the mwrender work can continue. Looks like I will finally be able to start my task for 0.12.0 too (next week).

[raevol]

Did pointers screw us up?

[Zini]

Nope. A reference. A function returned a copy of a container instead of a reference. That in turn broke an iteration over said container (begin and end belonging to different copies of the container).

[swick]

nice!

[Zini]

Okay. Done.

A few more tips:

- I suggest you do the remaining refactoring in separate steps: eliminating mwscene, objects/actors, sky, debug rendering (not necessarily in this order). It is obvious that during such a step OpenMW won't render properly at times. That is okay. You can even push commits in this state, if you want. But at the end of each step, everything should work again.

- For the actors I suggest only a basic implementation. Adding/removing a scenenode should be enough for now. jhooks can fill in the rest. If I understand it correctly most of the old animation implementation has to go anyway. Don't waste time on it.

- For getting rid of mwscene you will have to work with OpenEngine. This means git submodules for you, which is more than a bit fiddly. Make sure you know how to use submodules before starting this part of the work.

- If you transplant functionality from class A to class B, make sure to remove it from class A. This will serve you as an implicit To-Do list and also it will make the compiler catch pieces of code where you are still mistakenly use A instead of B.

- For this task precision is more important than speed. You did the first part of the refactoring impressively quick. But you produced some bugs that took us weeks to fix, which entirely killed the speed advantage. These were all pretty trivial mistakes, some hardly more than typing errors, and as such should be avoidable (or at least reducible in numbers), even if that means you are progressing a bit slower.