Hi Sam, It looks like a bug to me and your suggested fix looks appropriate. I've applied this fix to svn/trunk and OSG-3.2 braches.
Thanks, Robert. On 16 June 2014 12:00, Samuel M <[email protected]> wrote: > Hello, > > we were using the MergeGeometryVisitor for our geometry and lately realized > that it totally ignores the setTargetMaximumNumberOfVertices settingf 10000 > (or actually every setting). > > While trying to comprehend what bool > Optimizer::MergeGeometryVisitor::mergeGeode(osg::Geode& geode) is trying to > do I came across this code: > > unsigned int numVertices(duplicateList.front()->getVertexArray() ? > duplicateList.front()->getVertexArray()->getNumElements() : 0); > DuplicateList::iterator eachGeom(duplicateList.begin()+1); > // until all geometries have been checked or _targetMaximumNumberOfVertices > is reached > for (;eachGeom!=duplicateList.end(); ++eachGeom) > { > unsigned int numAddVertices((*eachGeom)->getVertexArray() ? > (*eachGeom)->getVertexArray()->getNumElements() : 0); > if (numVertices+numAddVertices<_targetMaximumNumberOfVertices) > { > break; > } > else > { > numVertices += numAddVertices; > } > } > > // push back if bellow the limit > if (numVertices<_targetMaximumNumberOfVertices) > { > if (duplicateList.size()>1) needToDoMerge = true; > mergeList.push_back(duplicateList); > mergeListChecked.erase(itr); > } > > > This piece of code iterates over a given list of duplicates and fetches the > vertex count of the first object. Then it iterates over second to last > objects BUT for the second element it fetches numAddVertices and enters the > given condition: > if (numVertices+numAddVertices<_targetMaximumNumberOfVertices) > > Our scenario has 3 vertices in first and 3 vertices in second geometry which > result in 3+3 < 100000. So the code directly breaks. > Then it checks again for numVertices < targetMaximumNumberOfVertices which > is still true and then PUSHES the entire list into mergeList. > > Our example is a flight file consisting of thousands of geodes having only 3 > vertcies. The duplicateList has several thousand elements but the code > checks only the first two and then pushes the first two AND the remaining > thousands of geodes into the list to merge exceeding the > _targetMaximumNumberOfVertices. > > > Shouldn't the condition > if (numVertices+numAddVertices<_targetMaximumNumberOfVertices) > be > if (numVertices+numAddVertices>_targetMaximumNumberOfVertices) > > and the mergeList.push_back(duplicateList); > pushing only second to eachGeom range into the list? > > Regards, > Sam > > _______________________________________________ > osg-users mailing list > [email protected] > http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org > _______________________________________________ osg-users mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
