HI Mathieu, I have just done a review of the IndexMeshVisitor::makeMesh() method and to me it looks like the switch against the PrimitiveSet:getMode() that sets the numSurfacePrimitives also has a default: return; entry which will mean that if their are any non surface primitives (i.e. lines/points) then the switch statement with return from the method without excuting any remeshing. The alternate of the the loop that does the switch not exiting will mean that the numSurfacePrimitives will always be non zero so that the !numSurfacePrimitives should always be false - the only exception would be if there are no primitivesets, which you would be more logical to test this directly.
Another oddity is that the statement: if (!numSurfacePrimitives || !numNonIndexedPrimitives) return; Is rather awkward to parse logically, a more clear version would be: if (numSurfacePrimitives==0 || numNonIndexedPrimitives==0) return; But personally I'd remove the numSurfacePrimitives and simply have a if (numPrimitiveSets==0) return; test which is done prior to any other of the tests. This leaves us with: if (numNonIndexedPrimitives==0) return; Which to me still is a bit odd, why does the method required that that has to be at least one non indexed primitive set? Surely it should work fine if all the primitive sets are indexed. In which case this statement is completely wrong and should exist at all. The only test that should exist is the return for non surface primitives and a return for an empty primitive set list. Thoughts? Robert. On 7 April 2014 14:05, Mathieu MARACHE <[email protected]> wrote: > While running the smooth normal visitor I found out that none of my meshes > got smoothed... Looking deeper into the code I found this code in > src/osgUtil/MeshOptimizers.cpp, the last line puzzled me : > > > // check for the existence of surface primitives > unsigned int numSurfacePrimitives = 0; > unsigned int numNonIndexedPrimitives = 0; > Geometry::PrimitiveSetList& primitives = geom.getPrimitiveSetList(); > Geometry::PrimitiveSetList::iterator itr; > for(itr=primitives.begin(); > itr!=primitives.end(); > ++itr) > { > switch((*itr)->getMode()) > { > case(PrimitiveSet::TRIANGLES): > case(PrimitiveSet::TRIANGLE_STRIP): > case(PrimitiveSet::TRIANGLE_FAN): > case(PrimitiveSet::QUADS): > case(PrimitiveSet::QUAD_STRIP): > case(PrimitiveSet::POLYGON): > ++numSurfacePrimitives; > break; > default: > // For now, only deal with polygons > return; > } > PrimitiveSet::Type type = (*itr)->getType(); > if (!(type == PrimitiveSet::DrawElementsUBytePrimitiveType > || type == PrimitiveSet::DrawElementsUShortPrimitiveType > || type == PrimitiveSet::DrawElementsUIntPrimitiveType)) > numNonIndexedPrimitives++; > } > > // nothing to index > if (!numSurfacePrimitives || !numNonIndexedPrimitives) return; > > > For me "nothing to index" should be testing either > > ! (numSurfacePrimitives || numNonIndexedPrimitive) > > or > > (!numSurfacePrimitives && !numNonIndexedPrimitives) > > Using one of the two gives me correct results. > > Please find the full file attached. > > -- > nǝıɥʇɐƜ > > _______________________________________________ > 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
