SceneMgr::_renderScene() updates scenegraph AFTER shadows?

[lunkhound]

I encountered a bug where shadows were lagging the camera by a frame. I tracked the problem down to the order of things in SceneManager::_renderScene().
The order was prepareShadowTextures(), then updateSceneGraph().

Here is a sort of pseudocode of the flow to illustrate the bug (each level of indention is a deeper level of the call stack):

Code: Select all

SceneManager::_renderScene( main camera )
    ->prepareShadowTextures()
          ->set up shadow cameras (shadow space projection matrices are pushed into GPU params)
          ->render shadow maps
                ->_renderScene( shadow camera ) (recursive call to render the first shadow map)
                      ->updateSceneGraph() (main camera is moved here because it is attached to a sceneNode, and this is the first time in the frame updateSceneGraph has been called)
                      ->render shadow map
    ->updateSceneGraph()
    ->render main camera view (using stale matrices from the previous camera state)

It seems pretty obvious that this is the wrong order to do things. Updating the scene graph and determining the final camera position should be done BEFORE preparing the shadow maps which DEPEND on the camera being up to date.
I believe the only reason this bug hasn't been noticed before is that the recursive calls to _renderScene() tend mask the problem in many cases. In my case, this bug was only apparent when camera-relative-rendering mode was on (and the camera had to be parented to a scene node).

I would expect that if there was a weird and obscure reason why updating the scenegraph should come after preparing shadow textures, that there would be a comment in the code explaining it. But there isn't.
So barring any objections, I'll submit a patch for this to v1-8 branch.

[Kaylx]

Keep us posted on when you submit that patch.

[lunkhound]

I've put the patch on my bitbucket fork:

https://bitbucket.org/cc8192/ogre/commi ... d6eebe15a3

and requested a pull.

[masterfalcon]

Thanks. I'll take a look at it. But I don't anticipate any problems

[TheSHEEEP]

But I don't anticipate any problems

Oh oh. Now you've challenged fate!

[Wolfmanfx]

I know that this changed by patch now its reverted by a patch funny thing (the first one were did in 1.7 branch long time ago)

[lunkhound]

I know that this changed by patch now its reverted by a patch funny thing (the first one were did in 1.7 branch long time ago)

Interesting. Its commit #2053 by Sinbad. Comment says "Patch 2986437: auto-tracking update should be after the re-entrant call to update shadow textures, otherwise old tracking state may be used"

Not sure why the auto-tracking would want to come after shadow textures. Anybody know what that means?

[dark_sylinc]

I don't know, but if our shadows were lagging behind one frame, it could really explain our overly excessive acne.

I wouldn't be surprised that changing the order breaks something else. As I was designing the docs for 2.0, I've discovered the update order wasn't really strongly defined, with a couple possible catch-22 scenarios.

[sparkprime]

Surely anything other than the following order is insane:

1) update scenegraph
2) update auto tracking
3) update shadows (including preparation of matrixes, culling for cast passes, and render)
4) receiver passes


Why is there any re-entrancy?

[lunkhound]

Well I hope my patch doesn't break things too badly. Or at least that it fixes more than it breaks...
I'll be so glad to see this mess cleaned up in 2.0!