Hi Sam, You fix looks sensible. I am ready to merge then change but will wait for clarification of the name you'd like me to assign for the submission - when I merged a submission from users I used the commit message of the form : "From Joe Blogs : bug fix that makes the OSG 1,000,000 times faster".
So could you let me know your full name or a psuedo name that you plan to use consistently. Cheers, Robert. On 17 June 2014 07:48, Samuel M <[email protected]> wrote: > > Hello, > > while the MergeGeometryVistor condition was patched, it still contains a > bug, effectively using the entire list of geometries to merge, which is > wrong. > > Attached Optimizer.cpp patches this. This is a modified revision 14262. > > > Regards, > Sam > > P.S: Sorry if this mail is sent twice. I had issues with the subscription. > > >> Date: Mon, 16 Jun 2014 17:19:20 +0100 >> From: [email protected] >> To: [email protected] >> Subject: Re: [osg-users] MergeGeometryVisitor behaviour, is this a bug? >> >> 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 > > _______________________________________________ > 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
