Hi Miha, Sorry for the delay. I have just got back to reviewing your changes for sharing PrimitiveSets and unfortunately there have been lots of recent changes to the serializer code that make it less clear what parts are written by you and are still appropriate to merge.
Could you have a look at applying your changes to svn/trunk and let me have the changes relative to svn/trunk. Thanks, Robert. On 5 September 2012 18:03, Miha Ravšelj <[email protected]> wrote: > Hi Robert, > > my example was meant just as an illustration of shared PrimitiveSet problem > from I/O perspective. > > Bellow is a complete working example which more clearly indicates shared > PrimitiveSet inconsistency. I haven't spent much time discovering shared > primitive set problem Paul desribed(MergeGeometryVisitor) but if we look at > purely I/O problem it produces invalid output (Attached geode.osgt) with > two instances of DrawArrays instead of one shared. > > //--------------------------------------------------------------------------------------------------------------------- > osg::ref_ptr<osg::Geode> geode = new osg::Geode; > > osg::ref_ptr<osg::Geometry> geometry1 = new osg::Geometry; > osg::ref_ptr<osg::Vec3Array> vertexArray1 = new osg::Vec3Array; > vertexArray1->push_back(osg::Vec3(0.f,0.f,0.f)); > vertexArray1->push_back(osg::Vec3(10.f,0.f,0.f)); > vertexArray1->push_back(osg::Vec3(10.f,0.f,10.f)); > vertexArray1->push_back(osg::Vec3(0.f,0.f,10.f)); > geometry1->setVertexArray(vertexArray1); > > osg::ref_ptr<osg::DrawArrays> sharedDrawArrays = new > osg::DrawArrays(osg::PrimitiveSet::LINE_LOOP,0,4); > geometry1->addPrimitiveSet(sharedDrawArrays); > > osg::ref_ptr<osg::Geometry> geometry2 = new osg::Geometry; > osg::ref_ptr<osg::Vec3Array> vertexArray2 = new osg::Vec3Array; > vertexArray2->push_back(osg::Vec3(10.f,0.f,10.f)); > vertexArray2->push_back(osg::Vec3(20.f,0.f,10.f)); > vertexArray2->push_back(osg::Vec3(20.f,0.f,20.f)); > vertexArray2->push_back(osg::Vec3(10.f,0.f,20.f)); > geometry2->setVertexArray(vertexArray2); > geometry2->addPrimitiveSet(sharedDrawArrays); > geode->addDrawable(geometry1); > geode->addDrawable(geometry2); > > osgDB::writeNodeFile(*geode,"geode.osgt"); > osg::ref_ptr<osg::Node> readNode = osgDB::readNodeFile("geode.osgt"); > > osg::PrimitiveSet* ps1 = > geode->getDrawable(0)->asGeometry()->getPrimitiveSet(0); > osg::PrimitiveSet* ps2 = > geode->getDrawable(1)->asGeometry()->getPrimitiveSet(0); > > bool sharedBeforeWrite = ps1 == ps2; //ok shared primitive set > ps1 = > readNode->asGeode()->getDrawable(0)->asGeometry()->getPrimitiveSet(0); > ps2 = > readNode->asGeode()->getDrawable(1)->asGeometry()->getPrimitiveSet(0); > > bool sharedAfterRead = ps1 == ps2; //not ok - two instances > //--------------------------------------------------------------------------------------------------------------------- > > I understand that previous changes cause problems with > backward-compatibility of osg files so I have made some more advanced code > changes that are attached in zip file that supports backward compatibility. > After applying patch upper code works as expected. Since PrimitiveSet is > special class that is actually inferior to Geometry class( only one > occurence ) this changes can be made without too much changes. > > Suggestion: Maybe it is also better to entirely remove readPrimitiveSet and > WritePrimitiveSet functions from InputStream and OutputStream classes and > just implement two UserSerializer: PrimitiveSetList and > UniquePrimitiveSetList inside Geometry wrapper class. This way code will be > better organized. > > My changes however preserves current code organization adding just one user > serializer to handle shared primitive set I/O and leaving old code as-is. > > *Geometry.cpp is now set to UPDATE_TO_VERSION(93). If changes are > appropriate and will be merged to trunk this number should be changed to > appropriate repository soversion. > > > > Time for beer. Cheers, > Miha > > > > > On Wed, Sep 5, 2012 at 12:03 PM, Robert Osfield <[email protected]> > wrote: >> >> Hi Miha, >> >> I've looked into the issue and have experimented with adding your >> changes, albeit with an extra version check to ensure backwards >> compatibility. I'm not yet sure whether to merge the changes though >> as it'll detract from the readability of the .osgt file as you get >> extra tokens that are used for 99% of OSG applications. >> >> You mention that you sample code results in a invalid osg file, but I >> believe it's valid, it's just written out as a object file rather than >> a scene file so can't be read as a scene file. To have a scene file >> you have to have nodes, so have to decorate the geometry with a geode >> and then write out the geode to disk via writeNodeFile(). Once I >> added this to your example code below the file wrote out and read in >> just fine using osgconv and osgviewer. >> >> My guess is that you were attempting to read an object file as a full >> scene graph file but the scene graph level loaders won't load it as >> it's not a scene graph without nodes. >> >> Robert. >> >> On 16 August 2012 16:32, Miha Ravšelj <[email protected]> wrote: >> > I found out that writePrimitiveSet and readPrimitiveSet does not have >> > shared >> > object implementation(uniqueID) in InputStream and OutputStream. Is this >> > by >> > design or was accidentally forgotten during development? >> > >> > The following example produces invalid osg file: >> > >> > osg::ref_ptr<osg::Geometry> geometry = new osg::Geometry; >> > osg::ref_ptr<osg::DrawArrays> drawArray = new >> > osg::DrawArrays(GL_TRIANGLE_STRIP, 0,4); >> > geometry->addPrimitiveSet(drawArray); >> > geometry->addPrimitiveSet(drawArray); >> > >> > osgDB::writeObjectFile(*geometry,"test.osgt"); >> > >> > Second primitiveset should be written only with uniqueID and not >> > duplicated. >> > This causes invalid state when object is read back. >> > >> > I have attached fixed InputStream.cpp and OutputStream.cpp which >> > resolves >> > issue described above. >> > >> > Miha >> > >> > _______________________________________________ >> > osg-submissions mailing list >> > [email protected] >> > >> > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org >> > >> _______________________________________________ >> osg-submissions mailing list >> [email protected] >> >> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org > > > > _______________________________________________ > osg-submissions mailing list > [email protected] > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org > _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
