[2.2] SceneFormat bugfix & suggestions

[rujialiu]

Hi!

I've took some serious look into the new SceneFormat component in Ogre 2.2. It's great to diagnose/optimize scenes, but I had to do a few adjustments to make it work:

1. OgreSceneFormat doesn't link HlmsPbs. I had to add it manually in VisualStudio
2. Only direct children of the root scene node are exported.
3. "saved_oitd_textures" is exported as a string, but the importer expects a bool.
Here is the patch to fix point 2&3. It works for all scenes I've tried so far.

Code: Select all

---
 .../SceneFormat/src/OgreSceneFormatExporter.cpp    | 40 +++++++++++++---------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/Components/SceneFormat/src/OgreSceneFormatExporter.cpp b/Components/SceneFormat/src/OgreSceneFormatExporter.cpp
index 6793ced..01ed748 100644
--- a/Components/SceneFormat/src/OgreSceneFormatExporter.cpp
+++ b/Components/SceneFormat/src/OgreSceneFormatExporter.cpp
@@ -720,9 +720,9 @@ namespace Ogre
                    MovableObject::getDefaultVisibilityFlags() );
 
         if( exportFlags & SceneFlags::TexturesOitd )
-            jsonStr.a( ",\n\t\"saved_oitd_textures\" : \"true\"" );
+            jsonStr.a( ",\n\t\"saved_oitd_textures\" : true" );
         if( exportFlags & SceneFlags::TexturesOriginal )
-            jsonStr.a( ",\n\t\"saved_original_textures\" : \"true\"" );
+            jsonStr.a( ",\n\t\"saved_original_textures\" : true" );
 
         flushLwString( jsonStr, outJson );
 
@@ -744,20 +744,28 @@ namespace Ogre
                 exportSceneNode( jsonStr, outJson, rootSceneNode );
                 outJson += "\n\t\t}";
 
-                Node::NodeVecIterator nodeItor = rootSceneNode->getChildIterator();
-                while( nodeItor.hasMoreElements() )
-                {
-                    Node *node = nodeItor.getNext();
-                    SceneNode *sceneNode = dynamic_cast<SceneNode*>( node );
-
-                    if( sceneNode && mListener->exportSceneNode( sceneNode ) )
-                    {
-                        mNodeToIdxMap[sceneNode] = nodeCount++;
-                        outJson += ",\n\t\t{";
-                        exportSceneNode( jsonStr, outJson, sceneNode );
-                        outJson += "\n\t\t}";
-                    }
-                }
+				std::queue<SceneNode*> nodeQueue;
+				nodeQueue.push(rootSceneNode);
+
+				while (!nodeQueue.empty()) {
+					SceneNode* frontNode = nodeQueue.front();
+					nodeQueue.pop();
+					Node::NodeVecIterator nodeItor = frontNode->getChildIterator();
+					while (nodeItor.hasMoreElements())
+					{
+						Node *node = nodeItor.getNext();
+						SceneNode *sceneNode = dynamic_cast<SceneNode*>(node);
+
+						if (sceneNode && mListener->exportSceneNode(sceneNode))
+						{
+							mNodeToIdxMap[sceneNode] = nodeCount++;
+							outJson += ",\n\t\t{";
+							exportSceneNode(jsonStr, outJson, sceneNode);
+							outJson += "\n\t\t}";
+							nodeQueue.push(sceneNode);
+						}
+					}
+				}
             }
             outJson += "\n\t]";
         }

Some other things:
* Can you export/import camera information? It'll be great if we can preserve camera's position/rotation/fov etc.
* Floating point numbers are exported as "raw bytes", which makes manual inspecting/writing scripts harder. For example, I'd like to write scripts to "prune" the scene graph before importing. Can we have an option to export "human-readable" floating point numbers?
* Datablocks: emissive is not exported
* PCC: the "pause" parameter is exported as integer, but importer expects a bool (I'm not using it anyway, but the code says so...)

Thanks!

[rujialiu]

Thanks for pushing my fix and make other changes :D

There was one more frustrating exception when exporting scenes:

Code: Select all

16:59:01: OGRE EXCEPTION(2:InvalidParametersException): Texture 'dummy.jpg' must be resident or becoming resident!!! in Image2::convertFromTexture at C:\OGRE22\OgreMain\src\OgreImage2.cpp (line 283)

It's somewhat common in our use case because some materials/textures are created procedurally or fetched from Internet on the fly, so some "never-to-be-resident" textures may exist but no material is using them. It'll be good if the exporter can ignore them, and the exported scene is still complete (no missing textures that are actually used)

My solution is:

Code: Select all

 Components/Hlms/Common/include/OgreHlmsTextureBaseClass.inl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Components/Hlms/Common/include/OgreHlmsTextureBaseClass.inl b/Components/Hlms/Common/include/OgreHlmsTextureBaseClass.inl
index fc7ae10..4e2d8de 100644
--- a/Components/Hlms/Common/include/OgreHlmsTextureBaseClass.inl
+++ b/Components/Hlms/Common/include/OgreHlmsTextureBaseClass.inl
@@ -85,6 +85,9 @@ namespace Ogre
 
             if( texture )
             {
+				if (texture->getResidencyStatus() != GpuResidency::Resident ||
+					texture->getNextResidencyStatus() != GpuResidency::Resident) continue; // See OgreImage2 line 283
+
                 String resourceName = texture->getRealResourceNameStr();
 
                 //Render Targets are... complicated. Let's not, for now.

It works for every large scene I've tried so far. Is it reasonable to do this?

[dark_sylinc]

Code: Select all

16:59:01: OGRE EXCEPTION(2:InvalidParametersException): Texture 'dummy.jpg' must be resident or becoming resident!!! in Image2::convertFromTexture at C:\OGRE22\OgreMain\src\OgreImage2.cpp (line 283)

It's somewhat common in our use case because some materials/textures are created procedurally or fetched from Internet on the fly, so some "never-to-be-resident" textures may exist but no material is using them. It'll be good if the exporter can ignore them, and the exported scene is still complete (no missing textures that are actually used)

My solution is:
...
It works for every large scene I've tried so far. Is it reasonable to do this?

I checked the code and we were already checking "if( texture->getNextResidencyStatus() == GpuResidency::Resident )" for OITD dumps; so your report shouldn't be happening. That's when I realized the error condition in Image2 is what was wrong.

If you hit that error, it's probably because the texture was not resident yet, but scheduled to be resident. Probably the exporter just "won" the race against the texture being loaded?

Alternatively, the texture was manually created and TextureGpu::_setNextResidencyStatus() was called and you're never calling _transitionTo first (i.e. see OgreFont.cpp, OgreIrradianceVolume.cpp). If that's the case (which is not valid/recommended), then Image2 will be deadlocked in the texture->waitForData() call.