Re: [osg-users] Bug in Cull Visitor

[Robert Osfield]

On 13 May 2018 at 18:48, Robert Osfield  wrote:
> On 13 May 2018 at 17:01, Gedalia Pasternak  wrote:
>> Does that mean it’s used incorrectly in
>> OcclusionQueryNode::getPassed()? That was what I was basing my understanding
>> on.
>
> I'm not the author of OcclisionQueyNode so I have to look it up,
> jikes, yep it shouldn't be using TraversalNumber, it should be using
> nv->getFrameStamp()->getFrameNumber().
>
> I we amend the code and get it checked in, but it'll have to wait a
> little longer as it's dinner time!! :-)

I have done another review the CullVisitor and OcclusionQueryNode and
the use of FrameNumber,  The OSG's rendering backend does
automatically assign the FrameStampe::getFrameNumber() to the
CullVisitor::setTraversalNumber() so it "should" be safe for it to use
the NodeVisitor::getTraveralNumber() in place of
getFrameStamp()->getFrameNumber().

However, looking that the code in CullVisitor that increments the
_traversalNumber value, this is mis-using that value for a traversal
order number which isn't the same thing as traversal number.  I've
amended the CullVisitor and RenderLeaf/RenderBin so that they now use
_traversalOrderNumber.  This is checked into 3.6 branch and master.

I also checked in a small tweak to the precision of the some of the
local variables in OcclusionQueryNode.

Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] Bug in Cull Visitor

[Robert Osfield]

Hi Gedalia,

On 13 May 2018 at 17:01, Gedalia Pasternak  wrote:
> Does that mean it’s used incorrectly in
> OcclusionQueryNode::getPassed()? That was what I was basing my understanding
> on.

I'm not the author of OcclisionQueyNode so I have to look it up,
jikes, yep it shouldn't be using TraversalNumber, it should be using
nv->getFrameStamp()->getFrameNumber().

I we amend the code and get it checked in, but it'll have to wait a
little longer as it's dinner time!! :-)

Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] Bug in Cull Visitor

[Gedalia Pasternak]

Does that mean it’s used incorrectly in
OcclusionQueryNode::getPassed()? That was what I was basing my
understanding on.
Gedalia


On Sun, May 13, 2018 at 10:57 AM Robert Osfield 
wrote:

> Hi Gedalia,
>
> On 13 May 2018 at 15:27, Gedalia Pasternak  wrote:
> >But won’t just simply removing it mean that adding a render leaf will
> be
> > changing what the current frame is?
>
> TraversalNumber is local value to a visitor it isn't a the frame
> number, this is retrieved with the osg::FrameStamp::getFrameStamp().
>
> >  For occlusion nodes I’ve realized that it’s not currently possible to
> use
> > them within instanced hierarchy without tracking which traversal
> generated a
> > given query, the cull visitor traversal index (along with other changes)
> can
> > help disambiguate which draw query it was.
>
> The FrameStamp is probably what you should be using, it's attached the
> CullVisitor and the osg::State as well as the viewer.
>
> Robert.
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
-- 
DI-Guy Engineering Lead, VT MÄK
150 Cambridge Park Drive, 3rd Floor, Cambridge, MA 02140
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] Bug in Cull Visitor

[Robert Osfield]

Hi Gedalia,

On 13 May 2018 at 15:27, Gedalia Pasternak  wrote:
>But won’t just simply removing it mean that adding a render leaf will be
> changing what the current frame is?

TraversalNumber is local value to a visitor it isn't a the frame
number, this is retrieved with the osg::FrameStamp::getFrameStamp().

>  For occlusion nodes I’ve realized that it’s not currently possible to use
> them within instanced hierarchy without tracking which traversal generated a
> given query, the cull visitor traversal index (along with other changes) can
> help disambiguate which draw query it was.

The FrameStamp is probably what you should be using, it's attached the
CullVisitor and the osg::State as well as the viewer.

Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] Bug in Cull Visitor

[Gedalia Pasternak]

Thanks Robert,
   But won’t just simply removing it mean that adding a render leaf will be
changing what the current frame is?
 For occlusion nodes I’ve realized that it’s not currently possible to use
them within instanced hierarchy without tracking which traversal generated
a given query, the cull visitor traversal index (along with other changes)
can help disambiguate which draw query it was.
Gedalia

On Sun, May 13, 2018 at 6:55 AM Robert Osfield 
wrote:

> Hi Gedalia,
>
> Well spotted, this issue has been in the code for almost two decades
> without anyone noticing.  I've removed the duplicate from CullVisitor
> and checked this into OSG master and the 3.6 branch.
>
> Robert.
>
> On 12 May 2018 at 21:34, Gedalia Pasternak  wrote:
> > Both CullVisitor and it's base class NodeVisitor have members named
> > _traversalNumber. Yielding duplicate member variables with the same name,
> > and inconsistent behavior depending on how you access the class.
> > Cull visitor's should be renamed, maybe to _cullTraversalNumber or
> something
> > that wouldn't conflict with the base class. An accessor would be nice as
> > well. In the case of NodeVisitor it's what frame it's up to, for cull
> > visitor it's what accepted object it's up to (
> >// Otherwise need to create new renderleaf.
> > RenderLeaf* renderleaf = new
> > RenderLeaf(drawable,projection,matrix,depth,_traversalNumber++);
> > )
> >
> > -Gedalia
> >
> > ___
> > osg-users mailing list
> > osg-users@lists.openscenegraph.org
> >
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
> >
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
-- 
DI-Guy Engineering Lead, VT MÄK
150 Cambridge Park Drive, 3rd Floor, Cambridge, MA 02140
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] Bug in Cull Visitor

[Robert Osfield]

Hi Gedalia,

Well spotted, this issue has been in the code for almost two decades
without anyone noticing.  I've removed the duplicate from CullVisitor
and checked this into OSG master and the 3.6 branch.

Robert.

On 12 May 2018 at 21:34, Gedalia Pasternak  wrote:
> Both CullVisitor and it's base class NodeVisitor have members named
> _traversalNumber. Yielding duplicate member variables with the same name,
> and inconsistent behavior depending on how you access the class.
> Cull visitor's should be renamed, maybe to _cullTraversalNumber or something
> that wouldn't conflict with the base class. An accessor would be nice as
> well. In the case of NodeVisitor it's what frame it's up to, for cull
> visitor it's what accepted object it's up to (
>// Otherwise need to create new renderleaf.
> RenderLeaf* renderleaf = new
> RenderLeaf(drawable,projection,matrix,depth,_traversalNumber++);
> )
>
> -Gedalia
>
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

[osg-users] Bug in Cull Visitor

[Gedalia Pasternak]

Both CullVisitor and it's base class NodeVisitor have members named
_traversalNumber. Yielding duplicate member variables with the same name,
and inconsistent behavior depending on how you access the class.
Cull visitor's should be renamed, maybe to _cullTraversalNumber or
something that wouldn't conflict with the base class. An accessor would be
nice as well. In the case of NodeVisitor it's what frame it's up to, for
cull visitor it's what accepted object it's up to (
   // Otherwise need to create new renderleaf.
RenderLeaf* renderleaf = new
RenderLeaf(drawable,projection,matrix,depth,_traversalNumber++);
)

-Gedalia
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org