From 5d03bb9a2bc66e9cc2352c2505fb561ad7d42bbc Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Tue, 1 Mar 2016 10:37:41 +0000 Subject: [PATCH] From Jannik Heller, "This submission fixes a stuttering issue that may occur when occlusion query nodes are in view. The problem is that OSG retrieves the occlusion query result without first checking if it's available (GL_QUERY_RESULT_AVAILABLE). Thus, the driver has to sync with the GPU i.e. wait for all queued draw calls to complete. This is particularly bad in V-Synced situations where the driver may be using multi frame queueing techniques - coupled with the fact that OSG only runs an occlusion query every 5th frame, results in very unpleasant stuttering in some situations. The change I made is to check GL_QUERY_RESULT_AVAILABLE before retrieving the query, to ensure that there won't be a stall. If the query result is not available yet, we'll leave it alone and try again in the next frame. Had to make a few more changes than I'd liked, mostly because the TestResult mechanism wasn't designed for holding on to query objects for more than one frame. As well, I'm thinking that RetrieveQueriesCallback and ClearQueriesCallback could be merged together, if we wanted to go for more refactoring. For though now my strategy is to make as little changes as possible. Let me know what you think of the patch." --- include/osg/OcclusionQueryNode | 4 +- src/osg/OcclusionQueryNode.cpp | 107 ++++++++++++++++++--------------- 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/include/osg/OcclusionQueryNode b/include/osg/OcclusionQueryNode index 07285f809..2b68e4ce2 100644 --- a/include/osg/OcclusionQueryNode +++ b/include/osg/OcclusionQueryNode @@ -40,7 +40,7 @@ osg::StateSet* initOQDebugState(); class TestResult : public osg::Referenced { public: - TestResult() : _init( false ), _id( 0 ), _contextID( 0 ), _active( false ), _numPixels( 0 ) {} + TestResult() : _init( false ), _id( 0 ), _contextID( 0 ), _active( false ), _numPixels( 0 ) {setThreadSafeRefUnref(true);} ~TestResult() {} bool _init; @@ -81,7 +81,7 @@ public: static void discardDeletedQueryObjects( unsigned int contextID ); protected: - typedef std::map< const osg::Camera*, TestResult > ResultMap; + typedef std::map< const osg::Camera*, osg::ref_ptr > ResultMap; mutable ResultMap _results; mutable OpenThreads::Mutex _mapMutex; diff --git a/src/osg/OcclusionQueryNode.cpp b/src/osg/OcclusionQueryNode.cpp index d16cceedb..7cd653c15 100644 --- a/src/osg/OcclusionQueryNode.cpp +++ b/src/osg/OcclusionQueryNode.cpp @@ -98,7 +98,7 @@ StateSet* initOQDebugState() struct RetrieveQueriesCallback : public osg::Camera::DrawCallback { - typedef std::vector ResultsVector; + typedef std::vector > ResultsVector; ResultsVector _results; RetrieveQueriesCallback( osg::GLExtensions* ext=NULL ) @@ -144,7 +144,7 @@ struct RetrieveQueriesCallback : public osg::Camera::DrawCallback ResultsVector::const_iterator it = _results.begin(); while (it != _results.end()) { - osg::TestResult* tr = const_cast( *it ); + osg::TestResult* tr = const_cast( (*it).get() ); if (!tr->_active || !tr->_init) { @@ -160,31 +160,20 @@ struct RetrieveQueriesCallback : public osg::Camera::DrawCallback OSG_DEBUG << "osgOQ: RQCB: Retrieving..." << std::endl; -#ifdef FORCE_QUERY_RESULT_AVAILABLE_BEFORE_RETRIEVAL - - // Should not need to do this, but is required on some platforms to - // work aroung issues in the device driver. For example, without this - // code, we've seen crashes on 64-bit Mac/Linux NVIDIA systems doing - // multithreaded, multipipe rendering (as in a CAVE). - // Tried with ATI and verified this workaround is not needed; the - // problem is specific to NVIDIA. - GLint ready( 0 ); - while( !ready ) + GLint ready = 0; + ext->glGetQueryObjectiv( tr->_id, GL_QUERY_RESULT_AVAILABLE, &ready ); + if (ready) { - // Apparently, must actually sleep here to avoid issues w/ NVIDIA Quadro. - OpenThreads::Thread::microSleep( 5 ); - ext->glGetQueryObjectiv( tr->_id, GL_QUERY_RESULT_AVAILABLE, &ready ); - }; -#endif + ext->glGetQueryObjectiv( tr->_id, GL_QUERY_RESULT, &(tr->_numPixels) ); + if (tr->_numPixels < 0) + OSG_WARN << "osgOQ: RQCB: " << + "glGetQueryObjectiv returned negative value (" << tr->_numPixels << ")." << std::endl; - ext->glGetQueryObjectiv( tr->_id, GL_QUERY_RESULT, &(tr->_numPixels) ); - if (tr->_numPixels < 0) - OSG_WARN << "osgOQ: RQCB: " << - "glGetQueryObjectiv returned negative value (" << tr->_numPixels << ")." << std::endl; - - // Either retrieve last frame's results, or ignore it because the - // camera is inside the view. In either case, _active is now false. - tr->_active = false; + // Either retrieve last frame's results, or ignore it because the + // camera is inside the view. In either case, _active is now false. + tr->_active = false; + } + // else: query result not available yet, try again next frame it++; count++; @@ -197,7 +186,13 @@ struct RetrieveQueriesCallback : public osg::Camera::DrawCallback void reset() { - _results.clear(); + for (ResultsVector::iterator it = _results.begin(); it != _results.end();) + { + if (!(*it)->_active || !(*it)->_init) // remove results that have already been retrieved or their query objects deleted. + it = _results.erase(it); + else + ++it; + } } void add( osg::TestResult* tr ) @@ -269,9 +264,9 @@ QueryGeometry::reset() ResultMap::iterator it = _results.begin(); while (it != _results.end()) { - TestResult& tr = it->second; - if (tr._init) - QueryGeometry::deleteQueryObject( tr._contextID, tr._id ); + osg::ref_ptr tr = it->second; + if (tr->_init) + QueryGeometry::deleteQueryObject( tr->_contextID, tr->_id ); it++; } _results.clear(); @@ -303,10 +298,30 @@ QueryGeometry::drawImplementation( osg::RenderInfo& renderInfo ) const } // Get TestResult from Camera map - TestResult* tr; + osg::ref_ptr tr; { OpenThreads::ScopedLock lock( _mapMutex ); - tr = &( _results[ cam ] ); + tr = ( _results[ cam ] ); + if (!tr.valid()) + { + tr = new osg::TestResult; + _results[ cam ] = tr; + } + } + + + // Issue query + if (!tr->_init) + { + ext->glGenQueries( 1, &(tr->_id) ); + tr->_contextID = contextID; + tr->_init = true; + } + + if (tr->_active) + { + // last query hasn't been retrieved yet + return; } // Add TestResult to RQCB. @@ -319,15 +334,6 @@ QueryGeometry::drawImplementation( osg::RenderInfo& renderInfo ) const } rqcb->add( tr ); - - // Issue query - if (!tr->_init) - { - ext->glGenQueries( 1, &(tr->_id) ); - tr->_contextID = contextID; - tr->_init = true; - } - OSG_DEBUG << "osgOQ: QG: Querying for: " << _oqnName << std::endl; @@ -358,12 +364,17 @@ QueryGeometry::drawImplementation( osg::RenderInfo& renderInfo ) const unsigned int QueryGeometry::getNumPixels( const osg::Camera* cam ) { - TestResult tr; + osg::ref_ptr tr; { OpenThreads::ScopedLock lock( _mapMutex ); tr = _results[ cam ]; + if (!tr.valid()) + { + tr = new osg::TestResult; + _results[ cam ] = tr; + } } - return tr._numPixels; + return tr->_numPixels; } @@ -384,15 +395,11 @@ QueryGeometry::releaseGLObjects( osg::State* state ) const ResultMap::iterator it = _results.begin(); while (it != _results.end()) { - TestResult& tr = it->second; - if (tr._contextID == contextID) + osg::ref_ptr tr = it->second; + if (tr->_contextID == contextID) { -#if 1 - osg::get(contextID)->scheduleGLObjectForDeletion(tr._id ); -#else - QueryGeometry::deleteQueryObject( contextID, tr._id ); -#endif - tr._init = false; + osg::get(contextID)->scheduleGLObjectForDeletion(tr->_id ); + tr->_init = false; } it++; }