TransformTool : Use standardised selection logic

Changes.md

@@ -54,6 +54,7 @@ Improvements
 - Checkerboard : Optimized image generation when rotation is 0.
 - InteractiveRender : Added a warning when attribute edits require geometry to be regenerated, as this can have a performance impact. Examples in Arnold include subdivision changes or changes to attributes inherited by procedurals.
 - NodeMenu : Removed Loop node. This node can have severe consequences for performance if used inappropriately. Depending on the use case, the Collect nodes and others often provide a more performant alternative. The Loop node can still be created via the scripting API, but we recommend you consider the alternatives and/or request advice before using it.
+- TransformTool : Improved tool state messaging to include node names and/or paths.
 
 Fixes
 -----
@@ -65,6 +66,7 @@ Fixes
   - Fixed handling of interpolation for normals.
   - Fixed writing of indexed primitive variables to non-indexed USD attributes.
   - Fixed handling of GeometricInterpretation/Role.
+- TransformTools : Fixed bug finding existing nodes inside EditScopes.
 - Viewer : Fixed bug that caused mouse clicks in empty toolbar regions to be ignored.
 - PlugAlgo/BoxIO : Fixed bug handling nested compound plugs.
 - Resample : Fixed hash of intermediate `deep` plug.

include/Gaffer/EditScope.h

@@ -44,8 +44,27 @@ namespace Gaffer
 
 class BoxOut;
 
-/// A container node for interactive tools to make nodes
-/// in as necessary.
+/// A container node for interactive tools to make nodes in as necessary.
+///
+/// EditScopes and Tools
+/// ====================
+///
+/// Tools that affect change by modifying nodes/plugs in the Node Graph
+/// should use the following logic to determine their edit target:
+///
+///  - If no EditScope has been selected, use the last (closest) upstream
+///    target.
+///
+///  - If an EditScope has been selected, prefer existing targets inside
+///    the EditScope over using the EditScopeAlgo to acquire a new target.
+///
+///  - If an EditScope has been selected, but is upstream of another target
+///    either error (if overrides preclude editing), or allow editing with
+///    a suitable warning identifying the last downstream target.
+///
+///  - If an EditScope has been selected but is not in the scene history,
+///    error.
+///
 class GAFFER_API EditScope : public Box
 {
 

include/GafferSceneUI/TransformTool.h

@@ -169,8 +169,16 @@ class GAFFERSCENEUI_API TransformTool : public GafferSceneUI::SelectionTool
 
 			private :
 
+				void initFromHistory( const GafferScene::SceneAlgo::History *history );
 				void initFromSceneNode( const GafferScene::SceneAlgo::History *history );
-				void initWalk( const GafferScene::SceneAlgo::History *history, bool &editScopeFound );
+				void initFromEditScope( const GafferScene::SceneAlgo::History *history );
+				void initWalk(
+					const GafferScene::SceneAlgo::History *history,
+					bool &editScopeFound,
+					const GafferScene::SceneAlgo::History *editScopeOutHistory = nullptr
+				);
+				bool initRequirementsSatisfied( bool editScopeFound );
+
 				void throwIfNotEditable() const;
 				Imath::M44f transformToLocalSpace() const;
 
@@ -189,6 +197,7 @@ class GAFFERSCENEUI_API TransformTool : public GafferSceneUI::SelectionTool
 				Imath::M44f m_transformSpace;
 				bool m_aimConstraint;
 
+				static std::string displayName( const GraphComponent *component );
 		};
 
 		/// Returns the current selection.

python/GafferSceneUITest/TransformToolTest.py

@@ -109,7 +109,7 @@ def testSelectionEditScopes( self ) :
 
 		selection = GafferSceneUI.TransformTool.Selection( plane["out"], "/plane", Gaffer.Context(), editScope )
 		self.assertFalse( selection.editable() )
-		self.assertEqual( selection.warning(), "EditScope not in history" )
+		self.assertEqual( selection.warning(), "The target EditScope \"EditScope\" is not in the scene history" )
 		self.assertRaises( RuntimeError, selection.acquireTransformEdit )
 		self.assertRaises( RuntimeError, selection.transformSpace )
 		self.assertEqual( selection.editScope(), editScope )
@@ -132,7 +132,7 @@ def testSelectionEditScopes( self ) :
 		editScope["enabled"].setValue( False )
 		selection = GafferSceneUI.TransformTool.Selection( transform["out"], "/plane", Gaffer.Context(), editScope )
 		self.assertFalse( selection.editable() )
-		self.assertEqual( selection.warning(), "EditScope disabled" )
+		self.assertEqual( selection.warning(), "The target EditScope \"EditScope\" is disabled" )
 		self.assertEqual( selection.editScope(), editScope )
 
 		editScope["enabled"].setValue( True )
@@ -150,7 +150,7 @@ def testSelectionEditScopes( self ) :
 
 		selection = GafferSceneUI.TransformTool.Selection( transform["out"], "/plane", Gaffer.Context(), editScope )
 		self.assertFalse( selection.editable() )
-		self.assertEqual( selection.warning(), "EditScope overridden downstream" )
+		self.assertEqual( selection.warning(), "The target EditScope \"EditScope\" is overridden downstream by \"Transform\"" )
 		self.assertEqual( selection.upstreamScene(), transform["out"] )
 
 		# Disable the downstream node and we should be back in business.
@@ -161,6 +161,59 @@ def testSelectionEditScopes( self ) :
 		self.assertEqual( selection.warning(), "" )
 		self.assertEqual( selection.upstreamScene(), editScope["out"] )
 
+		# Ensure that we use any existing node within an EditScope in preference
+		# to creating a new tweak node
+
+		plane2 = GafferScene.Plane()
+		plane2["name"].setValue( "otherPlane" )
+		editScope["plane2"] = plane2
+
+		editScope["parent"] = GafferScene.Parent()
+		editScope["parent"]["parent"].setValue( "/" )
+		editScope["parent"]["in"].setInput( editScope["BoxOut"]["in"].getInput() )
+		editScope["parent"]["children"][0].setInput( plane2["out"] )
+		editScope["BoxOut"]["in"].setInput( editScope["parent"]["out"] )
+
+		selection = GafferSceneUI.TransformTool.Selection( transform["out"], "/otherPlane", Gaffer.Context(), editScope )
+
+		self.assertTrue( selection.editable() )
+		self.assertEqual( selection.warning(), "" )
+		self.assertEqual( selection.upstreamScene(), plane2["out"] )
+		self.assertEqual( selection.editScope(), editScope )
+		self.assertEqual( selection.editTarget(), plane2["transform"] )
+
+		# Ensure we handle being inside the chosen edit scope
+
+		selection = GafferSceneUI.TransformTool.Selection( editScope["parent"]["out"], "/plane", Gaffer.Context(), editScope )
+		self.assertFalse( selection.editable() )
+		self.assertEqual( selection.warning(), "The output of the target EditScope \"EditScope\" is not in the scene history" )
+
+	def testNestedEditScopes( self ) :
+
+		outerScope = Gaffer.EditScope()
+		outerScope.setup( GafferScene.ScenePlug() )
+		innerScope = Gaffer.EditScope()
+		outerScope["Inner"] = innerScope
+		innerScope.setup( outerScope["BoxIn"]["out"] )
+		innerScope["in"].setInput( outerScope["BoxIn"]["out"] )
+		innerScope["parent"] = GafferScene.Parent()
+		innerScope["parent"]["parent"].setValue( "/" )
+		innerScope["parent"]["in"].setInput( innerScope["BoxIn"]["out"] )
+		innerScope["BoxOut"]["in"].setInput( innerScope["parent"]["out"] )
+		innerScope["plane"] = GafferScene.Plane()
+		innerScope["parent"]["children"][0].setInput( innerScope["plane"]["out"] )
+		outerScope["BoxOut"]["in"].setInput( innerScope["out"] )
+
+		selection = GafferSceneUI.TransformTool.Selection( outerScope["out"], "/plane", Gaffer.Context(), innerScope )
+		self.assertTrue( selection.editable() )
+		self.assertEqual( selection.editTarget(), innerScope["plane"]["transform"] )
+		self.assertEqual( selection.editScope(), innerScope )
+
+		selection = GafferSceneUI.TransformTool.Selection( outerScope["out"], "/plane", Gaffer.Context(), outerScope )
+		self.assertTrue( selection.editable() )
+		self.assertEqual( selection.editTarget(), outerScope )
+		self.assertEqual( selection.editScope(), outerScope )
+
 	def testSceneReaderSelectionEditability( self ) :
 
 		sceneReader = GafferScene.SceneReader()
@@ -203,6 +256,40 @@ def testAcquireTransformEdit( self ) :
 		edit = selection.acquireTransformEdit()
 		self.assertTrue( editScope.isAncestorOf( edit.translate ) )
 
+		# Check readOnly is respected
+
+		plane["name"].setValue( "plane2" )
+
+		Gaffer.MetadataAlgo.setReadOnly( plane["transform"]["translate"], True )
+
+		# We still allow 'editing' of existing plugs, but the handles should take care of disabling themselves.
+		selection = GafferSceneUI.TransformTool.Selection( plane["out"], "/plane2", Gaffer.Context(), None )
+		self.assertTrue( selection.editable() )
+		self.assertEqual( selection.warning(), "\"Plane.transform.translate\" is locked" )
+
+		Gaffer.MetadataAlgo.setReadOnly( editScope, True )
+
+		selection = GafferSceneUI.TransformTool.Selection( editScope["out"], "/plane2", Gaffer.Context(), editScope )
+		self.assertFalse( selection.editable() )
+		self.assertEqual( selection.warning(), "\"EditScope\" is locked" )
+		with six.assertRaisesRegex( self, RuntimeError, "Selection is not editable" ) :
+			self.assertIsNone( selection.acquireTransformEdit() )
+
+		Gaffer.MetadataAlgo.setReadOnly( editScope, False )
+
+		selection = GafferSceneUI.TransformTool.Selection( editScope["out"], "/plane2", Gaffer.Context(), editScope )
+		edit = selection.acquireTransformEdit()
+
+		Gaffer.MetadataAlgo.setReadOnly( edit.translate.ancestor( Gaffer.Spreadsheet.RowsPlug ).source(), True  )
+
+		plane["name"].setValue( "plane3" )
+
+		selection = GafferSceneUI.TransformTool.Selection( editScope["out"], "/plane3", Gaffer.Context(), editScope )
+		self.assertFalse( selection.editable() )
+		self.assertEqual( selection.warning(), "\"EditScope.TransformEdits.edits\" is locked" )
+		with six.assertRaisesRegex( self, RuntimeError, "Selection is not editable" ) :
+			self.assertIsNone( selection.acquireTransformEdit() )
+
 	def testDontEditUpstreamOfReference( self ) :
 
 		script = Gaffer.ScriptNode()
@@ -244,6 +331,7 @@ def testDontEditUpstreamOfReference( self ) :
 		self.assertEqual( selection.upstreamScene(), script["reference"]["transform"]["out"] )
 		self.assertEqual( selection.upstreamPath(), "/plane" )
 		self.assertEqual( selection.upstreamContext()["scene:path"], IECore.InternedStringVectorData( [ "plane" ] ) )
+		self.assertEqual( selection.warning(), "Transform is locked as it is inside \"reference\" which disallows edits to its children" )
 		self.assertFalse( selection.editable() )
 
 if __name__ == "__main__":

python/GafferSceneUITest/TranslateToolTest.py

@@ -670,7 +670,7 @@ def testEditAncestorIfSelectionNotTransformable( self ) :
 		self.assertEqual( len( selection ), 1 )
 		self.assertEqual( selection[0].editTarget(), script["sceneReader"]["transform"] )
 		self.assertEqual( selection[0].path(), "/group" )
-		self.assertEqual( selection[0].warning(), "Editing parent location" )
+		self.assertEqual( selection[0].warning(), "Editing parent location \"/group\"" )
 
 	def testSelectionRefersToFirstPublicPlug( self ) :