OcclusionQueryNode: fix use case of user defined query geometry

The user defined query geometry handling has been broken in several ways.

The previous way of defining a query geometry was using the non const
`getQueryGeometry` method and overriding its members. But then
`OcclusionQueryNode` wasn't aware of the geometry change and couldn't
internally handle it correctly.

The `computeBound` method never considered a user defined query geometry and
always just overrode the vertices of the geometry.

The member `_validQueryGeometry` wasn't correctly set.

This change should fix all this issues by introducing a small backward
compatibility break. The non const `getQueryGeometry` method is removed
forcing the user to use the `setQueryGeometry` method. But then `OcclusionQueryNode`
is aware of the user defined query geometry and can handle it correctly.

include/osg/OcclusionQueryNode

@@ -82,9 +82,9 @@ public:
     };
 
     /** return a QueryResult for specified Camera, where the QueryResult.valid is true when query results are available, and in which case the QueryResult.numPixels provides the num of pixels in the query result.*/
-    QueryResult getQueryResult( const osg::Camera* cam );
+    QueryResult getQueryResult( const osg::Camera* cam ) const;
 
-    unsigned int getNumPixels( const osg::Camera* cam );
+    unsigned int getNumPixels( const osg::Camera* cam ) const;
 
     virtual void releaseGLObjects( osg::State* state = 0 ) const;
 
@@ -158,8 +158,11 @@ public:
     osg::StateSet* getQueryStateSet();
     const osg::StateSet* getQueryStateSet() const;
 
-    // Get the QueryGeometry object used for occlusion query. Returns 0 if no QueryGeometry is created.
+    // Set and get the QueryGeometry object used for the occlusion query.
-    osg::QueryGeometry* getQueryGeometry();
+    // By default an axis aligned box is used as the query geometry.
+    // Resetting to the default query geometry is done by setting it to 0.
+    // Returns 0 if no QueryGeometry is created.
+    void setQueryGeometry( osg::QueryGeometry* geom );
     const osg::QueryGeometry* getQueryGeometry() const;
 
     // Set and get the StateSet used by the OcclusionQueryNode
@@ -189,17 +192,28 @@ public:
     static void discardDeletedQueryObjects( unsigned int contextID );
 
 protected:
+    enum QueryGeometryState {
+        INVALID,
+        VALID,
+        USER_DEFINED
+    };
+
     virtual ~OcclusionQueryNode();
 
     virtual void createSupportNodes();
 
+    bool isQueryGeometryValid() const { return _queryGeometryState != INVALID; }
+
+    void setQueryGeometryInternal( osg::QueryGeometry* queryGeom,
+                                   osg::Geometry* debugQueryGeom,
+                                   QueryGeometryState state );
+
     osg::ref_ptr< osg::Geode > _queryGeode;
     osg::ref_ptr< osg::Geode > _debugGeode;
 
     bool _enabled;
 
-    // If the box of the query geometry is valid.
+    mutable QueryGeometryState _queryGeometryState;
-    mutable bool _validQueryGeometry;
 
     // Tracks the last frame number that we performed a query.
     // User can set how many times  (See setQueryFrameCount).

src/osg/OcclusionQueryNode.cpp

@@ -48,6 +48,36 @@
 namespace osg
 {
 
+QueryGeometry* createDefaultQueryGeometry( const std::string& name )
+{
+    GLushort indices[] = { 0, 1, 2, 3,  4, 5, 6, 7,
+        0, 3, 6, 5,  2, 1, 4, 7,
+        5, 4, 1, 0,  2, 7, 6, 3 };
+
+    ref_ptr<QueryGeometry> geom = new QueryGeometry( name );
+    geom->setDataVariance( Object::DYNAMIC );
+    geom->addPrimitiveSet( new DrawElementsUShort( PrimitiveSet::QUADS, 24, indices ) );
+
+    return geom.release();
+}
+
+Geometry* createDefaultDebugQueryGeometry()
+{
+    GLushort indices[] = { 0, 1, 2, 3,  4, 5, 6, 7,
+        0, 3, 6, 5,  2, 1, 4, 7,
+        5, 4, 1, 0,  2, 7, 6, 3 };
+
+    ref_ptr<Vec4Array> ca = new Vec4Array;
+    ca->push_back( Vec4( 1.f, 1.f, 1.f, 1.f ) );
+
+    ref_ptr<Geometry> geom = new Geometry;
+    geom->setDataVariance( Object::DYNAMIC );
+    geom->setColorArray( ca.get(), Array::BIND_OVERALL );
+    geom->addPrimitiveSet( new DrawElementsUShort( PrimitiveSet::QUADS, 24, indices ) );
+
+    return geom.release();
+}
+
 // Create and return a StateSet appropriate for performing an occlusion
 //   query test (disable lighting, texture mapping, etc). Probably some
 //   room for improvement here. Could disable shaders, for example.
@@ -364,7 +394,7 @@ QueryGeometry::drawImplementation( osg::RenderInfo& renderInfo ) const
 
 }
 
-QueryGeometry::QueryResult QueryGeometry::getQueryResult( const osg::Camera* cam )
+QueryGeometry::QueryResult QueryGeometry::getQueryResult( const osg::Camera* cam ) const
 {
     osg::ref_ptr<osg::TestResult> tr;
     {
@@ -380,7 +410,7 @@ QueryGeometry::QueryResult QueryGeometry::getQueryResult( const osg::Camera* cam
 }
 
 unsigned int
-QueryGeometry::getNumPixels( const osg::Camera* cam )
+QueryGeometry::getNumPixels( const osg::Camera* cam ) const
 {
     return getQueryResult(cam).numPixels;
 }
@@ -442,7 +472,7 @@ QueryGeometry::discardDeletedQueryObjects( unsigned int contextID )
 
 OcclusionQueryNode::OcclusionQueryNode()
   : _enabled( true ),
-    _validQueryGeometry( false ),
+    _queryGeometryState( INVALID ),
     _passed(false),
     _visThreshold( 500 ),
     _queryFrameCount( 5 ),
@@ -460,7 +490,7 @@ OcclusionQueryNode::~OcclusionQueryNode()
 
 OcclusionQueryNode::OcclusionQueryNode( const OcclusionQueryNode& oqn, const CopyOp& copyop )
   : Group( oqn, copyop ),
-    _validQueryGeometry( false ),
+    _queryGeometryState( INVALID ),
     _passed( false )
 {
     _enabled = oqn._enabled;
@@ -485,7 +515,7 @@ bool OcclusionQueryNode::getPassed( const Camera* camera, NodeVisitor& nv )
 
     QueryGeometry* qg = static_cast< QueryGeometry* >( _queryGeode->getDrawable( 0 ) );
 
-    if ( !_validQueryGeometry )
+    if ( !isQueryGeometryValid() )
     {
         // There're cases that the occlusion test result has been retrieved
         // after the query geometry has been changed, it's the result of the
@@ -556,7 +586,7 @@ bool OcclusionQueryNode::getPassed( const Camera* camera, NodeVisitor& nv )
 
 void OcclusionQueryNode::traverseQuery( const Camera* camera, NodeVisitor& nv )
 {
-    if (!_validQueryGeometry)
+    if (!isQueryGeometryValid())
         return;
 
     bool issueQuery;
@@ -590,6 +620,8 @@ BoundingSphere OcclusionQueryNode::computeBound() const
         //   an application thread or by a non-osgViewer application.
         OpenThreads::ScopedLock<OpenThreads::Mutex> lock( _computeBoundMutex )  ;
 
+        if (_queryGeometryState != USER_DEFINED)
+        {
             // This is the logical place to put this code, but the method is const. Cast
             //   away constness to compute the bounding box and modify the query geometry.
             osg::OcclusionQueryNode* nonConstThis = const_cast<osg::OcclusionQueryNode*>( this );
@@ -599,7 +631,7 @@ BoundingSphere OcclusionQueryNode::computeBound() const
             nonConstThis->accept( cbv );
             BoundingBox bb = cbv.getBoundingBox();
             const bool bbValid = bb.valid();
-        _validQueryGeometry = bbValid;
+            _queryGeometryState = bbValid ? VALID : INVALID;
 
             osg::ref_ptr<Vec3Array> v = new Vec3Array;
             v->resize( 8 );
@@ -627,6 +659,7 @@ BoundingSphere OcclusionQueryNode::computeBound() const
             geom = static_cast< osg::Geometry* >( nonConstThis->_debugGeode->getDrawable( 0 ) );
             geom->setVertexArray( v.get() );
         }
+    }
 
     return Group::computeBound();
 }
@@ -723,21 +756,12 @@ bool OcclusionQueryNode::getPassed() const
 
 void OcclusionQueryNode::createSupportNodes()
 {
-    GLushort indices[] = { 0, 1, 2, 3,  4, 5, 6, 7,
-        0, 3, 6, 5,  2, 1, 4, 7,
-        5, 4, 1, 0,  2, 7, 6, 3 };
-
     {
         // Add the test geometry Geode
         _queryGeode = new Geode;
         _queryGeode->setName( "OQTest" );
         _queryGeode->setDataVariance( Object::DYNAMIC );
-
+        _queryGeode->addDrawable( createDefaultQueryGeometry( getName() ) );
-        ref_ptr< QueryGeometry > geom = new QueryGeometry( getName() );
-        geom->setDataVariance( Object::DYNAMIC );
-        geom->addPrimitiveSet( new DrawElementsUShort( PrimitiveSet::QUADS, 24, indices ) );
-
-        _queryGeode->addDrawable( geom.get() );
     }
 
     {
@@ -746,17 +770,7 @@ void OcclusionQueryNode::createSupportNodes()
         _debugGeode = new Geode;
         _debugGeode->setName( "Debug" );
         _debugGeode->setDataVariance( Object::DYNAMIC );
-
+        _debugGeode->addDrawable( createDefaultDebugQueryGeometry() );
-        ref_ptr<Geometry> geom = new Geometry;
-        geom->setDataVariance( Object::DYNAMIC );
-
-        ref_ptr<Vec4Array> ca = new Vec4Array;
-        ca->push_back( Vec4( 1.f, 1.f, 1.f, 1.f ) );
-        geom->setColorArray( ca.get(), Array::BIND_OVERALL );
-
-        geom->addPrimitiveSet( new DrawElementsUShort( PrimitiveSet::QUADS, 24, indices ) );
-
-        _debugGeode->addDrawable( geom.get() );
     }
 
     // Creste state sets. Note that the osgOQ visitors (which place OQNs throughout
@@ -767,6 +781,28 @@ void OcclusionQueryNode::createSupportNodes()
 }
 
 
+void OcclusionQueryNode::setQueryGeometryInternal( QueryGeometry* queryGeom,
+                                                   Geometry* debugQueryGeom,
+                                                   QueryGeometryState state )
+{
+    if (!queryGeom || !debugQueryGeom)
+    {
+        OSG_FATAL << "osgOQ: OcclusionQueryNode: No QueryGeometry." << std::endl;
+        return;
+    }
+
+    OpenThreads::ScopedLock<OpenThreads::Mutex> lock( _computeBoundMutex )  ;
+
+    _queryGeometryState = state;
+
+    _queryGeode->removeDrawables(0, _queryGeode->getNumDrawables());
+    _queryGeode->addDrawable(queryGeom);
+
+    _debugGeode->removeDrawables(0, _debugGeode->getNumDrawables());
+    _debugGeode->addDrawable(debugQueryGeom);
+}
+
+
 void OcclusionQueryNode::releaseGLObjects( State* state ) const
 {
     if (_queryGeode.valid()) _queryGeode->releaseGLObjects(state);
@@ -787,14 +823,18 @@ void OcclusionQueryNode::discardDeletedQueryObjects( unsigned int contextID )
     QueryGeometry::discardDeletedQueryObjects( contextID );
 }
 
-osg::QueryGeometry* OcclusionQueryNode::getQueryGeometry()
+void OcclusionQueryNode::setQueryGeometry( QueryGeometry* geom )
 {
-    if (_queryGeode && _queryGeode->getDrawable( 0 ))
+    if (geom)
     {
-        QueryGeometry* qg = static_cast< QueryGeometry* >( _queryGeode->getDrawable( 0 ) );
+        setQueryGeometryInternal( geom, geom, USER_DEFINED );
-        return qg;
+    }
+    else
+    {
+        setQueryGeometryInternal( createDefaultQueryGeometry( getName() ),
+                                  createDefaultDebugQueryGeometry(),
+                                  INVALID);
     }
-    return 0;
 }
 
 const osg::QueryGeometry* OcclusionQueryNode::getQueryGeometry() const