From 98b2d5d921a4f15f15921f4d6893b5d27685d642 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 19 Aug 2020 19:29:19 +0100 Subject: [PATCH 1/4] Make shadow debug HUD thread-safe * Double buffer the frustum uniforms. * Don't mess with the debug geometry's StateSet. * Change two-element vectors to arrays so the size is explicit. --- components/sceneutil/mwshadowtechnique.cpp | 20 +++++++++++--------- components/sceneutil/mwshadowtechnique.hpp | 5 +++-- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/components/sceneutil/mwshadowtechnique.cpp b/components/sceneutil/mwshadowtechnique.cpp index fa38da54e..24ec190ba 100644 --- a/components/sceneutil/mwshadowtechnique.cpp +++ b/components/sceneutil/mwshadowtechnique.cpp @@ -3104,12 +3104,12 @@ SceneUtil::MWShadowTechnique::DebugHUD::DebugHUD(int numberOfShadowMapsPerLight) fragmentShader = new osg::Shader(osg::Shader::FRAGMENT, debugFrustumFragmentShaderSource); frustumProgram->addShader(fragmentShader); - for (int i = 0; i < 2; ++i) + for (auto& frustumGeometry : mFrustumGeometries) { - mFrustumGeometries.emplace_back(new osg::Geometry()); - mFrustumGeometries[i]->setCullingActive(false); + frustumGeometry = new osg::Geometry(); + frustumGeometry->setCullingActive(false); - mFrustumGeometries[i]->getOrCreateStateSet()->setAttributeAndModes(frustumProgram, osg::StateAttribute::ON); + frustumGeometry->getOrCreateStateSet()->setAttributeAndModes(frustumProgram, osg::StateAttribute::ON); } osg::ref_ptr frustumDrawElements = new osg::DrawElementsUShort(osg::PrimitiveSet::LINE_STRIP); @@ -3145,12 +3145,14 @@ void SceneUtil::MWShadowTechnique::DebugHUD::draw(osg::ref_ptr t // It might be possible to change shadow settings at runtime if (shadowMapNumber > mDebugCameras.size()) addAnotherShadowMap(); - - mFrustumUniforms[shadowMapNumber]->set(matrix); - osg::ref_ptr stateSet = mDebugGeometry[shadowMapNumber]->getOrCreateStateSet(); + osg::ref_ptr stateSet = new osg::StateSet(); stateSet->setTextureAttributeAndModes(sDebugTextureUnit, texture, osg::StateAttribute::ON); + auto frustumUniform = mFrustumUniforms[cv.getTraversalNumber() % 2][shadowMapNumber]; + frustumUniform->set(matrix); + stateSet->addUniform(frustumUniform); + // Some of these calls may be superfluous. unsigned int traversalMask = cv.getTraversalMask(); cv.setTraversalMask(mDebugGeometry[shadowMapNumber]->getNodeMask()); @@ -3205,6 +3207,6 @@ void SceneUtil::MWShadowTechnique::DebugHUD::addAnotherShadowMap() mFrustumTransforms[shadowMapNumber]->setCullingActive(false); mDebugCameras[shadowMapNumber]->addChild(mFrustumTransforms[shadowMapNumber]); - mFrustumUniforms.push_back(new osg::Uniform(osg::Uniform::FLOAT_MAT4, "transform")); - mFrustumTransforms[shadowMapNumber]->getOrCreateStateSet()->addUniform(mFrustumUniforms[shadowMapNumber]); + for(auto& uniformVector : mFrustumUniforms) + uniformVector.push_back(new osg::Uniform(osg::Uniform::FLOAT_MAT4, "transform")); } diff --git a/components/sceneutil/mwshadowtechnique.hpp b/components/sceneutil/mwshadowtechnique.hpp index 77d0bd2b2..2078fc2d6 100644 --- a/components/sceneutil/mwshadowtechnique.hpp +++ b/components/sceneutil/mwshadowtechnique.hpp @@ -19,6 +19,7 @@ #ifndef COMPONENTS_SCENEUTIL_MWSHADOWTECHNIQUE_H #define COMPONENTS_SCENEUTIL_MWSHADOWTECHNIQUE_H 1 +#include #include #include @@ -282,8 +283,8 @@ namespace SceneUtil { osg::ref_ptr mDebugProgram; std::vector> mDebugGeometry; std::vector> mFrustumTransforms; - std::vector> mFrustumUniforms; - std::vector> mFrustumGeometries; + std::array>, 2> mFrustumUniforms; + std::array, 2> mFrustumGeometries; }; osg::ref_ptr _debugHud; From ce98d7053bc28913551f9188f575c432cb12ece8 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 19 Aug 2020 22:55:41 +0100 Subject: [PATCH 2/4] Double buffer view-dependent data stateset --- components/sceneutil/mwshadowtechnique.cpp | 17 +++++++++-------- components/sceneutil/mwshadowtechnique.hpp | 6 +++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/components/sceneutil/mwshadowtechnique.cpp b/components/sceneutil/mwshadowtechnique.cpp index 24ec190ba..ae3a29295 100644 --- a/components/sceneutil/mwshadowtechnique.cpp +++ b/components/sceneutil/mwshadowtechnique.cpp @@ -749,7 +749,8 @@ MWShadowTechnique::ViewDependentData::ViewDependentData(MWShadowTechnique* vdsm) _viewDependentShadowMap(vdsm) { OSG_INFO<<"ViewDependentData::ViewDependentData()"<0) { - decoratorStateGraph->setStateSet(selectStateSetForRenderingShadow(*vdd)); + decoratorStateGraph->setStateSet(selectStateSetForRenderingShadow(*vdd, cv.getTraversalNumber())); } // OSG_NOTICE<<"End of shadow setup Projection matrix "<<*cv.getProjectionMatrix()< stateset = vdd.getStateSet(); + osg::ref_ptr stateset = vdd.getStateSet(traversalNumber); std::lock_guard lock(_accessUniformsAndProgramMutex); - vdd.getStateSet()->clear(); + stateset->clear(); - vdd.getStateSet()->setTextureAttributeAndModes(0, _fallbackBaseTexture.get(), osg::StateAttribute::ON); + stateset->setTextureAttributeAndModes(0, _fallbackBaseTexture.get(), osg::StateAttribute::ON); for(Uniforms::const_iterator itr=_uniforms.begin(); itr!=_uniforms.end(); @@ -3047,7 +3048,7 @@ osg::StateSet* MWShadowTechnique::selectStateSetForRenderingShadow(ViewDependent stateset->setTextureMode(sd._textureUnit,GL_TEXTURE_GEN_Q,osg::StateAttribute::ON); } - return vdd.getStateSet(); + return stateset; } void MWShadowTechnique::resizeGLObjectBuffers(unsigned int /*maxSize*/) diff --git a/components/sceneutil/mwshadowtechnique.hpp b/components/sceneutil/mwshadowtechnique.hpp index 2078fc2d6..95f1117c0 100644 --- a/components/sceneutil/mwshadowtechnique.hpp +++ b/components/sceneutil/mwshadowtechnique.hpp @@ -192,7 +192,7 @@ namespace SceneUtil { ShadowDataList& getShadowDataList() { return _shadowDataList; } - osg::StateSet* getStateSet() { return _stateset.get(); } + osg::StateSet* getStateSet(unsigned int traversalNumber) { return _stateset[traversalNumber % 2].get(); } virtual void releaseGLObjects(osg::State* = 0) const; @@ -201,7 +201,7 @@ namespace SceneUtil { MWShadowTechnique* _viewDependentShadowMap; - osg::ref_ptr _stateset; + std::array, 2> _stateset; LightDataList _lightDataList; ShadowDataList _shadowDataList; @@ -231,7 +231,7 @@ namespace SceneUtil { virtual void cullShadowCastingScene(osgUtil::CullVisitor* cv, osg::Camera* camera) const; - virtual osg::StateSet* selectStateSetForRenderingShadow(ViewDependentData& vdd) const; + virtual osg::StateSet* selectStateSetForRenderingShadow(ViewDependentData& vdd, unsigned int traversalNumber) const; protected: virtual ~MWShadowTechnique(); From 707204133d1f7c0932734ef893f289b52aa07f5a Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Thu, 20 Aug 2020 00:38:13 +0100 Subject: [PATCH 3/4] Double-buffer shadow uniforms that change each frame --- components/sceneutil/mwshadowtechnique.cpp | 41 +++++++++++----------- components/sceneutil/mwshadowtechnique.hpp | 3 +- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/components/sceneutil/mwshadowtechnique.cpp b/components/sceneutil/mwshadowtechnique.cpp index ae3a29295..7a435fe90 100644 --- a/components/sceneutil/mwshadowtechnique.cpp +++ b/components/sceneutil/mwshadowtechnique.cpp @@ -1344,9 +1344,7 @@ void MWShadowTechnique::cull(osgUtil::CullVisitor& cv) std::string validRegionUniformName = "validRegionMatrix" + std::to_string(sm_i); osg::ref_ptr validRegionUniform; - std::lock_guard lock(_accessUniformsAndProgramMutex); - - for (auto uniform : _uniforms) + for (auto uniform : _uniforms[cv.getTraversalNumber() % 2]) { if (uniform->getName() == validRegionUniformName) validRegionUniform = uniform; @@ -1355,7 +1353,7 @@ void MWShadowTechnique::cull(osgUtil::CullVisitor& cv) if (!validRegionUniform) { validRegionUniform = new osg::Uniform(osg::Uniform::FLOAT_MAT4, validRegionUniformName); - _uniforms.push_back(validRegionUniform); + _uniforms[cv.getTraversalNumber() % 2].push_back(validRegionUniform); } validRegionUniform->set(validRegionMatrix); @@ -1468,8 +1466,6 @@ void MWShadowTechnique::createShaders() unsigned int _baseTextureUnit = 0; - std::lock_guard lock(_accessUniformsAndProgramMutex); - _shadowCastingStateSet = new osg::StateSet; ShadowSettings* settings = getShadowedScene()->getShadowSettings(); @@ -1502,15 +1498,20 @@ void MWShadowTechnique::createShaders() _shadowCastingStateSet->setMode(GL_POLYGON_OFFSET_FILL, osg::StateAttribute::ON | osg::StateAttribute::OVERRIDE); - _uniforms.clear(); osg::ref_ptr baseTextureSampler = new osg::Uniform("baseTexture",(int)_baseTextureUnit); - _uniforms.push_back(baseTextureSampler.get()); - osg::ref_ptr baseTextureUnit = new osg::Uniform("baseTextureUnit",(int)_baseTextureUnit); - _uniforms.push_back(baseTextureUnit.get()); - _uniforms.push_back(new osg::Uniform("maximumShadowMapDistance", (float)settings->getMaximumShadowMapDistance())); - _uniforms.push_back(new osg::Uniform("shadowFadeStart", (float)_shadowFadeStart)); + osg::ref_ptr maxDistance = new osg::Uniform("maximumShadowMapDistance", (float)settings->getMaximumShadowMapDistance()); + osg::ref_ptr fadeStart = new osg::Uniform("shadowFadeStart", (float)_shadowFadeStart); + + for (auto& perFrameUniformList : _uniforms) + { + perFrameUniformList.clear(); + perFrameUniformList.push_back(baseTextureSampler); + perFrameUniformList.push_back(baseTextureUnit.get()); + perFrameUniformList.push_back(maxDistance); + perFrameUniformList.push_back(fadeStart); + } for(unsigned int sm_i=0; sm_igetNumShadowMapsPerLight(); ++sm_i) { @@ -1518,14 +1519,16 @@ void MWShadowTechnique::createShaders() std::stringstream sstr; sstr<<"shadowTexture"< shadowTextureSampler = new osg::Uniform(sstr.str().c_str(),(int)(settings->getBaseShadowTextureUnit()+sm_i)); - _uniforms.push_back(shadowTextureSampler.get()); + for (auto& perFrameUniformList : _uniforms) + perFrameUniformList.push_back(shadowTextureSampler.get()); } { std::stringstream sstr; sstr<<"shadowTextureUnit"< shadowTextureUnit = new osg::Uniform(sstr.str().c_str(),(int)(settings->getBaseShadowTextureUnit()+sm_i)); - _uniforms.push_back(shadowTextureUnit.get()); + for (auto& perFrameUniformList : _uniforms) + perFrameUniformList.push_back(shadowTextureUnit.get()); } } @@ -2981,18 +2984,14 @@ osg::StateSet* MWShadowTechnique::selectStateSetForRenderingShadow(ViewDependent osg::ref_ptr stateset = vdd.getStateSet(traversalNumber); - std::lock_guard lock(_accessUniformsAndProgramMutex); - stateset->clear(); stateset->setTextureAttributeAndModes(0, _fallbackBaseTexture.get(), osg::StateAttribute::ON); - for(Uniforms::const_iterator itr=_uniforms.begin(); - itr!=_uniforms.end(); - ++itr) + for(auto uniform : _uniforms[traversalNumber % 2]) { - OSG_INFO<<"addUniform("<<(*itr)->getName()<<")"<addUniform(itr->get()); + OSG_INFO<<"addUniform("<getName()<<")"<addUniform(uniform); } if (_program.valid()) diff --git a/components/sceneutil/mwshadowtechnique.hpp b/components/sceneutil/mwshadowtechnique.hpp index 95f1117c0..bdfb44e46 100644 --- a/components/sceneutil/mwshadowtechnique.hpp +++ b/components/sceneutil/mwshadowtechnique.hpp @@ -248,8 +248,7 @@ namespace SceneUtil { osg::ref_ptr _fallbackShadowMapTexture; typedef std::vector< osg::ref_ptr > Uniforms; - mutable std::mutex _accessUniformsAndProgramMutex; - Uniforms _uniforms; + std::array _uniforms; osg::ref_ptr _program; bool _enableShadows; From fd14dad7897e0c80836b33d2ed281bb4da0b551a Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Thu, 20 Aug 2020 03:01:43 +0100 Subject: [PATCH 4/4] const osg::ref_ptf reference should be faster than value as constructor and destructor are non-trivial I played around in GodBolt and got into an argument to determine this. The difference will be immeasurably small, but my curiosity has been satisfied. --- components/sceneutil/mwshadowtechnique.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/sceneutil/mwshadowtechnique.cpp b/components/sceneutil/mwshadowtechnique.cpp index 7a435fe90..dc22d4d80 100644 --- a/components/sceneutil/mwshadowtechnique.cpp +++ b/components/sceneutil/mwshadowtechnique.cpp @@ -2988,7 +2988,7 @@ osg::StateSet* MWShadowTechnique::selectStateSetForRenderingShadow(ViewDependent stateset->setTextureAttributeAndModes(0, _fallbackBaseTexture.get(), osg::StateAttribute::ON); - for(auto uniform : _uniforms[traversalNumber % 2]) + for(const auto& uniform : _uniforms[traversalNumber % 2]) { OSG_INFO<<"addUniform("<getName()<<")"<addUniform(uniform);