Merge pull request #6028 from murraystevenson/inspectorDisableEdit

Inspector : Add support for disabling edits

Changes.md

@@ -28,6 +28,7 @@ Improvements
 - Spreadsheet : Added yellow underlining to the currently active row.
 - PlugLayout : Summaries and activators are now evaluated in a context determined relative to the focus node.
 - Editor : The node graph is now evaluated in a context determined relative to the focus node.
+- LightEditor, RenderPassEditor : The "Disable Edit" right-click menu item and <kdb>D</kdb> shortcut now act as a toggle, where edits disabled in the current session via these actions can be reenabled with <kbd>D</kbd> or by selecting "Reenable Edit" from the right-click menu.
 
 Fixes
 -----
@@ -55,6 +56,7 @@ Fixes
 - FreezeTransform : Constant primitive variables with point/vector interpretations are now also transformed.
 - usdview : Added Windows support (#5599).
 - ContextTracker : Removed unnecessary reference increment/decrement from `isTracked()`, `context()` and `isEnabled()`.
+- Menu : Fixed bug causing a menu item's tooltip to not hide when moving the cursor to another menu item without a tooltip.
 
 API
 ---

include/GafferSceneUI/Private/AttributeInspector.h

@@ -65,9 +65,9 @@ class GAFFERSCENEUI_API AttributeInspector : public Inspector
 
 		GafferScene::SceneAlgo::History::ConstPtr history() const override;
 		IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *scope, const GafferScene::SceneAlgo::History *history) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 
 		bool attributeExists() const;
 

include/GafferSceneUI/Private/Inspector.h

@@ -170,7 +170,7 @@ class GAFFERSCENEUI_API Inspector : public IECore::RefCounted, public Gaffer::Si
 		/// history class?
 		virtual Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const;
 
-		using EditFunction = std::function<Gaffer::ValuePlugPtr ()>;
+		using EditFunction = std::function<Gaffer::ValuePlugPtr ( bool createIfNecessary )>;
 		using EditFunctionOrFailure = boost::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.
@@ -183,6 +183,14 @@ class GAFFERSCENEUI_API Inspector : public IECore::RefCounted, public Gaffer::Si
 		/// > that edits the processor itself.
 		virtual EditFunctionOrFailure editFunction( Gaffer::EditScope *editScope, const GafferScene::SceneAlgo::History *history ) const;
 
+		using DisableEditFunction = std::function<void ()>;
+		using DisableEditFunctionOrFailure = boost::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).
+		/// Called with `history->context` as the current context.
+		virtual DisableEditFunctionOrFailure disableEditFunction( Gaffer::ValuePlug *plug, const GafferScene::SceneAlgo::History *history ) const;
+
 	protected :
 
 		Gaffer::EditScope *targetEditScope() const;
@@ -338,11 +346,21 @@ class GAFFERSCENEUI_API Inspector::Result : public IECore::RefCounted
 		/// Returns a plug that can be used to edit the property
 		/// represented by this inspector, creating it if necessary.
 		/// Throws if `!editable()`.
-		Gaffer::ValuePlugPtr acquireEdit() const;
+		Gaffer::ValuePlugPtr acquireEdit( bool createIfNecessary = true ) const;
 		/// Returns a warning associated with the plug returned
 		/// by `acquireEdit()`. This should be displayed to the user.
 		std::string editWarning() const;
 
+		/// Returns `true` if `disableEdit()` will disable the edit
+		/// at `source()`, and `false` otherwise.
+		bool canDisableEdit() const;
+		/// If `canDisableEdit()` returns false, returns the reason why.
+		/// This should be displayed to the user.
+		std::string nonDisableableReason() const;
+		/// Disables the edit at `source()`. Throws if
+		/// `!canDisableEdit()`
+		void disableEdit() const;
+
 	private :
 
 		Result( const IECore::ConstObjectPtr &value, const Gaffer::EditScopePtr &editScope );
@@ -359,6 +377,8 @@ class GAFFERSCENEUI_API Inspector::Result : public IECore::RefCounted
 		EditFunctionOrFailure m_editFunction;
 		std::string m_editWarning;
 
+		DisableEditFunctionOrFailure m_disableEditFunction;
+
 };
 
 } // namespace Private

include/GafferSceneUI/Private/OptionInspector.h

@@ -63,9 +63,9 @@ class GAFFERSCENEUI_API OptionInspector : public Inspector
 
 		GafferScene::SceneAlgo::History::ConstPtr history() const override;
 		IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history ) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *scope, const GafferScene::SceneAlgo::History *history ) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 
 	private :
 

include/GafferSceneUI/Private/ParameterInspector.h

@@ -67,9 +67,9 @@ class GAFFERSCENEUI_API ParameterInspector : public AttributeInspector
 	private :
 
 		IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history ) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *editScope, const GafferScene::SceneAlgo::History *history ) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 
 		const IECoreScene::ShaderNetwork::Parameter m_parameter;
 

include/GafferSceneUI/Private/SetMembershipInspector.h

@@ -75,13 +75,14 @@ class GAFFERSCENEUI_API SetMembershipInspector : public Inspector
 
 		GafferScene::SceneAlgo::History::ConstPtr history() const override;
 		IECore::ConstObjectPtr value( const GafferScene::SceneAlgo::History *history) const override;
+		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
 		/// For the given `history`, returns either the "sets" `StringPlug` of an `ObjectSource`
 		/// node, the "name" `StringPlug` of a `Set` node, the `Spreadsheet::RowPlug` for the
 		/// appropriate row of a set membership processor spreadsheet or `nullptr` if none of
 		/// those are found.
 		Gaffer::ValuePlugPtr source( const GafferScene::SceneAlgo::History *history, std::string &editWarning ) const override;
 		EditFunctionOrFailure editFunction( Gaffer::EditScope *scope, const GafferScene::SceneAlgo::History *history ) const override;
-		IECore::ConstObjectPtr fallbackValue( const GafferScene::SceneAlgo::History *history, std::string &description ) const override;
+		DisableEditFunctionOrFailure disableEditFunction( Gaffer::ValuePlug *plug, const GafferScene::SceneAlgo::History *history ) const override;
 
 	private :
 

python/GafferSceneUI/_InspectorColumn.py

@@ -162,9 +162,11 @@ def __editSelectedCells( pathListing, quickBoolean = True ) :
 
 		__inspectorColumnPopup.popup( parent = pathListing )
 
-def __disablableInspectionTweaks( pathListing ) :
+def __toggleableInspections( pathListing ) :
 
-	tweaks = []
+	inspections = []
+	nonEditableReason = ""
+	toggleShouldDisable = True
 
 	path = pathListing.getPath().copy()
 	for columnSelection, column in zip( pathListing.getSelection(), pathListing.getColumns() ) :
@@ -176,43 +178,55 @@ def __disablableInspectionTweaks( pathListing ) :
 
 			with inspectionContext :
 				inspection = column.inspector().inspect()
-				if inspection is not None and inspection.editable() :
-					source = inspection.source()
-					editScope = inspection.editScope()
-					if (
-						(
-							(
-								isinstance( source, ( Gaffer.TweakPlug, Gaffer.NameValuePlug ) ) and
-								source["enabled"].getValue()
-							) or
-							isinstance( column.inspector(), GafferSceneUI.Private.SetMembershipInspector )
-						) and
-						( editScope is None or editScope.isAncestorOf( source ) )
-					) :
-						tweaks.append( ( pathString, column.inspector() ) )
-					else :
-						return []
-				else :
-					return []
+				if inspection is None :
+					continue
 
-	return tweaks
+				canReenableEdit = False
+				if not inspection.canDisableEdit() and inspection.editable() :
+					edit = inspection.acquireEdit( createIfNecessary = False )
+					canReenableEdit = isinstance( edit, ( Gaffer.NameValuePlug, Gaffer.OptionalValuePlug, Gaffer.TweakPlug ) ) and Gaffer.Metadata.value( edit, "inspector:disabledEdit" )
+					if canReenableEdit :
+						toggleShouldDisable = False
 
-def __disableEdits( pathListing ) :
+				if canReenableEdit or inspection.canDisableEdit() :
+					inspections.append( inspection )
+				elif nonEditableReason == "" :
+					# Prefix reason with the column header to disambiguate when more than one column has selection
+					nonEditableReason = "{} : ".format( column.headerData().value ) if len( [ x for x in pathListing.getSelection() if not x.isEmpty() ] ) > 1 else ""
+					nonEditableReason += inspection.nonDisableableReason() if toggleShouldDisable else inspection.nonEditableReason()
 
-	edits = __disablableInspectionTweaks( pathListing )
-	path = pathListing.getPath().copy()
-	with Gaffer.UndoScope( pathListing.ancestor( GafferUI.Editor ).scriptNode() ) :
-		for pathString, inspector in edits :
-			path.setFromString( pathString )
-			with path.inspectionContext() :
-				inspection = inspector.inspect()
-				if inspection is not None and inspection.editable() :
-					source = inspection.source()
+	return inspections, nonEditableReason, toggleShouldDisable
 
-					if isinstance( source, ( Gaffer.TweakPlug, Gaffer.NameValuePlug ) ) :
-						source["enabled"].setValue( False )
-					elif isinstance( inspector, GafferSceneUI.Private.SetMembershipInspector ) :
-						inspector.editSetMembership( inspection, pathString, GafferScene.EditScopeAlgo.SetMembership.Unchanged )
+def __toggleEditEnabled( pathListing ) :
+
+	global __inspectorColumnPopup
+
+	inspections, nonEditableReason, shouldDisable = __toggleableInspections( pathListing )
+
+	if nonEditableReason != "" :
+		with GafferUI.PopupWindow() as __inspectorColumnPopup :
+			with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Horizontal, spacing = 4 ) :
+				GafferUI.Image( "warningSmall.png" )
+				GafferUI.Label( "<h4>{}</h4>".format( nonEditableReason ) )
+
+			__inspectorColumnPopup.popup( parent = pathListing )
+
+		return
+
+	with Gaffer.UndoScope( pathListing.ancestor( GafferUI.Editor ).scriptNode() ) :
+		for inspection in inspections :
+			if shouldDisable :
+				inspection.disableEdit()
+				# We register non-persistent metadata on disabled edits to later determine
+				# whether the disabled edit is a suitable candidate for enabling. This allows
+				# investigative toggling of edits in the current session while avoiding enabling
+				# edits the user may not expect to exist, such as previously unedited spreadsheet
+				# cells in EditScope processors.
+				Gaffer.Metadata.registerValue( inspection.source(), "inspector:disabledEdit", True, persistent = False )
+			else :
+				edit = inspection.acquireEdit( createIfNecessary = False )
+				edit["enabled"].setValue( True )
+				Gaffer.Metadata.deregisterValue( edit, "inspector:disabledEdit" )
 
 def __removableAttributeInspections( pathListing ) :
 
@@ -385,12 +399,14 @@ def __contextMenu( column, pathListing, menuDefinition ) :
 			"command" : functools.partial( __editSelectedCells, pathListing, toggleOnly ),
 		}
 	)
+	inspections, nonEditableReason, disable = __toggleableInspections( pathListing )
 	menuDefinition.append(
-		"Disable Edit",
+		"{} Edit{}".format( "Disable" if disable else "Reenable", "s" if len( inspections ) > 1 else "" ),
 		{
-			"command" : functools.partial( __disableEdits, pathListing ),
-			"active" : len( __disablableInspectionTweaks( pathListing ) ) > 0,
+			"command" : functools.partial( __toggleEditEnabled, pathListing ),
+			"active" : len( inspections ) > 0 and nonEditableReason == "",
 			"shortCut" : "D",
+			"description" : nonEditableReason,
 		}
 	)
 
@@ -427,8 +443,11 @@ def __keyPress( column, pathListing, event ) :
 			return True
 
 		if event.key == "D" :
-			if len( __disablableInspectionTweaks( pathListing ) ) > 0 :
-				__disableEdits( pathListing )
+			inspections, nonEditableReason, _ = __toggleableInspections( pathListing )
+			# We allow toggling when there is a nonEditableReason to let __toggleEditEnabled
+			# present the reason to the user via a popup.
+			if len( inspections ) > 0 or nonEditableReason != "" :
+				__toggleEditEnabled( pathListing )
 			return True
 
 		if event.key in ( "Backspace", "Delete" ) :

python/GafferSceneUITest/AttributeInspectorTest.py

@@ -219,7 +219,7 @@ def testSourceAndEdits( self ) :
 			source = s["light"]["visualiserAttributes"]["scale"],
 			sourceType = SourceType.Other,
 			editable=False,
-			nonEditableReason = "The target EditScope (editScope1) is not in the scene history."
+			nonEditableReason = "The target edit scope editScope1 is not in the scene history."
 		)
 
 		# If it is in the history though, and we're told to use it, then we will.
@@ -369,7 +369,7 @@ def testSourceAndEdits( self ) :
 			source = independentAttributeTweakPlug,
 			sourceType = SourceType.Downstream,
 			editable = False,
-			nonEditableReason = "The target EditScope (editScope2) is disabled."
+			nonEditableReason = "The target edit scope editScope2 is disabled."
 		)
 
 		s["editScope2"]["enabled"].setValue( True )
@@ -455,7 +455,7 @@ def testEditScopeNotInHistory( self ) :
 			source = light["visualiserAttributes"]["scale"],
 			sourceType = SourceType.Other,
 			editable = False,
-			nonEditableReason = "The target EditScope (EditScope) is not in the scene history."
+			nonEditableReason = "The target edit scope EditScope is not in the scene history."
 		)
 
 		self.__assertExpectedResult(
@@ -471,7 +471,7 @@ def testEditScopeNotInHistory( self ) :
 			source = attributeTweaks["tweaks"][0],
 			sourceType = SourceType.Other,
 			editable = False,
-			nonEditableReason = "The target EditScope (EditScope) is not in the scene history."
+			nonEditableReason = "The target edit scope EditScope is not in the scene history."
 		)
 
 	def testDisabledTweaks( self ) :
@@ -821,6 +821,105 @@ def testLightFilter( self ) :
 			edit = edit
 		)
 
+	def testAcquireEditCreateIfNecessary( self ) :
+
+		s = Gaffer.ScriptNode()
+
+		s["light"] = GafferSceneTest.TestLight()
+		s["light"]["visualiserAttributes"]["scale"]["enabled"].setValue( True )
+
+		s["group"] = GafferScene.Group()
+		s["editScope"] = Gaffer.EditScope()
+
+		s["group"]["in"][0].setInput( s["light"]["out"] )
+
+		s["editScope"].setup( s["group"]["out"] )
+		s["editScope"]["in"].setInput( s["group"]["out"] )
+
+		inspection = self.__inspect( s["group"]["out"], "/group/light", "gl:visualiser:scale", None )
+		self.assertEqual( inspection.acquireEdit( createIfNecessary = False ), s["light"]["visualiserAttributes"]["scale"] )
+
+		inspection = self.__inspect( s["editScope"]["out"], "/group/light", "gl:visualiser:scale", s["editScope"] )
+		self.assertIsNone( inspection.acquireEdit( createIfNecessary = False ) )
+
+		edit = inspection.acquireEdit( createIfNecessary = True )
+		self.assertIsNotNone( edit )
+		self.assertEqual( inspection.acquireEdit( createIfNecessary = False ), edit )
+
+	def testDisableEdit( self ) :
+
+		s = Gaffer.ScriptNode()
+
+		s["light"] = GafferSceneTest.TestLight()
+		s["light"]["visualiserAttributes"]["scale"]["enabled"].setValue( True )
+
+		s["group"] = GafferScene.Group()
+		s["editScope1"] = Gaffer.EditScope()
+		s["editScope2"] = Gaffer.EditScope()
+
+		s["group"]["in"][0].setInput( s["light"]["out"] )
+
+		s["editScope1"].setup( s["group"]["out"] )
+		s["editScope1"]["in"].setInput( s["group"]["out"] )
+
+		s["editScope2"].setup( s["editScope1"]["out"] )
+		s["editScope2"]["in"].setInput( s["editScope1"]["out"] )
+
+		Gaffer.MetadataAlgo.setReadOnly( s["light"]["visualiserAttributes"]["scale"]["enabled"], True )
+		inspection = self.__inspect( s["group"]["out"], "/group/light", "gl:visualiser:scale", None )
+		self.assertFalse( inspection.canDisableEdit() )
+		self.assertEqual( inspection.nonDisableableReason(), "light.visualiserAttributes.scale.enabled is locked." )
+		self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : light.visualiserAttributes.scale.enabled is locked.", inspection.disableEdit )
+
+		Gaffer.MetadataAlgo.setReadOnly( s["light"]["visualiserAttributes"]["scale"]["enabled"], False )
+		Gaffer.MetadataAlgo.setReadOnly( s["light"]["visualiserAttributes"]["scale"], True )
+		inspection = self.__inspect( s["group"]["out"], "/group/light", "gl:visualiser:scale", None )
+		self.assertFalse( inspection.canDisableEdit() )
+		self.assertEqual( inspection.nonDisableableReason(), "light.visualiserAttributes.scale is locked." )
+		self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : light.visualiserAttributes.scale is locked.", inspection.disableEdit )
+
+		Gaffer.MetadataAlgo.setReadOnly( s["light"]["visualiserAttributes"]["scale"], False )
+		inspection = self.__inspect( s["group"]["out"], "/group/light", "gl:visualiser:scale", None )
+		self.assertTrue( inspection.canDisableEdit() )
+		self.assertEqual( inspection.nonDisableableReason(), "" )
+		inspection.disableEdit()
+		self.assertFalse( s["light"]["visualiserAttributes"]["scale"]["enabled"].getValue() )
+
+		lightEdit = GafferScene.EditScopeAlgo.acquireAttributeEdit(
+			s["editScope1"], "/group/light", "gl:visualiser:scale", createIfNecessary = True
+		)
+		lightEdit["enabled"].setValue( True )
+		lightEdit["value"].setValue( 2.0 )
+
+		inspection = self.__inspect( s["editScope1"]["out"], "/group/light", "gl:visualiser:scale", s["editScope2"] )
+		self.assertFalse( inspection.canDisableEdit() )
+		self.assertEqual( inspection.nonDisableableReason(), "The target edit scope editScope2 is not in the scene history." )
+
+		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 )
+
+		Gaffer.MetadataAlgo.setReadOnly( s["editScope1"], True )
+		inspection = self.__inspect( s["editScope1"]["out"], "/group/light", "gl:visualiser:scale", s["editScope1"] )
+		self.assertFalse( inspection.canDisableEdit() )
+		self.assertEqual( inspection.nonDisableableReason(), "editScope1 is locked." )
+		self.assertRaisesRegex( IECore.Exception, "Cannot disable edit : editScope1 is locked.", inspection.disableEdit )
+
+		Gaffer.MetadataAlgo.setReadOnly( s["editScope1"], False )
+		inspection = self.__inspect( s["editScope1"]["out"], "/group/light", "gl:visualiser:scale", s["editScope1"] )
+		self.assertTrue( inspection.canDisableEdit() )
+		self.assertEqual( inspection.nonDisableableReason(), "" )
+		inspection.disableEdit()
+		self.assertFalse( lightEdit["enabled"].getValue() )
+
+		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." )
+
+		inspection = self.__inspect( s["editScope1"]["out"], "/group/light", "gl:visualiser:scale", None )
+		self.assertFalse( inspection.canDisableEdit() )
+		self.assertEqual( inspection.nonDisableableReason(), "light.visualiserAttributes.scale.enabled is not enabled." )
 
 if __name__ == "__main__" :
 	unittest.main()