From ddb72691bcc60ba34137369e61f93c56976c763b Mon Sep 17 00:00:00 2001
From: Daniel Trstenjak <daniel.trstenjak@gmail.com>
Date: Fri, 23 Aug 2019 09:46:02 +0200
Subject: [PATCH] OcclusionQueryNode: fix resetting to default query geometry

When the query geometry gets reset to its default then its
vertices have to be updated by the bounding box dimensions of
the current children of the OcclusionQueryNode.
---
 include/osg/OcclusionQueryNode |  2 +
 src/osg/OcclusionQueryNode.cpp | 79 ++++++++++++++++++++--------------
 2 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/include/osg/OcclusionQueryNode b/include/osg/OcclusionQueryNode
index 1bb87ff53..9d8ebabc3 100644
--- a/include/osg/OcclusionQueryNode
+++ b/include/osg/OcclusionQueryNode
@@ -208,6 +208,8 @@ protected:
                                    osg::Geometry* debugQueryGeom,
                                    QueryGeometryState state );
 
+    void updateDefaultQueryGeometry();
+
     osg::ref_ptr< osg::Geode > _queryGeode;
     osg::ref_ptr< osg::Geode > _debugGeode;
 
diff --git a/src/osg/OcclusionQueryNode.cpp b/src/osg/OcclusionQueryNode.cpp
index b3cdee6ba..6fd2a3123 100644
--- a/src/osg/OcclusionQueryNode.cpp
+++ b/src/osg/OcclusionQueryNode.cpp
@@ -625,39 +625,7 @@ BoundingSphere OcclusionQueryNode::computeBound() const
             // 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 );
-
-
-            ComputeBoundsVisitor cbv;
-            nonConstThis->accept( cbv );
-            BoundingBox bb = cbv.getBoundingBox();
-            const bool bbValid = bb.valid();
-            _queryGeometryState = bbValid ? VALID : INVALID;
-
-            osg::ref_ptr<Vec3Array> v = new Vec3Array;
-            v->resize( 8 );
-
-            // Having (0,0,0) as vertices for the case of the invalid query geometry
-            // still isn't quite the right thing. But the query geometry is public
-            // accessible and therefore a user might expect eight vertices, so
-            // it seems safer to keep eight vertices in the geometry.
-
-            if (bbValid)
-            {
-                (*v)[0] = Vec3( bb._min.x(), bb._min.y(), bb._min.z() );
-                (*v)[1] = Vec3( bb._max.x(), bb._min.y(), bb._min.z() );
-                (*v)[2] = Vec3( bb._max.x(), bb._min.y(), bb._max.z() );
-                (*v)[3] = Vec3( bb._min.x(), bb._min.y(), bb._max.z() );
-                (*v)[4] = Vec3( bb._max.x(), bb._max.y(), bb._min.z() );
-                (*v)[5] = Vec3( bb._min.x(), bb._max.y(), bb._min.z() );
-                (*v)[6] = Vec3( bb._min.x(), bb._max.y(), bb._max.z() );
-                (*v)[7] = Vec3( bb._max.x(), bb._max.y(), bb._max.z() );
-            }
-
-            Geometry* geom = static_cast< Geometry* >( nonConstThis->_queryGeode->getDrawable( 0 ) );
-            geom->setVertexArray( v.get() );
-
-            geom = static_cast< osg::Geometry* >( nonConstThis->_debugGeode->getDrawable( 0 ) );
-            geom->setVertexArray( v.get() );
+            nonConstThis->updateDefaultQueryGeometry();
         }
     }
 
@@ -803,6 +771,49 @@ void OcclusionQueryNode::setQueryGeometryInternal( QueryGeometry* queryGeom,
 }
 
 
+void OcclusionQueryNode::updateDefaultQueryGeometry()
+{
+    if (_queryGeometryState == USER_DEFINED)
+    {
+        OSG_FATAL << "osgOQ: OcclusionQueryNode: Unexpected QueryGeometryState=USER_DEFINED." << std::endl;
+        return;
+    }
+
+    ComputeBoundsVisitor cbv;
+    accept( cbv );
+
+    BoundingBox bb = cbv.getBoundingBox();
+    const bool bbValid = bb.valid();
+    _queryGeometryState = bbValid ? VALID : INVALID;
+
+    osg::ref_ptr<Vec3Array> v = new Vec3Array;
+    v->resize( 8 );
+
+    // Having (0,0,0) as vertices for the case of the invalid query geometry
+    // still isn't quite the right thing. But the query geometry is public
+    // accessible and therefore a user might expect eight vertices, so
+    // it seems safer to keep eight vertices in the geometry.
+
+    if (bbValid)
+    {
+        (*v)[0] = Vec3( bb._min.x(), bb._min.y(), bb._min.z() );
+        (*v)[1] = Vec3( bb._max.x(), bb._min.y(), bb._min.z() );
+        (*v)[2] = Vec3( bb._max.x(), bb._min.y(), bb._max.z() );
+        (*v)[3] = Vec3( bb._min.x(), bb._min.y(), bb._max.z() );
+        (*v)[4] = Vec3( bb._max.x(), bb._max.y(), bb._min.z() );
+        (*v)[5] = Vec3( bb._min.x(), bb._max.y(), bb._min.z() );
+        (*v)[6] = Vec3( bb._min.x(), bb._max.y(), bb._max.z() );
+        (*v)[7] = Vec3( bb._max.x(), bb._max.y(), bb._max.z() );
+    }
+
+    Geometry* geom = static_cast< Geometry* >( _queryGeode->getDrawable( 0 ) );
+    geom->setVertexArray( v.get() );
+
+    geom = static_cast< osg::Geometry* >( _debugGeode->getDrawable( 0 ) );
+    geom->setVertexArray( v.get() );
+}
+
+
 void OcclusionQueryNode::releaseGLObjects( State* state ) const
 {
     if (_queryGeode.valid()) _queryGeode->releaseGLObjects(state);
@@ -834,6 +845,8 @@ void OcclusionQueryNode::setQueryGeometry( QueryGeometry* geom )
         setQueryGeometryInternal( createDefaultQueryGeometry( getName() ),
                                   createDefaultDebugQueryGeometry(),
                                   INVALID);
+
+        updateDefaultQueryGeometry();
     }
 }