Horde3D

Next-Generation Graphics Engine
It is currently 02.01.2025, 14:42

View unanswered posts | View active topics


Board index » Horde3D Usage » General Discussion

All times are UTC + 1 hour




Post new topic Reply to topic  Page 1 of 1
  [ 6 posts ] 
  Print view Previous topic | Next topic 
Author Message
hayasaka
PostPosted: 30.11.2008, 01:29 
Offline

Joined: 21.11.2008, 04:06
Posts: 6
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 );
Top
 Profile  
Reply with quote  
Volker
PostPosted: 30.11.2008, 17:08 
Offline
Tool Developer

Joined: 13.11.2007, 11:07
Posts: 1150
Location: Germany
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.
Top
 Profile  
Reply with quote  
marciano
PostPosted: 30.11.2008, 18:59 
Offline
Engine Developer

Joined: 10.09.2006, 15:52
Posts: 1217
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 );
Top
 Profile  
Reply with quote  
hayasaka
PostPosted: 01.12.2008, 01:49 
Offline

Joined: 21.11.2008, 04:06
Posts: 6
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 );
Top
 Profile  
Reply with quote  
marciano
PostPosted: 06.12.2008, 21:33 
Offline
Engine Developer

Joined: 10.09.2006, 15:52
Posts: 1217
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).
Top
 Profile  
Reply with quote  
hayasaka
PostPosted: 07.12.2008, 03:26 
Offline

Joined: 21.11.2008, 04:06
Posts: 6
Thank you for the review.
I finally understand you are right and there is no bugs on your patch.
Top
 Profile  
Reply with quote  
Display posts from previous:  Sort by  
Post new topic Reply to topic  Page 1 of 1
  [ 6 posts ] 

Board index » Horde3D Usage » General Discussion

All times are UTC + 1 hour


Who is online

Users browsing this forum: No registered users and 12 guests

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group