From 79f7727a5919b239687ecffd297778c09a947c9f Mon Sep 17 00:00:00 2001 From: Daniel Trstenjak Date: Fri, 23 Aug 2019 09:46:02 +0200 Subject: [PATCH 1/2] 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 fff343cd8..9cdc0e64f 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( this ); - - - ComputeBoundsVisitor cbv; - nonConstThis->accept( cbv ); - BoundingBox bb = cbv.getBoundingBox(); - const bool bbValid = bb.valid(); - _queryGeometryState = bbValid ? VALID : INVALID; - - osg::ref_ptr 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 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(); } } From 718383cac006d086b0a4116f0130c6d1e91330dc Mon Sep 17 00:00:00 2001 From: Daniel Trstenjak Date: Fri, 23 Aug 2019 09:59:54 +0200 Subject: [PATCH 2/2] OcclusionQueryNode: make all usages of 'updateDefaultQueryGeometry' thread safe --- src/osg/OcclusionQueryNode.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/osg/OcclusionQueryNode.cpp b/src/osg/OcclusionQueryNode.cpp index 9cdc0e64f..02509788d 100644 --- a/src/osg/OcclusionQueryNode.cpp +++ b/src/osg/OcclusionQueryNode.cpp @@ -759,8 +759,6 @@ void OcclusionQueryNode::setQueryGeometryInternal( QueryGeometry* queryGeom, return; } - OpenThreads::ScopedLock lock( _computeBoundMutex ) ; - _queryGeometryState = state; _queryGeode->removeDrawables(0, _queryGeode->getNumDrawables()); @@ -836,6 +834,8 @@ void OcclusionQueryNode::discardDeletedQueryObjects( unsigned int contextID ) void OcclusionQueryNode::setQueryGeometry( QueryGeometry* geom ) { + OpenThreads::ScopedLock lock( _computeBoundMutex ) ; + if (geom) { setQueryGeometryInternal( geom, geom, USER_DEFINED );