Horde3D
http://horde3d.org/forums/

Disabling software skinning makes _baseGeoRes dead
http://horde3d.org/forums/viewtopic.php?f=1&t=568		 Page 1 of 1
Author:  hayasaka [ 30.11.2008, 01:29 ]
Post subject:  Disabling software skinning makes _baseGeoRes dead
Changing ModelNodeParams::SoftwareSkinning of a model node from 1 to 0 discards the link to the base geometry resource and the node's own morphed geometry resource remains alive.

Is this an expected behavior?

I prefer disabling software skinning gets the base geometry resource back and accessible via _geometryRes as:

Code:
diff -u -r --exclude='*~' horde3d-svn-rev121/Horde3D/Source/Horde3DEngine/egModel.cpp horde3d-modified/Horde3D/Source/Horde3DEngine/egModel.cpp
--- horde3d-svn-rev121/Horde3D/Source/Horde3DEngine/egModel.cpp   2008-11-10 02:26:29.000000000 +0900
+++ horde3d-modified/Horde3D/Source/Horde3DEngine/egModel.cpp   2008-11-30 08:11:45.000000000 +0900
@@ -352,7 +352,8 @@
       return true;
    case ModelNodeParams::SoftwareSkinning:
       _softwareSkinning = (value != 0);
-      if( _geometryRes != 0x0 ) setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );
+      if( !_softwareSkinning && _baseGeoRes != 0x0 ) setParami( ModelNodeParams::GeometryRes, _baseGeoRes->getHandle() );
+      else if( _geometryRes != 0x0 ) setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );
       return true;
    default:
       return SceneNode::setParami( param, value );
Author:  Volker [ 30.11.2008, 17:08 ]
Post subject:  Re: Disabling software skinning makes _baseGeoRes dead
I think you're right. It might be also possible to avoid cloning the resource again, if we don't set the _baseGeoRes Resource to 0x0.

Code:
Index: Horde3DEngine/egModel.cpp
===================================================================
--- Horde3DEngine/egModel.cpp   (revision 122)
+++ Horde3DEngine/egModel.cpp   (working copy)
@@ -345,14 +345,15 @@
       else
       {
          _geometryRes = (GeometryResource *)res;
-         _baseGeoRes = 0x0;
+         _baseGeoRes = _geometryRes;
       }
 
       markMeshBBoxesDirty();
       return true;
    case ModelNodeParams::SoftwareSkinning:
       _softwareSkinning = (value != 0);
-      if( _geometryRes != 0x0 ) setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );
+      if( !_softwareSkinning && _baseGeoRes != 0x0 && _morphers.empty() ) setParami( ModelNodeParams::GeometryRes, _baseGeoRes->getHandle() );
+      else if( _geometryRes != 0x0 && _baseGeoRes.getPtr() == _geometryRes.getPtr() ) setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );      
       return true;
    default:
       return SceneNode::setParami( param, value );
@@ -364,7 +365,7 @@
 {
    if( !skinningDirty && !_morpherDirty ) return false;
 
-   if( _baseGeoRes == 0x0 || _baseGeoRes->getVertData() == 0x0 ) return false;
+   if( _baseGeoRes == 0x0 || _baseGeoRes.getPtr() == _geometryRes.getPtr() || _baseGeoRes->getVertData() == 0x0 ) return false;
    if( _geometryRes == 0x0 || _geometryRes->getVertData() == 0x0 ) return false;
    
    // Reset vertices to base data

Didn't test that code intensively so someone might think about it before we patch it in the repositories.
Author:  marciano [ 30.11.2008, 18:59 ]
Post subject:  Re: Disabling software skinning makes _baseGeoRes dead
I would do it like that:

Code:
Index: egModel.cpp
===================================================================
--- egModel.cpp   (revision 1)
+++ egModel.cpp   (working copy)
@@ -338,7 +338,7 @@
       if( !_morphers.empty() || _softwareSkinning )
       {
          Resource *clonedRes =  Modules::resMan().resolveResHandle(
-            Modules::resMan().cloneResource( _geometryRes->getHandle(), "" ) );
+            Modules::resMan().cloneResource( res->getHandle(), "" ) );
          _geometryRes = (GeometryResource *)clonedRes;
          _baseGeoRes = (GeometryResource *)res;
       }
@@ -352,7 +352,12 @@
       return true;
    case ModelNodeParams::SoftwareSkinning:
       _softwareSkinning = (value != 0);
-      if( _geometryRes != 0x0 ) setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );
+      
+      if( _softwareSkinning && _baseGeoRes == 0x0 && _geometryRes != 0x0 )
+         setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );
+      else if( !_softwareSkinning && _morphers.empty() && _baseGeoRes != 0x0 )
+         setParami( ModelNodeParams::GeometryRes, _baseGeoRes->getHandle() );
+
       return true;
    default:
       return SceneNode::setParami( param, value );
Author:  hayasaka [ 01.12.2008, 01:49 ]
Post subject:  Re: Disabling software skinning makes _baseGeoRes dead
The patch on my first post had a bug which missed to check _morphers and I found similar bug in marciano's patch.

Code:
Index: egModel.cpp
===================================================================
--- egModel.cpp (revision 122)
+++ egModel.cpp (working copy)
@@ -338,7 +338,7 @@
                if( !_morphers.empty() || _softwareSkinning )
                {
                        Resource *clonedRes =  Modules::resMan().resolveResHandle(
-                               Modules::resMan().cloneResource( _geometryRes->getHandle(), "" ) );
+                               Modules::resMan().cloneResource( res->getHandle(), "" ) );
                        _geometryRes = (GeometryResource *)clonedRes;
                        _baseGeoRes = (GeometryResource *)res;
                }
@@ -352,7 +352,15 @@
                return true;
        case ModelNodeParams::SoftwareSkinning:
                _softwareSkinning = (value != 0);
-               if( _geometryRes != 0x0 ) setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );
+     
+               {
+                       bool needClone = !_morphers.empty() || _softwareSkinning;
+                       if( needClone && _baseGeoRes == 0x0 && _geometryRes != 0x0 )
+                               setParami( ModelNodeParams::GeometryRes, _geometryRes->getHandle() );
+                       else if( !needClone && _baseGeoRes != 0x0 )
+                               setParami( ModelNodeParams::GeometryRes, _baseGeoRes->getHandle() );
+               }
+
                return true;
        default:
                return SceneNode::setParami( param, value );
Author:  marciano [ 06.12.2008, 21:33 ]
Post subject:  Re: Disabling software skinning makes _baseGeoRes dead
Thanks for your feedback. The only functional difference in your proposal is that you check if a morpher exists in the first if statement. But this information is already implicitely given by the _baseGeoRes == 0x0 check. If the geometry has a morpher, it also has cloned geometry and hence it stores a pointer to the base resource (_baseGeoRes != 0x0).
Author:  hayasaka [ 07.12.2008, 03:26 ]
Post subject:  Re: Disabling software skinning makes _baseGeoRes dead
Thank you for the review.
I finally understand you are right and there is no bugs on your patch.
Page 1 of 1 All times are UTC + 1 hour
Powered by phpBB® Forum Software © phpBB Group
https://www.phpbb.com/