Hi Kristofer,
I have now had the work window to allow me to do another review and have
have gone ahead and merged your proposed changes. The only tweaks I needed
to make were to the header:
$ diff AntiSquish ~/OpenSceneGraph/include/osgManipulator/
23,26d22
< namespace osg {
< class NodeVisitor;
< }
<
64,65d59
< virtual ~AntiSquish();
<
69a64,66
>
> virtual ~AntiSquish();
>
78c75
< mutable OpenThreads::Mutex _cacheLock;
---
> mutable OpenThreads::Mutex _cacheLock;
The forward declare of NodeVisitor isn't required as the include/osg/Group
header includes NodeVisitor already, moved the destructor to protected
section to avoid it being created on the heap (this is C++ trick to avoid
nodes being created on the stack and being deleted unintentionally) and a
small tweak to the indentation.
Unfortunately I don't have an OSG example that tests the AntiSquish node so
I don't yet have an evidence that it's working perfectly. If you made one
in your own testing or a .osgt model any chance you could pass it along?
On the assumption that you'll be testing I've checked in the new
AntiSquish implementation to svn/trunk so you can get testing it.
Cheers,
Robert.
On 10 January 2014 08:34, Kristofer Tingdahl
<[email protected]>wrote:
> Hi Robert,
>
> I don't necessarily agree that a way to check if the transforms along a
> nodepath using integers is not worthwhile, as you replace hundreds of
> double-operations with a few integer ones, and the comparison becomes the
> comparison of two ints. That seems useful, especially as there is hardly
> any overhead (a few bytes per transform).
>
> I'll leave that discussion and submit a new update of the AntiSquish. It
> now inherits Transform rather than MatrixTransform, so there are no changes
> to the object while traversing (apart from one cache). This solves most of
> your objections as we rely on the standardness of the Transform. One
> question is however what to do when there is no node-path available, such
> as the Transform::computeBound(). In those cases I rely on the cache (if
> available). But if I do that, I should really dirty the bound when I
> re-compute my cache. This is implemented now in the version I'm sending,
> but I'm not sure it is necessary. It will create an overload of
> computeBound calls.
>
> I opted to lock the cache as this is the only way to protect from corrupt
> cache if there are multiple traversals going on at the same time. We don't
> do that, but I thought it's safer.
>
> Best regards,
>
>
> Kristofer Tingdahl
>
> _______________________________________________
> 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
