Yeah, the patch is incorrect. I've taken an initial relook at the code and have spotted a couple of possible optimizations, in addition to general cleanup. I will try to get an updated version to you by the end of the week.
Tim On Mon, Apr 20, 2015 at 11:21 AM, Robert Osfield <[email protected]> wrote: > Hi Tim, > > On 15 April 2015 at 09:01, Tim Moore <[email protected]> wrote: > > > > > > On Mon, Mar 16, 2015 at 2:51 PM, Robert Osfield < > [email protected]> > > wrote: > >> > >> Hi Alberto, > >> > >> Thanks for the further info. I'm not the author of this particular file > >> so I'm learning from just sitting down reviewing the code. The code in > >> question is a good example of C++ gone bad - it's an unreadable mess. > > > > Unreadability is in the eyes of the reader :) I personally find that > adding > > std:: to symbols that are quite, well, standard clutters up the code and > > makes it less, if not un-, readable. To each his own. Having said that, > this > > code is quite complex and the comments are sparse. I'll see if I can > improve > > them in another patch. > >> > >> > >> The use of the using namespace std at the top of the file is hindering > >> reability. So I've just removed this. My compiler seems to be a bit > too > >> lenient with not reporting all of the missing std::'s so I'll reboot > later > >> under Windows to see if VS2013 is any better. > >> > >> Looking at the MeshOptimizers.cpp there are two instances of the use of > >> std::remove(), with only one of them modified in your submitted code. I > >> need to sit down and fully review the code to see what is meant in each > >> instance. > >> > > The first use of remove() in LRUCache::addEntries() must not be wrapped > with > > erase(). The removed entries are overwritten with new entries. > > > > The patch concerns the remove() in > VertexCacheVisitor::doVertexOptimization. > > erase() isn't appropriate here either: the code is in a tight loop, and > the > > whole vector is deleted at the end of the function anyway. However, I'm > not > > convinced that the remove() is doing anything useful and will ponder > that. > > Does this mean the original patch with the addition of the erase() is > incorrect? > > Have you had a chance to have a look at refining the MeshoOptimizers.cpp? > > Cheers, > Robert. > _______________________________________________ > 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
