From e4d145b55f428f46130596ebce1172b103b52380 Mon Sep 17 00:00:00 2001 From: Daniel Dresser Date: Wed, 3 Jul 2024 17:33:44 -0700 Subject: [PATCH] ShaderTweaks : Support wildcards in shader name --- Changes.md | 1 + python/GafferSceneTest/ShaderTweaksTest.py | 83 +++++ src/GafferScene/ShaderTweaks.cpp | 394 ++++++++++++--------- 3 files changed, 303 insertions(+), 175 deletions(-) diff --git a/Changes.md b/Changes.md index 801379070a0..465f2584847 100644 --- a/Changes.md +++ b/Changes.md @@ -20,6 +20,7 @@ Improvements - LightEditor : Mute and solo columns now accurately reflect the presence of the `light:mute` attribute (for the Mute column) and membership in the `soloLights` set (for the Solo column) for all scene locations, not just for lights. - RenderPassEditor : The currently active render pass can now be unset by double clicking on its green dot in the "Active" column. - HierarchyView, LightEditor, RenderPassEditor, SetEditor : Reduced potential UI stalls when first showing a scene. +- ShaderTweaks : Added support for wildcards in the shader name, in order to tweak multiple shaders at once. Fixes ----- diff --git a/python/GafferSceneTest/ShaderTweaksTest.py b/python/GafferSceneTest/ShaderTweaksTest.py index 3d519acaeb1..28f62bdcc3d 100644 --- a/python/GafferSceneTest/ShaderTweaksTest.py +++ b/python/GafferSceneTest/ShaderTweaksTest.py @@ -466,5 +466,88 @@ def testRemove( self ) : ) self.assertNotIn( "intensity", tweaks["out"].attributes( "/light" )["light"].outputShader().parameters ) + def testWildcards( self ) : + + shaderA_A = GafferSceneTest.TestShader( "A_A" ) + shaderA_B = GafferSceneTest.TestShader( "A_B" ) + shaderA_B["parameters"]["c"].setInput( shaderA_A["out"] ) + shaderB_A = GafferSceneTest.TestShader( "B_A" ) + shaderB_A["parameters"]["c"].setInput( shaderA_B["out"] ) + + light = GafferSceneTest.TestLight() + light["parameters"]["intensity"].setInput( shaderB_A["out"] ) + + pathFilter = GafferScene.PathFilter() + pathFilter["paths"].setValue( IECore.StringVectorData( [ "/light" ] ) ) + + tweaks = GafferScene.ShaderTweaks() + tweaks["in"].setInput( light["out"] ) + tweaks["shader"].setValue( "light" ) + tweaks["filter"].setInput( pathFilter["out"] ) + + originalNetwork = tweaks["out"].attributes( "/light" )["light"] + + self.assertEqual( len( originalNetwork ), 4 ) + + tweaks["tweaks"].addChild( + Gaffer.TweakPlug( "*.i", 42, mode = Gaffer.TweakPlug.Mode.Add ) + ) + + tweaked = tweaks["out"].attributes( "/light" )["light"] + self.assertEqual( tweaked.shaders()["A_A"].parameters["i"].value, 42 ) + self.assertEqual( tweaked.shaders()["A_B"].parameters["i"].value, 42 ) + self.assertEqual( tweaked.shaders()["B_A"].parameters["i"].value, 42 ) + + tweaks["tweaks"][0]["name"].setValue( "A_A.*" ) + + # Wildcards are currently supported only in shader names, not in parameter names ( since we can't think of + # a legitimate use case for doing it for parameters. Do we need a more specific error though? Currently, a + # confusing case is trying *.*, which silently does nothing ( it looks at all shaders, and finds that none of + # them have a parameter with the literal name `*` ). + with self.assertRaisesRegex( RuntimeError, 'Cannot apply tweak with mode Add to "A_A.*" : This parameter does not exist' ) : + tweaks["out"].attributes( "/light" ) + + tweaks["tweaks"][0]["name"].setValue( "*_A.i" ) + + tweaked = tweaks["out"].attributes( "/light" )["light"] + self.assertEqual( tweaked.shaders()["A_A"].parameters["i"].value, 42 ) + self.assertEqual( tweaked.shaders()["A_B"].parameters["i"].value, 0 ) + self.assertEqual( tweaked.shaders()["B_A"].parameters["i"].value, 42 ) + + extraShader = GafferSceneTest.TestShader( "extra" ) + tweaks["tweaks"][0]["value"].setInput( extraShader["out"]["r"] ) + + with self.assertRaisesRegex( RuntimeError, r'Cannot apply tweak "\*_A.i" to "A_A.i" : Mode must be "Replace" when inserting a connection' ) : + tweaks["out"].attributes( "/light" ) + + tweaks["tweaks"][0]["mode"].setValue( Gaffer.TweakPlug.Mode.Replace ) + tweaked = tweaks["out"].attributes( "/light" )["light"] + + # It would perhaps be better behaviour if we inserted the new shader once, and linked it in each place it + # is used, but it is easier to just treat this the same as is you made two tweaks to "A_A.i" and "B_A.i": + # create two copies of the input network. Maybe in an ideal world, two separate tweaks using the same + # input network would also be shared? + self.assertEqual( len( tweaked ), 6 ) + self.assertEqual( tweaked.inputConnections("A_A")[-1].source, IECoreScene.ShaderNetwork.Parameter( "extra", "out.r" ) ) + self.assertEqual( tweaked.inputConnections("B_A")[-1].source, IECoreScene.ShaderNetwork.Parameter( "extra1", "out.r" ) ) + + + del tweaks["tweaks"][0] + + tweaks["tweaks"].addChild( + Gaffer.TweakPlug( "*.c", imath.Color3f( 1, 2, 3), mode = Gaffer.TweakPlug.Mode.Add ) + ) + + with self.assertRaisesRegex( RuntimeError, r'ShaderTweaks.out.attributes : Cannot apply tweak "\*.c" to "A_B.c" : Mode must be "Replace" when a previous connection exists' ) : + tweaks["out"].attributes( "/light" ) + + tweaks["tweaks"][0]["name"].setValue( "*.intensity" ) + tweaks["tweaks"][0]["mode"].setValue( Gaffer.TweakPlug.Mode.Replace ) + + # The shader with an intensity parameter is the last one in the chain, so this removes all upstream shaders + tweaked = tweaks["out"].attributes( "/light" )["light"] + self.assertEqual( len( tweaked ), 1 ) + self.assertEqual( tweaked.shaders()["__shader"].parameters["intensity"].value, imath.Color3f( 1, 2, 3 ) ) + if __name__ == "__main__": unittest.main() diff --git a/src/GafferScene/ShaderTweaks.cpp b/src/GafferScene/ShaderTweaks.cpp index 2865fc9ead3..b0e3ff19c79 100644 --- a/src/GafferScene/ShaderTweaks.cpp +++ b/src/GafferScene/ShaderTweaks.cpp @@ -178,6 +178,183 @@ void checkForCycle( const ShaderNetwork &network, const IECore::InternedString & } } +bool applyTweakInternal( ShaderNetwork *shaderNetwork, unordered_map &modifiedShaders, const TweakPlug *tweakPlug, const ShaderNetwork *inputNetwork, const std::string &tweakLabel, const ShaderNetwork::Parameter ¶meter, const IECoreScene::Shader *shader, TweakPlug::MissingMode missingMode, bool &removedConnections ) +{ + if( !shader ) + { + shader = shaderNetwork->getShader( parameter.shader ); + } + + if( !shader ) + { + if( missingMode != TweakPlug::MissingMode::Ignore ) + { + throw IECore::Exception( fmt::format( + "Cannot apply tweak \"{}\" because shader \"{}\" does not exist", + tweakLabel, parameter.shader.string() + ) ); + } + else + { + return false; + } + } + + const TweakPlug::Mode mode = static_cast( tweakPlug->modePlug()->getValue() ); + + ShaderNetwork::Parameter originalInput = shaderNetwork->input( parameter ); + if( originalInput ) + { + if( mode != TweakPlug::Mode::Replace ) + { + throw IECore::Exception( fmt::format( "Cannot apply tweak \"{}\" to \"{}.{}\" : Mode must be \"Replace\" when a previous connection exists", tweakLabel, parameter.shader.string(), parameter.name.string() ) ); + } + shaderNetwork->removeConnection( { originalInput, parameter } ); + removedConnections = true; + } + + if( inputNetwork ) + { + if( !inputNetwork->getOutput() ) + { + // Nothing to connect + return false; + } + + if( !shader->parametersData()->member( parameter.name ) ) + { + if( missingMode != TweakPlug::MissingMode::Ignore ) + { + throw IECore::Exception( fmt::format( + "Cannot apply tweak \"{}\" because shader \"{}\" does not have parameter \"{}\"", + tweakLabel, parameter.shader.string(), parameter.name.string() + ) ); + } + else + { + return false; + } + } + + if( mode != TweakPlug::Mode::Replace ) + { + throw IECore::Exception( fmt::format( "Cannot apply tweak \"{}\" to \"{}.{}\" : Mode must be \"Replace\" when inserting a connection", tweakLabel, parameter.shader.string(), parameter.name.string() ) ); + } + + const auto inputParameter = ShaderNetworkAlgo::addShaders( shaderNetwork, inputNetwork ); + shaderNetwork->addConnection( { inputParameter, parameter } ); + + static IECore::InternedString hasProxyNodesIdentifier( "__hasProxyNodes" ); + + const BoolData* hasProxyNodes = inputNetwork->blindData()->member( hasProxyNodesIdentifier ); + if( hasProxyNodes && hasProxyNodes->readable() ) + { + // It would be more efficient to search for and process tweak sources just in + // `inputNetwork` before merging it to `shaderNetwork` ... but this would require + // dealing with weird connections where the input node handle is relative to `shaderNetwork`, + // but the output handle is relative to `inputNetwork`. This can't currenty be done if there + // are nodes in the two networks with the same name, which get uniquified during addShaders. + // This could be solved with an optional output unordered_map + // from addShaders(). For the moment, however, Doing this after merging simplifies all that. + + // If we need to check for cycles, we will need to populate a set of dependent shaders. + // We cache it in case there are multiple proxies connected to the same tweak. + std::unordered_set dependentShadersCache; + + for( const auto &i : shaderNetwork->shaders() ) + { + if( !ShaderTweakProxy::isProxy( i.second.get() ) ) + { + continue; + } + + const StringData* targetShaderData = + i.second->parametersData()->member( "targetShader" ); + if( !targetShaderData ) + { + throw IECore::Exception( "Cannot find target shader parameter on ShaderTweakProxy" ); + } + const std::string &sourceShader = targetShaderData->readable(); + + ShaderNetwork::ConnectionRange range = shaderNetwork->outputConnections( i.first ); + const std::vector outputConnections( range.begin(), range.end() ); + + + for( const auto &c : outputConnections ) + { + + shaderNetwork->removeConnection( c ); + removedConnections = true; + + if( sourceShader == "" ) + { + if( originalInput ) + { + shaderNetwork->addConnection( { originalInput, c.destination } ); + } + else + { + const IECoreScene::Shader *proxyConnectedShader = shaderNetwork->getShader( c.destination.shader ); + if( !proxyConnectedShader ) + { + throw IECore::Exception( fmt::format( "ShaderTweakProxy connected to non-existent shader \"{}\"", c.destination.shader.string() ) ); + } + + // Regular tweak + auto modifiedShader = modifiedShaders.insert( { c.destination.shader, nullptr } ); + if( modifiedShader.second ) + { + modifiedShader.first->second = proxyConnectedShader->copy(); + } + + const IECore::Data *origDestParameter = modifiedShader.first->second->parametersData()->member(c.destination.name, /* throwExceptions = */ true ); + modifiedShader.first->second->parameters()[c.destination.name] = castDataToType( shader->parametersData()->member( parameter.name, /* throwExceptions = */ true ), origDestParameter ); + } + } + else + { + checkForCycle( *shaderNetwork, parameter.shader, dependentShadersCache, sourceShader ); + shaderNetwork->addConnection( { { sourceShader, c.source.name }, c.destination } ); + } + } + } + } + + return true; + } + else + { + // Regular tweak + auto modifiedShader = modifiedShaders.insert( { parameter.shader, nullptr } ); + if( modifiedShader.second ) + { + modifiedShader.first->second = shader->copy(); + } + + return tweakPlug->applyTweak( + [¶meter, &modifiedShader]( const std::string &valueName ) + { + return modifiedShader.first->second->parametersData()->member( parameter.name ); + }, + [¶meter, &modifiedShader]( const std::string &valueName, DataPtr newData ) + { + if( newData ) + { + modifiedShader.first->second->parameters()[parameter.name] = newData; + return true; + } + else + { + return static_cast( + modifiedShader.first->second->parameters().erase( parameter.name ) + ); + } + }, + missingMode + ); + } +} + } // namespace GAFFER_NODE_DEFINE_TYPE( ShaderTweaks ); @@ -356,200 +533,67 @@ bool ShaderTweaks::applyTweaks( IECoreScene::ShaderNetwork *shaderNetwork, Tweak continue; } - ShaderNetwork::Parameter parameter; - size_t dotPos = name.find_last_of( '.' ); - if( dotPos == string::npos ) - { - parameter.shader = shaderNetwork->getOutput().shader; - parameter.name = name; - } - else - { - parameter.shader = InternedString( name.c_str(), dotPos ); - parameter.name = InternedString( name.c_str() + dotPos + 1 ); - } - - const IECoreScene::Shader *shader = shaderNetwork->getShader( parameter.shader ); - if( !shader ) - { - if( missingMode != TweakPlug::MissingMode::Ignore ) - { - throw IECore::Exception( fmt::format( - "Cannot apply tweak \"{}\" because shader \"{}\" does not exist", - name, parameter.shader.string() - ) ); - } - else - { - continue; - } - } - - const TweakPlug::Mode mode = static_cast( tweakPlug->modePlug()->getValue() ); - - ShaderNetwork::Parameter originalInput = shaderNetwork->input( parameter ); - if( originalInput ) - { - if( mode != TweakPlug::Mode::Replace ) - { - throw IECore::Exception( fmt::format( "Cannot apply tweak to \"{}\" : Mode must be \"Replace\" when a previous connection exists", name ) ); - } - shaderNetwork->removeConnection( { originalInput, parameter } ); - removedConnections = true; - } - const auto shaderOutput = ::shaderOutput( tweakPlug.get() ); + ConstCompoundObjectPtr shaderAttributes; + const ShaderNetwork *inputNetwork = nullptr; if( shaderOutput.first ) { - if( !shader->parametersData()->member( parameter.name ) ) - { - if( missingMode != TweakPlug::MissingMode::Ignore ) - { - throw IECore::Exception( fmt::format( - "Cannot apply tweak \"{}\" because shader \"{}\" does not have parameter \"{}\"", - name, parameter.shader.string(), parameter.name.string() - ) ); - } - else - { - continue; - } - } - // New connection - ConstCompoundObjectPtr shaderAttributes = shaderOutput.first->attributes( shaderOutput.second ); - const ShaderNetwork *inputNetwork = nullptr; + shaderAttributes = shaderOutput.first->attributes( shaderOutput.second ); for( const auto &a : shaderAttributes->members() ) { - if( ( inputNetwork = runTimeCast( a.second.get() ) ) ) + inputNetwork = runTimeCast( a.second.get() ); + if( inputNetwork ) { break; } } - if( inputNetwork && inputNetwork->getOutput() ) + if( !inputNetwork ) { - if( mode != TweakPlug::Mode::Replace ) - { - throw IECore::Exception( fmt::format( "Cannot apply tweak to \"{}\" : Mode must be \"Replace\" when inserting a connection", name ) ); - } - - const auto inputParameter = ShaderNetworkAlgo::addShaders( shaderNetwork, inputNetwork ); - shaderNetwork->addConnection( { inputParameter, parameter } ); - - static IECore::InternedString hasProxyNodesIdentifier( "__hasProxyNodes" ); - - const BoolData* hasProxyNodes = inputNetwork->blindData()->member( hasProxyNodesIdentifier ); - if( hasProxyNodes && hasProxyNodes->readable() ) - { - // It would be more efficient to search for and process tweak sources just in - // `inputNetwork` before merging it to `shaderNetwork` ... but this would require - // dealing with weird connections where the input node handle is relative to `shaderNetwork`, - // but the output handle is relative to `inputNetwork`. This can't currenty be done if there - // are nodes in the two networks with the same name, which get uniquified during addShaders. - // This could be solved with an optional output unordered_map - // from addShaders(). For the moment, however, Doing this after merging simplifies all that. - - // If we need to check for cycles, we will need to populate a set of dependent shaders. - // We cache it in case there are multiple proxies connected to the same tweak. - std::unordered_set dependentShadersCache; - - for( const auto &i : shaderNetwork->shaders() ) - { - if( !ShaderTweakProxy::isProxy( i.second.get() ) ) - { - continue; - } - - const StringData* targetShaderData = - i.second->parametersData()->member( "targetShader" ); - if( !targetShaderData ) - { - throw IECore::Exception( "Cannot find target shader parameter on ShaderTweakProxy" ); - } - const std::string &sourceShader = targetShaderData->readable(); - - ShaderNetwork::ConnectionRange range = shaderNetwork->outputConnections( i.first ); - const std::vector outputConnections( range.begin(), range.end() ); - - - for( const auto &c : outputConnections ) - { - - shaderNetwork->removeConnection( c ); - removedConnections = true; + // Handle a weird corner case ... if none of the attributes output by Shader::attributes actually + // contain a shader, then the consistent behaviour is to not assign anything ( same as we would + // for empty shader networks ). I'm not sure if there's any situation where this branch would actually + // trigger. + static ConstShaderNetworkPtr emptyShaderNetwork = new ShaderNetwork(); + inputNetwork = emptyShaderNetwork.get(); + } - if( sourceShader == "" ) - { - if( originalInput ) - { - shaderNetwork->addConnection( { originalInput, c.destination } ); - } - else - { - const IECoreScene::Shader *proxyConnectedShader = shaderNetwork->getShader( c.destination.shader ); - if( !proxyConnectedShader ) - { - throw IECore::Exception( fmt::format( "ShaderTweakProxy connected to non-existent shader \"{}\"", c.destination.shader.string() ) ); - } - - // Regular tweak - auto modifiedShader = modifiedShaders.insert( { c.destination.shader, nullptr } ); - if( modifiedShader.second ) - { - modifiedShader.first->second = proxyConnectedShader->copy(); - } - - const IECore::Data *origDestParameter = modifiedShader.first->second->parametersData()->member(c.destination.name, /* throwExceptions = */ true ); - modifiedShader.first->second->parameters()[c.destination.name] = castDataToType( shader->parametersData()->member( parameter.name, /* throwExceptions = */ true ), origDestParameter ); - } - } - else - { - checkForCycle( *shaderNetwork, parameter.shader, dependentShadersCache, sourceShader ); - shaderNetwork->addConnection( { { sourceShader, c.source.name }, c.destination } ); - } - } - } - } + } - appliedTweaks = true; - } + ShaderNetwork::Parameter parameter; + const size_t dotPos = name.find_last_of( '.' ); + if( dotPos == string::npos ) + { + parameter.shader = shaderNetwork->getOutput().shader; + parameter.name = name; } else { - // Regular tweak - auto modifiedShader = modifiedShaders.insert( { parameter.shader, nullptr } ); - if( modifiedShader.second ) - { - modifiedShader.first->second = shader->copy(); - } + parameter.shader = InternedString( name.c_str(), dotPos ); + parameter.name = InternedString( name.c_str() + dotPos + 1 ); + } - if( - tweakPlug->applyTweak( - [¶meter, &modifiedShader]( const std::string &valueName ) - { - return modifiedShader.first->second->parametersData()->member( parameter.name ); - }, - [¶meter, &modifiedShader]( const std::string &valueName, DataPtr newData ) - { - if( newData ) - { - modifiedShader.first->second->parameters()[parameter.name] = newData; - return true; - } - else - { - return static_cast( - modifiedShader.first->second->parameters().erase( parameter.name ) - ); - } - }, - missingMode - ) - ) + if( !IECore::StringAlgo::hasWildcards( parameter.shader.string() ) ) + { + appliedTweaks |= applyTweakInternal( + shaderNetwork, modifiedShaders, tweakPlug.get(), inputNetwork, + name, parameter, nullptr, + missingMode, removedConnections + ); + } + else + { + for( const auto s : shaderNetwork->shaders() ) { - appliedTweaks = true; + if( StringAlgo::match( s.first, parameter.shader ) ) + { + appliedTweaks |= applyTweakInternal( + shaderNetwork, modifiedShaders, tweakPlug.get(), inputNetwork, + name, { s.first, parameter.name }, s.second.get(), + TweakPlug::MissingMode::Ignore, removedConnections + ); + } } } }