From aee1edaf9e8df35004a53e94c9d0037c2474a147 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 8 Mar 2023 00:15:49 +0000 Subject: [PATCH 1/2] Partially revert "Attach shaders to geometry that lacks a stateset if necessary" This reverts commit 6aef366fd335547004b49c474749aee7d5391e7f. --- components/shader/shadervisitor.cpp | 80 ++++++++++++++++++----------- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/components/shader/shadervisitor.cpp b/components/shader/shadervisitor.cpp index e904e4b75a..06f246e9bc 100644 --- a/components/shader/shadervisitor.cpp +++ b/components/shader/shadervisitor.cpp @@ -865,51 +865,73 @@ namespace Shader void ShaderVisitor::apply(osg::Geometry& geometry) { - pushRequirements(geometry); + bool needPop = (geometry.getStateSet() != nullptr); if (geometry.getStateSet()) // TODO: check if stateset affects shader permutation before pushing it + { + pushRequirements(geometry); applyStateSet(geometry.getStateSet(), geometry); + } - const ShaderRequirements& reqs = mRequirements.back(); - adjustGeometry(geometry, reqs); - createProgram(reqs); + if (!mRequirements.empty()) + { + const ShaderRequirements& reqs = mRequirements.back(); - popRequirements(); + adjustGeometry(geometry, reqs); + + createProgram(reqs); + } + else + ensureFFP(geometry); + + if (needPop) + popRequirements(); } void ShaderVisitor::apply(osg::Drawable& drawable) { - pushRequirements(drawable); + bool needPop = drawable.getStateSet(); - if (drawable.getStateSet()) - applyStateSet(drawable.getStateSet(), drawable); - - const ShaderRequirements& reqs = mRequirements.back(); - createProgram(reqs); - - if (auto rig = dynamic_cast(&drawable)) + if (needPop) { - osg::ref_ptr sourceGeometry = rig->getSourceGeometry(); - if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) - rig->setSourceGeometry(sourceGeometry); + pushRequirements(drawable); + + if (drawable.getStateSet()) + applyStateSet(drawable.getStateSet(), drawable); } - else if (auto morph = dynamic_cast(&drawable)) + + if (!mRequirements.empty()) { - osg::ref_ptr sourceGeometry = morph->getSourceGeometry(); - if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) - morph->setSourceGeometry(sourceGeometry); - } - else if (auto osgaRig = dynamic_cast(&drawable)) - { - osg::ref_ptr sourceOsgaRigGeometry = osgaRig->getSourceRigGeometry(); - osg::ref_ptr sourceGeometry = sourceOsgaRigGeometry->getSourceGeometry(); - if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) + const ShaderRequirements& reqs = mRequirements.back(); + createProgram(reqs); + + if (auto rig = dynamic_cast(&drawable)) { - sourceOsgaRigGeometry->setSourceGeometry(sourceGeometry); - osgaRig->setSourceRigGeometry(sourceOsgaRigGeometry); + osg::ref_ptr sourceGeometry = rig->getSourceGeometry(); + if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) + rig->setSourceGeometry(sourceGeometry); + } + else if (auto morph = dynamic_cast(&drawable)) + { + osg::ref_ptr sourceGeometry = morph->getSourceGeometry(); + if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) + morph->setSourceGeometry(sourceGeometry); + } + else if (auto osgaRig = dynamic_cast(&drawable)) + { + osg::ref_ptr sourceOsgaRigGeometry = osgaRig->getSourceRigGeometry(); + osg::ref_ptr sourceGeometry = sourceOsgaRigGeometry->getSourceGeometry(); + if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) + { + sourceOsgaRigGeometry->setSourceGeometry(sourceGeometry); + osgaRig->setSourceRigGeometry(sourceOsgaRigGeometry); + } } } + else + ensureFFP(drawable); - popRequirements(); + if (needPop) + popRequirements(); } void ShaderVisitor::setAllowedToModifyStateSets(bool allowed) From ccdb1bf6b7e5be014eeeed800e8b6acd1ef53aba Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 8 Mar 2023 00:28:48 +0000 Subject: [PATCH 2/2] Ensure shader requirements are pushed at least once for subgraph Shaders, if deemed necessary, get attached to the node mentioned by the top of the requirements stack. Previously an empty stack was incorrectly assumed to mean no shaders were required, but we found out that was wrong. We need to put shaders *somewhere*, and the root of the subgraph we're modifying should be the best place. --- components/shader/shadervisitor.cpp | 71 ++++++++++++++--------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/components/shader/shadervisitor.cpp b/components/shader/shadervisitor.cpp index 06f246e9bc..69e91f057a 100644 --- a/components/shader/shadervisitor.cpp +++ b/components/shader/shadervisitor.cpp @@ -212,15 +212,17 @@ namespace Shader void ShaderVisitor::apply(osg::Node& node) { - if (node.getStateSet()) + bool needPop = false; + if (node.getStateSet() || mRequirements.empty()) { + needPop = true; pushRequirements(node); - applyStateSet(node.getStateSet(), node); - traverse(node); - popRequirements(); + if (node.getStateSet()) + applyStateSet(node.getStateSet(), node); } - else - traverse(node); + traverse(node); + if (needPop) + popRequirements(); } osg::StateSet* getWritableStateSet(osg::Node& node) @@ -865,12 +867,12 @@ namespace Shader void ShaderVisitor::apply(osg::Geometry& geometry) { - bool needPop = (geometry.getStateSet() != nullptr); - if (geometry.getStateSet()) // TODO: check if stateset affects shader permutation before pushing it - { + bool needPop = geometry.getStateSet() || mRequirements.empty(); + if (needPop) pushRequirements(geometry); + + if (geometry.getStateSet()) // TODO: check if stateset affects shader permutation before pushing it applyStateSet(geometry.getStateSet(), geometry); - } if (!mRequirements.empty()) { @@ -889,7 +891,7 @@ namespace Shader void ShaderVisitor::apply(osg::Drawable& drawable) { - bool needPop = drawable.getStateSet(); + bool needPop = drawable.getStateSet() || mRequirements.empty(); if (needPop) { @@ -899,36 +901,31 @@ namespace Shader applyStateSet(drawable.getStateSet(), drawable); } - if (!mRequirements.empty()) - { - const ShaderRequirements& reqs = mRequirements.back(); - createProgram(reqs); + const ShaderRequirements& reqs = mRequirements.back(); + createProgram(reqs); - if (auto rig = dynamic_cast(&drawable)) + if (auto rig = dynamic_cast(&drawable)) + { + osg::ref_ptr sourceGeometry = rig->getSourceGeometry(); + if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) + rig->setSourceGeometry(sourceGeometry); + } + else if (auto morph = dynamic_cast(&drawable)) + { + osg::ref_ptr sourceGeometry = morph->getSourceGeometry(); + if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) + morph->setSourceGeometry(sourceGeometry); + } + else if (auto osgaRig = dynamic_cast(&drawable)) + { + osg::ref_ptr sourceOsgaRigGeometry = osgaRig->getSourceRigGeometry(); + osg::ref_ptr sourceGeometry = sourceOsgaRigGeometry->getSourceGeometry(); + if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) { - osg::ref_ptr sourceGeometry = rig->getSourceGeometry(); - if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) - rig->setSourceGeometry(sourceGeometry); - } - else if (auto morph = dynamic_cast(&drawable)) - { - osg::ref_ptr sourceGeometry = morph->getSourceGeometry(); - if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) - morph->setSourceGeometry(sourceGeometry); - } - else if (auto osgaRig = dynamic_cast(&drawable)) - { - osg::ref_ptr sourceOsgaRigGeometry = osgaRig->getSourceRigGeometry(); - osg::ref_ptr sourceGeometry = sourceOsgaRigGeometry->getSourceGeometry(); - if (sourceGeometry && adjustGeometry(*sourceGeometry, reqs)) - { - sourceOsgaRigGeometry->setSourceGeometry(sourceGeometry); - osgaRig->setSourceRigGeometry(sourceOsgaRigGeometry); - } + sourceOsgaRigGeometry->setSourceGeometry(sourceGeometry); + osgaRig->setSourceRigGeometry(sourceOsgaRigGeometry); } } - else - ensureFFP(drawable); if (needPop) popRequirements();