From 41d12cfdb81996a9bb744849a50d152953064915 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Thu, 5 Dec 2024 11:52:52 +0000 Subject: [PATCH 1/3] Inspector : Fix references to "None" rather than "Source" --- Changes.md | 1 + python/GafferSceneUITest/ParameterInspectorTest.py | 2 +- src/GafferSceneUI/Inspector.cpp | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Changes.md b/Changes.md index 97c2f43ddfc..f82f07c8d0f 100644 --- a/Changes.md +++ b/Changes.md @@ -27,6 +27,7 @@ Fixes - Widget : Fixed `event.sourceWidget` for DragDropEvents generated from a Qt native drag within the same Gaffer process. This will now reference the `GafferUI.Widget` that the Qt source widget belongs to, if any. - Catalogue : Fixed bug which "stole" drags that crossed the image listing but which were destined elsewhere, for instance a drag from the HierarchyView to a PathFilter in the GraphEditor. - GadgetWidget : Fixed signal handling bug in `setViewportGadget()`. This could cause the widget to attempt to redraw unnecessarily when the _old_ viewport requested a redraw. +- AttributeEditor, LightEditor, RenderPassEditor : Fixed warning message which referred to "None" rather than the "Source" scope. API --- diff --git a/python/GafferSceneUITest/ParameterInspectorTest.py b/python/GafferSceneUITest/ParameterInspectorTest.py index 7cece6205ce..7fcfdc75c37 100644 --- a/python/GafferSceneUITest/ParameterInspectorTest.py +++ b/python/GafferSceneUITest/ParameterInspectorTest.py @@ -502,7 +502,7 @@ def testDisableEdit( self ) : inspection = self.__inspect( s["editScope"]["out"], "/light", "exposure", s["editScope"] ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to None to disable." ) + self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to Source to disable." ) inspection = self.__inspect( s["editScope"]["out"], "/light", "exposure", None ) self.assertEqual( inspection.source(), s["light"]["parameters"]["exposure"] ) diff --git a/src/GafferSceneUI/Inspector.cpp b/src/GafferSceneUI/Inspector.cpp index ce4f4b39eef..710eeea455a 100644 --- a/src/GafferSceneUI/Inspector.cpp +++ b/src/GafferSceneUI/Inspector.cpp @@ -292,7 +292,7 @@ void Inspector::inspectHistoryWalk( const GafferScene::SceneAlgo::History *histo if( !result->m_editScope && node->ancestor() ) { // We don't allow editing if the user hasn't requested a specific scope - // (they have selected "None" from the Menu) and the upstream edit is + // (they have selected "Source" from the Menu) and the upstream edit is // inside _any_ EditScope. result->m_editFunction = fmt::format( "Source is in an EditScope. Change scope to {} to edit.", @@ -328,7 +328,7 @@ void Inspector::inspectHistoryWalk( const GafferScene::SceneAlgo::History *histo } else if( !node->ancestor() && result->m_editScope ) { - result->m_disableEditFunction = "Edit is not in the current edit scope. Change scope to None to disable."; + result->m_disableEditFunction = "Edit is not in the current edit scope. Change scope to Source to disable."; } } } From 8034c18ca31b034757902198b59e6e16f26dad99 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Thu, 5 Dec 2024 12:43:38 +0000 Subject: [PATCH 2/3] Inspector : Prefer `std::variant` to `boost::variant` --- include/GafferSceneUI/Private/Inspector.h | 6 +++--- src/GafferSceneUI/Inspector.cpp | 26 ++++++++++++----------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/include/GafferSceneUI/Private/Inspector.h b/include/GafferSceneUI/Private/Inspector.h index 04f4e9c2243..ea4ac85cdc6 100644 --- a/include/GafferSceneUI/Private/Inspector.h +++ b/include/GafferSceneUI/Private/Inspector.h @@ -51,10 +51,10 @@ #include "boost/multi_index/member.hpp" #include "boost/multi_index/random_access_index.hpp" #include "boost/multi_index_container.hpp" -#include "boost/variant.hpp" #include #include +#include namespace GafferSceneUIModule { @@ -171,7 +171,7 @@ class GAFFERSCENEUI_API Inspector : public IECore::RefCounted, public Gaffer::Si virtual Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const; using EditFunction = std::function; - using EditFunctionOrFailure = boost::variant; + using EditFunctionOrFailure = std::variant; /// Should be implemented to return a function that will acquire /// an edit from the EditScope at the specified point in the history. /// If this is not possible, should return an error explaining why @@ -184,7 +184,7 @@ class GAFFERSCENEUI_API Inspector : public IECore::RefCounted, public Gaffer::Si virtual EditFunctionOrFailure editFunction( Gaffer::EditScope *editScope, const GafferScene::SceneAlgo::History *history ) const; using DisableEditFunction = std::function; - using DisableEditFunctionOrFailure = boost::variant; + using DisableEditFunctionOrFailure = std::variant; /// Can be implemented to return a function that will disable an edit /// at the specified plug. If this is not possible, should return an /// error explaining why (this is typically due to `readOnly` metadata). diff --git a/src/GafferSceneUI/Inspector.cpp b/src/GafferSceneUI/Inspector.cpp index 710eeea455a..571313d3f77 100644 --- a/src/GafferSceneUI/Inspector.cpp +++ b/src/GafferSceneUI/Inspector.cpp @@ -732,14 +732,15 @@ const std::string &Inspector::Result::fallbackDescription() const bool Inspector::Result::editable() const { - return m_editFunction.which() == 0 && boost::get( m_editFunction ) != nullptr; + auto f = std::get_if( &m_editFunction ); + return f && *f; } std::string Inspector::Result::nonEditableReason() const { - if( m_editFunction.which() == 1 ) + if( auto s = std::get_if( &m_editFunction ) ) { - return boost::get( m_editFunction ); + return *s; } return ""; @@ -747,24 +748,25 @@ std::string Inspector::Result::nonEditableReason() const Gaffer::ValuePlugPtr Inspector::Result::acquireEdit( bool createIfNecessary ) const { - if( m_editFunction.which() == 0 ) + if( auto f = std::get_if( &m_editFunction ) ) { - return boost::get( m_editFunction )( createIfNecessary ); + return (*f)( createIfNecessary ); } - throw IECore::Exception( "Not editable : " + boost::get( m_editFunction ) ); + throw IECore::Exception( "Not editable : " + std::get( m_editFunction ) ); } bool Inspector::Result::canDisableEdit() const { - return m_disableEditFunction.which() == 0 && boost::get( m_disableEditFunction ) != nullptr; + auto f = std::get_if( &m_disableEditFunction ); + return f && *f; } std::string Inspector::Result::nonDisableableReason() const { - if( m_disableEditFunction.which() == 1 ) + if( auto s = std::get_if( &m_disableEditFunction ) ) { - return boost::get( m_disableEditFunction ); + return *s; } return ""; @@ -772,12 +774,12 @@ std::string Inspector::Result::nonDisableableReason() const void Inspector::Result::disableEdit() const { - if( m_disableEditFunction.which() == 0 ) + if( auto f = std::get_if( &m_disableEditFunction ) ) { - return boost::get( m_disableEditFunction )(); + return (*f)(); } - throw IECore::Exception( "Cannot disable edit : " + boost::get( m_disableEditFunction ) ); + throw IECore::Exception( "Cannot disable edit : " + std::get( m_disableEditFunction ) ); } std::string Inspector::Result::editWarning() const From a7c8103c4453144b12fbb3d14a98278cea64dc55 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Thu, 5 Dec 2024 15:43:24 +0000 Subject: [PATCH 3/3] Inspector : Make handling of downstream overrides consistent We were already allowing edits in a nominated EditScope even when there was a downstream override that would prevent you from seeing the result. This was communicated in the UI by an orange "overridden" background colour in the inspector cell, and a warning message in the editing popup window. But in "Source" mode we made no such allowance, meaning that you couldn't make an edit at source if it was overridden downstream, and the UI didn't even show you that it was overridden downstream. There was no background colour and a spurious message about the target not being in the history. We now treat "Source" mode the same as EditScope mode, allowing edits at source even when overridden downstream. Along with that the UI now also shows the overridden status in the background colour and shows the appropriate warning when editing. This commit bundles a couple of other things which might ideally have been separate commits, but which arose naturally while figuring out the implementation : - Consolidated some member data into a single `Result::m_editors` struct. This encourages everything to be initialised together, making the data flow a little clearer, and using `std::optional` to make it unambiguous whether we have initialised yet or not. - Adjusted the failure messages when an attempt is made to disabled a non-existent edit. Instead of directing the user to change edit scope and disabled a _different_ edit, we now just explain that there is no edit to disable. Fixes #6172. --- Changes.md | 5 +- include/GafferSceneUI/Private/Inspector.h | 11 +- .../AttributeInspectorTest.py | 6 +- .../GafferSceneUITest/OptionInspectorTest.py | 14 +- .../ParameterInspectorTest.py | 77 ++++- .../SetMembershipInspectorTest.py | 7 +- src/GafferSceneUI/Inspector.cpp | 276 +++++++++--------- 7 files changed, 242 insertions(+), 154 deletions(-) diff --git a/Changes.md b/Changes.md index f82f07c8d0f..efe9fdff1e3 100644 --- a/Changes.md +++ b/Changes.md @@ -27,7 +27,10 @@ Fixes - Widget : Fixed `event.sourceWidget` for DragDropEvents generated from a Qt native drag within the same Gaffer process. This will now reference the `GafferUI.Widget` that the Qt source widget belongs to, if any. - Catalogue : Fixed bug which "stole" drags that crossed the image listing but which were destined elsewhere, for instance a drag from the HierarchyView to a PathFilter in the GraphEditor. - GadgetWidget : Fixed signal handling bug in `setViewportGadget()`. This could cause the widget to attempt to redraw unnecessarily when the _old_ viewport requested a redraw. -- AttributeEditor, LightEditor, RenderPassEditor : Fixed warning message which referred to "None" rather than the "Source" scope. +- AttributeEditor, LightEditor, RenderPassEditor : + - Fixed bugs which prevented edits being made in "Source" scope when there was a downstream edit in an EditScope (#6172). + - Fixed warning messages when attempting to disable a non-existent edit. + - Fixed warning message which referred to "None" rather than the "Source" scope. API --- diff --git a/include/GafferSceneUI/Private/Inspector.h b/include/GafferSceneUI/Private/Inspector.h index ea4ac85cdc6..2d9bec0e81f 100644 --- a/include/GafferSceneUI/Private/Inspector.h +++ b/include/GafferSceneUI/Private/Inspector.h @@ -374,10 +374,15 @@ class GAFFERSCENEUI_API Inspector::Result : public IECore::RefCounted Gaffer::EditScopePtr m_editScope; bool m_editScopeInHistory; - EditFunctionOrFailure m_editFunction; - std::string m_editWarning; + struct Editors + { + /// \todo Rename to `acquireEditFunction`? + EditFunctionOrFailure editFunction; + std::string editWarning; + DisableEditFunctionOrFailure disableEditFunction; + }; - DisableEditFunctionOrFailure m_disableEditFunction; + std::optional m_editors; }; diff --git a/python/GafferSceneUITest/AttributeInspectorTest.py b/python/GafferSceneUITest/AttributeInspectorTest.py index 2c99d5868b8..aa8581150a4 100644 --- a/python/GafferSceneUITest/AttributeInspectorTest.py +++ b/python/GafferSceneUITest/AttributeInspectorTest.py @@ -912,8 +912,8 @@ def testDisableEdit( self ) : inspection = self.__inspect( s["editScope2"]["out"], "/group/light", "gl:visualiser:scale", s["editScope2"] ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to editScope1 to disable." ) - self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : Edit is not in the current edit scope. Change scope to editScope1 to disable.", inspection.disableEdit ) + self.assertEqual( inspection.nonDisableableReason(), "There is no edit in editScope2." ) + self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : There is no edit in editScope2.", inspection.disableEdit ) Gaffer.MetadataAlgo.setReadOnly( s["editScope1"], True ) inspection = self.__inspect( s["editScope1"]["out"], "/group/light", "gl:visualiser:scale", s["editScope1"] ) @@ -930,7 +930,7 @@ def testDisableEdit( self ) : inspection = self.__inspect( s["editScope1"]["out"], "/group/light", "gl:visualiser:scale", s["editScope1"] ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to None to disable." ) + self.assertEqual( inspection.nonDisableableReason(), "There is no edit in editScope1." ) inspection = self.__inspect( s["editScope1"]["out"], "/group/light", "gl:visualiser:scale", None ) self.assertFalse( inspection.canDisableEdit() ) diff --git a/python/GafferSceneUITest/OptionInspectorTest.py b/python/GafferSceneUITest/OptionInspectorTest.py index 118332b9741..56e791f90cb 100644 --- a/python/GafferSceneUITest/OptionInspectorTest.py +++ b/python/GafferSceneUITest/OptionInspectorTest.py @@ -652,13 +652,15 @@ def testRenderPassSourceAndEdits( self ) : ) # When using no scope, make sure that we don't inadvertently edit the contents of an EditScope. + # We should be allowed to edit the source before the scope, but only with a warning about there + # being a downstream override. self.__assertExpectedResult( self.__inspect( s["editScope2"]["out"], "render:camera", None, context ), source = s["editScope1"]["standardOptions3"]["options"]["renderCamera"], - sourceType = SourceType.Other, - editable = False, - nonEditableReason = "Source is in an EditScope. Change scope to editScope1 to edit." + sourceType = SourceType.Downstream, + editable = True, + editWarning = "Option has edits downstream in editScope1." ) # If there is a StandardOptions node outside of an edit scope, make sure we use that with no scope @@ -950,7 +952,7 @@ def testDisableEdit( self ) : inspection = self.__inspect( s["editScope2"]["out"], "render:camera", s["editScope2"] ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to None to disable." ) + self.assertEqual( inspection.nonDisableableReason(), "There is no edit in editScope2." ) Gaffer.MetadataAlgo.setReadOnly( s["standardOptions"]["options"], True ) inspection = self.__inspect( s["group"]["out"], "render:camera", None ) @@ -986,8 +988,8 @@ def testDisableEdit( self ) : inspection = self.__inspect( s["editScope2"]["out"], "render:camera", s["editScope2"] ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to editScope1 to disable." ) - self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : Edit is not in the current edit scope. Change scope to editScope1 to disable.", inspection.disableEdit ) + self.assertEqual( inspection.nonDisableableReason(), "There is no edit in editScope2." ) + self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : There is no edit in editScope2.", inspection.disableEdit ) Gaffer.MetadataAlgo.setReadOnly( s["editScope1"], True ) inspection = self.__inspect( s["editScope1"]["out"], "render:camera", s["editScope1"] ) diff --git a/python/GafferSceneUITest/ParameterInspectorTest.py b/python/GafferSceneUITest/ParameterInspectorTest.py index 7fcfdc75c37..25c87209192 100644 --- a/python/GafferSceneUITest/ParameterInspectorTest.py +++ b/python/GafferSceneUITest/ParameterInspectorTest.py @@ -478,14 +478,15 @@ def testDisableEdit( self ) : self.assertEqual( inspection.nonDisableableReason(), "The target edit scope editScope2 is not in the scene history." ) inspection = self.__inspect( s["editScope2"]["out"], "/light", "exposure", None ) + self.assertTrue( inspection.acquireEdit( False ).isSame( s["light"]["parameters"]["exposure"] ) ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Source is in an EditScope. Change scope to editScope to disable." ) - self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : Source is in an EditScope. Change scope to editScope to disable.", inspection.disableEdit ) + self.assertEqual( inspection.nonDisableableReason(), "Disabling edits not supported for this plug." ) + self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : Disabling edits not supported for this plug.", inspection.disableEdit ) inspection = self.__inspect( s["editScope2"]["out"], "/light", "exposure", s["editScope2"] ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to editScope to disable." ) - self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : Edit is not in the current edit scope. Change scope to editScope to disable.", inspection.disableEdit ) + self.assertEqual( inspection.nonDisableableReason(), "There is no edit in editScope2." ) + self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : There is no edit in editScope2.", inspection.disableEdit ) Gaffer.MetadataAlgo.setReadOnly( s["editScope"], True ) inspection = self.__inspect( s["editScope"]["out"], "/light", "exposure", s["editScope"] ) @@ -502,7 +503,7 @@ def testDisableEdit( self ) : inspection = self.__inspect( s["editScope"]["out"], "/light", "exposure", s["editScope"] ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Edit is not in the current edit scope. Change scope to Source to disable." ) + self.assertEqual( inspection.nonDisableableReason(), "There is no edit in editScope." ) inspection = self.__inspect( s["editScope"]["out"], "/light", "exposure", None ) self.assertEqual( inspection.source(), s["light"]["parameters"]["exposure"] ) @@ -1088,6 +1089,72 @@ def testShaderNetwork( self ) : editWarning = "Edits to box.add may affect other locations in the scene." ) + def testLightCreatedInEditScope( self ) : + + light = GafferSceneTest.TestLight() + + editScope1 = Gaffer.EditScope( "EditScope1" ) + editScope1.setup( light["out"] ) + + editScope1["light"] = light + editScope1["parent"] = GafferScene.Parent() + editScope1["parent"]["parent"].setValue( "/" ) + editScope1["parent"]["in"].setInput( editScope1["BoxIn"]["out"] ) + editScope1["parent"]["children"][0].setInput( editScope1["light"]["out"] ) + + editScope1["BoxOut"]["in"].setInput( editScope1["parent"]["out"] ) + + editScope2 = Gaffer.EditScope( "EditScope2" ) + editScope2.setup( editScope1["out"] ) + editScope2["in"].setInput( editScope1["out"] ) + + # Make edit in EditScope2. + + i = self.__inspect( editScope2["out"], "/light", "exposure", editScope2 ) + scope2Edit = i.acquireEdit() + self.assertTrue( editScope2.isAncestorOf( scope2Edit ) ) + scope2Edit["enabled"].setValue( True ) + scope2Edit["value"].setValue( 2 ) + + # Check that we can still edit in EditScope1, accompanied by + # a suitable warning. + + self.__assertExpectedResult( + self.__inspect( editScope2["out"], "/light", "exposure", editScope1 ), + source = scope2Edit, + sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Downstream, + editable = True, + edit = light["parameters"]["exposure"], + editWarning = "Parameter has edits downstream in EditScope2." + ) + + def testSourceWithDownstreamOverride( self ) : + + light = GafferSceneTest.TestLight() + + editScope = Gaffer.EditScope() + editScope.setup( light["out"] ) + editScope["in"].setInput( light["out"] ) + + # Make edit in EditScope. + + i = self.__inspect( editScope["out"], "/light", "exposure", editScope ) + scopeEdit = i.acquireEdit() + self.assertTrue( editScope.isAncestorOf( scopeEdit ) ) + scopeEdit["enabled"].setValue( True ) + scopeEdit["value"].setValue( 2 ) + + # Check that we can still edit the source, accompanied by + # a suitable warning. + + self.__assertExpectedResult( + self.__inspect( editScope["out"], "/light", "exposure", editScope = None ), + source = scopeEdit, + sourceType = GafferSceneUI.Private.Inspector.Result.SourceType.Downstream, + editable = True, + edit = light["parameters"]["exposure"], + editWarning = "Parameter has edits downstream in EditScope." + ) if __name__ == "__main__": unittest.main() diff --git a/python/GafferSceneUITest/SetMembershipInspectorTest.py b/python/GafferSceneUITest/SetMembershipInspectorTest.py index ee18066a440..401e10ae6b9 100644 --- a/python/GafferSceneUITest/SetMembershipInspectorTest.py +++ b/python/GafferSceneUITest/SetMembershipInspectorTest.py @@ -690,9 +690,10 @@ def testDisableEdit( self ) : Gaffer.MetadataAlgo.setReadOnly( s["editScope1"], False ) inspection = self.__inspect( s["editScope1"]["out"], "/group/plane", "planeSetEditScope", None ) + self.assertTrue( inspection.acquireEdit( False ).isSame( s["plane"]["sets"] ) ) self.assertFalse( inspection.canDisableEdit() ) - self.assertEqual( inspection.nonDisableableReason(), "Source is in an EditScope. Change scope to editScope1 to disable." ) - self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : Source is in an EditScope. Change scope to editScope1 to disable.", inspection.disableEdit ) + self.assertEqual( inspection.nonDisableableReason(), "plane.sets has no edit to disable." ) + self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : plane.sets has no edit to disable.", inspection.disableEdit ) inspection = self.__inspect( s["editScope1"]["out"], "/group/plane", "planeSetEditScope", s["editScope1"] ) self.assertTrue( inspection.canDisableEdit() ) @@ -705,4 +706,4 @@ def testDisableEdit( self ) : ) if __name__ == "__main__" : - unittest.main() \ No newline at end of file + unittest.main() diff --git a/src/GafferSceneUI/Inspector.cpp b/src/GafferSceneUI/Inspector.cpp index 571313d3f77..d7de94939fe 100644 --- a/src/GafferSceneUI/Inspector.cpp +++ b/src/GafferSceneUI/Inspector.cpp @@ -210,25 +210,37 @@ Inspector::ResultPtr Inspector::inspect() const return nullptr; } - if( result->editScope() && !result->m_editScopeInHistory ) + // If we failed to initialise our editors, then initialise with failures + // explaining why. + if( !result->m_editors ) { - const std::string nonEditableReason = fmt::format( - "The target edit scope {} is not in the scene history.", - result->editScope()->relativeName( result->editScope()->scriptNode() ) - ); - result->m_editFunction = nonEditableReason; - result->m_disableEditFunction = nonEditableReason; - result->m_sourceType = Result::SourceType::Other; - } - else if( !result->m_source ) - { - if( !result->editable() ) + std::string formatString; + if( !result->m_source ) { - // There's no source plug and no way of making - // the property. - result->m_editFunction = "No editable source found in history."; + formatString = "No editable source found in history."; } - result->m_disableEditFunction = "No editable source found in history."; + else if( result->m_editScope && !result->m_editScopeInHistory ) + { + formatString = fmt::format( + "The target edit scope {} is not in the scene history.", + result->editScope()->relativeName( result->editScope()->scriptNode() ) + ); + } + else if( !result->m_editScope ) + { + const EditScope *sourceEditScope = result->m_source->ancestor(); + if( sourceEditScope ) + { + formatString = fmt::format( + "Source is in an EditScope. Change scope to {} to {{}}.", + sourceEditScope->relativeName( sourceEditScope->scriptNode() ) + ); + } + } + + result->m_editors = { + fmt::format( formatString, "edit" ), "", fmt::format( formatString, "disable" ) + }; } if( fallbackValue ) @@ -249,10 +261,15 @@ void Inspector::inspectHistoryWalk( const GafferScene::SceneAlgo::History *histo { Node *node = history->scene->node(); - // If we haven't found the source yet, call `source()` - // to see if we can find one here. + // If we might have a use for it, see if there's a source for the inspected + // value at this point in the history. - if( !result->m_source && history->scene->direction() == Plug::Out ) + std::string editWarning; + ValuePlugPtr source; + if( + history->scene->direction() == Plug::Out && + ( !result->m_source || !result->m_editors ) + ) { if( auto dependencyNode = runTimeCast( node ) ) { @@ -260,130 +277,125 @@ void Inspector::inspectHistoryWalk( const GafferScene::SceneAlgo::History *histo const BoolPlug *enabledPlug = dependencyNode->enabledPlug(); if( !enabledPlug || enabledPlug->getValue() ) { - std::string editWarning; - if( auto source = this->source( history, editWarning ) ) + source = this->source( history, editWarning ); + if( source ) { - // We've found the source of the value we're inspecting. - - result->m_source = static_cast( spreadsheetAwareSource( source.get() ) ); - - if( result->m_editScope && result->m_editScopeInHistory ) - { - result->m_sourceType = Result::SourceType::Upstream; - } - else if( result->m_editScope && node->ancestor() == result->m_editScope ) - { - result->m_sourceType = Result::SourceType::EditScope; - result->m_editScopeInHistory = true; - } - else if( result->m_editScope ) - { - // We'll convert this to `Other` later if we don't - // find the EditScope. - result->m_sourceType = Result::SourceType::Downstream; - } - else - { - result->m_sourceType = Result::SourceType::Other; - } - - // See if we can use it for editing - - if( !result->m_editScope && node->ancestor() ) - { - // We don't allow editing if the user hasn't requested a specific scope - // (they have selected "Source" from the Menu) and the upstream edit is - // inside _any_ EditScope. - result->m_editFunction = fmt::format( - "Source is in an EditScope. Change scope to {} to edit.", - node->ancestor()->relativeName( node->ancestor()->scriptNode() ) - ); - result->m_disableEditFunction = fmt::format( - "Source is in an EditScope. Change scope to {} to disable.", - node->ancestor()->relativeName( node->ancestor()->scriptNode() ) - ); - } - else if( !result->m_editScope || node->ancestor() == result->m_editScope ) - { - const std::string nonEditableReason = ::nonEditableReason( result->m_source.get() ); - - if( nonEditableReason.empty() ) - { - result->m_editFunction = [source = result->m_source] ( bool unused ) { return source; }; - result->m_disableEditFunction = disableEditFunction( result->m_source.get(), history ); - result->m_editWarning = editWarning; - } - else - { - result->m_editFunction = nonEditableReason; - result->m_disableEditFunction = nonEditableReason; - } - } - else if( node->ancestor() && node->ancestor() != result->m_editScope ) - { - result->m_disableEditFunction = fmt::format( - "Edit is not in the current edit scope. Change scope to {} to disable.", - node->ancestor()->relativeName( node->ancestor()->scriptNode() ) - ); - } - else if( !node->ancestor() && result->m_editScope ) - { - result->m_disableEditFunction = "Edit is not in the current edit scope. Change scope to Source to disable."; - } + source = static_cast( spreadsheetAwareSource( source.get() ) ); } } } } - // Check to see if we're at the `targetEditScope()`. + // If this is the first source we've seen, then initialise + // `Result::source()` and `Result::sourceType()` from it. - if( auto editScope = runTimeCast( node ) ) + const bool hadSourceAlready = (bool)result->m_source; + if( source && !hadSourceAlready ) { - if( !result->m_editScopeInHistory && history->scene == editScope->inPlug() && editScope == result->m_editScope ) + result->m_source = source; + + if( result->m_editScope && result->m_editScopeInHistory ) { - // We are leaving the target EditScope for the first time. We - // consider EditScopes on the way out to allow other nodes within - // the scope to take precedence. An existing edit in the scope will - // have been picked up via `source()` already. - // - // \todo Should call `editFunction()` with the context from the - // `outPlug()` of the EditScope - see TransformTool. We should also - // explicitly prefer branches where `scene:path` matches the value - // in the `outPlug()` context, to avoid making edits to locations - // other than the one emerging from the EditScope. + result->m_sourceType = Result::SourceType::Upstream; + } + else if( result->m_editScope && node->ancestor() == result->m_editScope ) + { + result->m_sourceType = Result::SourceType::EditScope; result->m_editScopeInHistory = true; - Context::Scope scope( history->context.get() ); - if( editScope->enabledPlug()->getValue() ) + } + else + { + // We'll convert this to Downstream if we later find the edit scope. + result->m_sourceType = Result::SourceType::Other; + } + } + + // If we haven't initialised the editors yet, see if we can do that here. + + if( !result->m_editors ) + { + // Initialise editors from source if we can. + if( source && source->ancestor() == result->m_editScope ) + { + const std::string nonEditableReason = ::nonEditableReason( result->m_source.get() ); + if( nonEditableReason.empty() ) { - result->m_editFunction = editFunction( editScope, history ); - if( result->m_source && result->m_sourceType == Result::SourceType::Downstream ) - { - const Node *downstreamNode = result->m_source->node(); - const auto *downstreamEditScope = downstreamNode->ancestor(); - downstreamNode = downstreamEditScope ? downstreamEditScope : downstreamNode; - result->m_editWarning = fmt::format( - "{} has edits downstream in {}.", - std::string( 1, std::toupper( type()[0] ) ) + type().substr( 1 ), - downstreamNode->relativeName( downstreamNode->scriptNode() ) - ); - } + result->m_editors = { + [source = source] ( bool unused ) { return source; }, + editWarning, + disableEditFunction( source.get(), history ) + }; } else { - result->m_editFunction = fmt::format( - "The target edit scope {} is disabled.", - editScope->relativeName( editScope->scriptNode() ) - ); + result->m_editors = { nonEditableReason, "", nonEditableReason }; + } + } + // Otherwise try to initialise from EditScope if we've hit it. + else if( auto editScope = runTimeCast( node ) ) + { + if( !result->m_editScopeInHistory && history->scene == editScope->inPlug() && editScope == result->m_editScope ) + { + // We are leaving the target EditScope for the first time. We + // consider EditScopes on the way out to allow other nodes within + // the scope to take precedence. An existing edit in the scope will + // have been picked up via `source()` already. + // + // \todo Should call `editFunction()` with the context from the + // `outPlug()` of the EditScope - see TransformTool. We should also + // explicitly prefer branches where `scene:path` matches the value + // in the `outPlug()` context, to avoid making edits to locations + // other than the one emerging from the EditScope. + result->m_editScopeInHistory = true; + Context::Scope scope( history->context.get() ); + EditFunctionOrFailure func; + if( editScope->enabledPlug()->getValue() ) + { + func = editFunction( editScope, history ); + } + else + { + func = fmt::format( + "The target edit scope {} is disabled.", + editScope->relativeName( editScope->scriptNode() ) + ); + } + + result->m_editors = { + func, "", + fmt::format( + "There is no edit in {}.", editScope->relativeName( editScope->scriptNode() ) + ) + }; } } + + if( result->m_editors && hadSourceAlready ) + { + result->m_sourceType = Result::SourceType::Downstream; + } + + // If we initialised the edit function, tag on a warning if any edits won't be visible + // due to being overridden downstream. + if( result->m_editors && std::holds_alternative( result->m_editors->editFunction ) && result->m_sourceType == Result::SourceType::Downstream ) + { + const Node *downstreamNode = result->m_source->node(); + const auto *downstreamEditScope = downstreamNode->ancestor(); + downstreamNode = downstreamEditScope ? downstreamEditScope : downstreamNode; + result->m_editors->editWarning = fmt::format( + "{} has edits downstream in {}.", + std::string( 1, std::toupper( type()[0] ) ) + type().substr( 1 ), + downstreamNode->relativeName( downstreamNode->scriptNode() ) + ); + } } - // If we haven't found the source and the EditScope, then recurse up the - // history until we have. + // If we haven't found everything we want yet, then recurse up the history + // until we have. for( const auto &predecessor : history->predecessors ) { - if( result->m_source && ((bool)result->m_editScope == result->m_editScopeInHistory) ) + if( result->m_source && ((bool)result->m_editScope == result->m_editScopeInHistory) && result->m_editors ) { return; } @@ -732,13 +744,12 @@ const std::string &Inspector::Result::fallbackDescription() const bool Inspector::Result::editable() const { - auto f = std::get_if( &m_editFunction ); - return f && *f; + return m_editors && std::holds_alternative( m_editors->editFunction ); } std::string Inspector::Result::nonEditableReason() const { - if( auto s = std::get_if( &m_editFunction ) ) + if( auto s = std::get_if( &m_editors.value().editFunction ) ) { return *s; } @@ -748,23 +759,22 @@ std::string Inspector::Result::nonEditableReason() const Gaffer::ValuePlugPtr Inspector::Result::acquireEdit( bool createIfNecessary ) const { - if( auto f = std::get_if( &m_editFunction ) ) + if( auto f = std::get_if( &m_editors.value().editFunction ) ) { return (*f)( createIfNecessary ); } - throw IECore::Exception( "Not editable : " + std::get( m_editFunction ) ); + throw IECore::Exception( "Not editable : " + std::get( m_editors.value().editFunction ) ); } bool Inspector::Result::canDisableEdit() const { - auto f = std::get_if( &m_disableEditFunction ); - return f && *f; + return std::holds_alternative( m_editors.value().disableEditFunction ); } std::string Inspector::Result::nonDisableableReason() const { - if( auto s = std::get_if( &m_disableEditFunction ) ) + if( auto s = std::get_if( &m_editors.value().disableEditFunction ) ) { return *s; } @@ -774,15 +784,15 @@ std::string Inspector::Result::nonDisableableReason() const void Inspector::Result::disableEdit() const { - if( auto f = std::get_if( &m_disableEditFunction ) ) + if( auto f = std::get_if( &m_editors.value().disableEditFunction ) ) { return (*f)(); } - throw IECore::Exception( "Cannot disable edit : " + std::get( m_disableEditFunction ) ); + throw IECore::Exception( "Cannot disable edit : " + std::get( m_editors.value().disableEditFunction ) ); } std::string Inspector::Result::editWarning() const { - return m_editWarning; + return m_editors.value().editWarning; }