Hi Mathieu, I don't see why the code would need to not run when DrawArrays are used. All surface mesh types can be optimized, rejecting DrawArrays would make the assumption that all DrawArray based geometries are somehow made perfectly efficiently and not need mesh optimization.
I can't see what DrawArrays and DrawElements should be treated any differently. On 17 April 2014 13:07, Mathieu MARACHE <[email protected]> wrote: > Hi Robert, > > On 15 April 2014 19:44, Robert Osfield <[email protected]> wrote: >> >> 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; > > > Yes that look fine by me > >> >> >> 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. > > > I think the idea is to return also if any drawArrays are present. I changed > the code to be like this > > > // check for the existence of surface primitives > Geometry::PrimitiveSetList& primitives = geom.getPrimitiveSetList(); > Geometry::PrimitiveSetList::iterator itr; > if (primitives.empty()) > return; > > for(itr=primitives.begin(); > itr!=primitives.end(); > ++itr) > { > PrimitiveSet::Mode mode = (*itr)->getMode(); > if (!( mode == PrimitiveSet::TRIANGLES || > mode == PrimitiveSet::TRIANGLE_STRIP || > mode == PrimitiveSet::TRIANGLE_FAN || > mode == PrimitiveSet::QUADS || > mode == PrimitiveSet::QUAD_STRIP || > mode == PrimitiveSet::POLYGON )) > return; > PrimitiveSet::Type type = (*itr)->getType(); > if (!( type == PrimitiveSet::DrawElementsUBytePrimitiveType || > type == PrimitiveSet::DrawElementsUShortPrimitiveType || > type == PrimitiveSet::DrawElementsUIntPrimitiveType )) > return; > } > > Regards, > 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
