Inspector : Make handling of downstream overrides consistent

Changes.md

@@ -27,6 +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 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
 ---

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 <unordered_set>
 #include <unordered_map>
+#include <variant>
 
 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<Gaffer::ValuePlugPtr ( bool createIfNecessary )>;
-		using EditFunctionOrFailure = boost::variant<EditFunction, std::string>;
+		using EditFunctionOrFailure = std::variant<EditFunction, std::string>;
 		/// 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<void ()>;
-		using DisableEditFunctionOrFailure = boost::variant<DisableEditFunction, std::string>;
+		using DisableEditFunctionOrFailure = std::variant<DisableEditFunction, std::string>;
 		/// 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).
@@ -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<Editors> m_editors;
 
 };
 

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() )

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"] )

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 None 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()

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()
+	unittest.main()