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

Reply via email to