Inspector, TransformTool : Do not edit EditScopes when target is "None"

Changes.md

@@ -29,6 +29,7 @@ Improvements
 - 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.
+- EditScope : Setting a Viewer or Editor's target edit scope to "None" will now prevent edits from being made within any upstream edit scope. To make edits in an edit scope, it must be set as the target.
 
 Fixes
 -----

python/GafferSceneUITest/AttributeInspectorTest.py

@@ -318,6 +318,16 @@ def testSourceAndEdits( self ) :
 			edit = editScopeAttributeTweak
 		)
 
+		# When using no scope, make sure that we don't inadvertently edit the contents of an EditScope.
+
+		self.__assertExpectedResult(
+			self.__inspect( s["editScope2"]["out"], "/light2", "gl:visualiser:scale", None ),
+			source = editScopeAttributeTweak,
+			sourceType = SourceType.Other,
+			editable = False,
+			nonEditableReason = "Source is in an EditScope. Change scope to editScope1 to edit."
+		)
+
 		# If there is a manual tweak outside of an edit scope, make sure we use that with no scope
 		s["independentAttributeTweak"] = GafferScene.AttributeTweaks()
 		s["independentAttributeTweak"]["in"].setInput( s["editScope2"]["out"] )
@@ -895,6 +905,11 @@ def testDisableEdit( self ) :
 		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", None )
+		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 )
+
 		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." )

python/GafferSceneUITest/OptionInspectorTest.py

@@ -651,6 +651,16 @@ def testRenderPassSourceAndEdits( self ) :
 				edit = s["editScope1"]["standardOptions3"]["options"]["renderCamera"]
 			)
 
+			# When using no scope, make sure that we don't inadvertently edit the contents of an EditScope.
+
+			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."
+			)
+
 			# If there is a StandardOptions node outside of an edit scope, make sure we use that with no scope
 
 			s["independentOptions"] = GafferScene.StandardOptions()
@@ -969,6 +979,11 @@ def testDisableEdit( self ) :
 		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"], "render:camera", None )
+		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 )
+
 		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." )

python/GafferSceneUITest/ParameterInspectorTest.py

@@ -247,6 +247,14 @@ def testSourceAndEdits( self ) :
 			editable = True, edit = editScopeShaderTweak
 		)
 
+		# When using no scope, make sure that we don't inadvertently edit the contents of an EditScope.
+
+		self.__assertExpectedResult(
+			self.__inspect( s["editScope2"]["out"], "/light2", "intensity", None ),
+			source = editScopeShaderTweak, sourceType = SourceType.Other,
+			editable = False, nonEditableReason = "Source is in an EditScope. Change scope to editScope1 to edit."
+		)
+
 		# If there is a manual tweak outside of an edit scope make sure we use that with no scope
 
 		s["independentLightTweak"] = GafferScene.ShaderTweaks()
@@ -469,6 +477,11 @@ def testDisableEdit( self ) :
 		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"], "/light", "exposure", None )
+		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 )
+
 		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." )

python/GafferSceneUITest/SetMembershipInspectorTest.py

@@ -293,6 +293,16 @@ def testSourceAndEdits( self ) :
 			edit = s["editScope1"]["setPlane2"]["name"]
 		)
 
+		# When using no scope, make sure that we don't inadvertently edit the contents of an EditScope.
+
+		self.__assertExpectedResult(
+			self.__inspect( s["editScope2"]["out"], "/plane2", "planeSet", None ),
+			source = s["editScope1"]["setPlane2"]["name"],
+			sourceType = SourceType.Other,
+			editable = False,
+			nonEditableReason = "Source is in an EditScope. Change scope to editScope1 to edit."
+		)
+
 		# If there is a manual set node outside of an edit scope, make sure we use that with no scope
 		s["independentSet"] = GafferScene.Set()
 		s["independentSet"]["name"].setValue( "planeSet" )
@@ -673,13 +683,18 @@ def testDisableEdit( self ) :
 			)
 
 			Gaffer.MetadataAlgo.setReadOnly( s["editScope1"], True )
-			inspection = self.__inspect( s["editScope1"]["out"], "/group/plane", "planeSetEditScope", None )
+			inspection = self.__inspect( s["editScope1"]["out"], "/group/plane", "planeSetEditScope", 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/plane", "planeSetEditScope", None )
+			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 )
+
+			inspection = self.__inspect( s["editScope1"]["out"], "/group/plane", "planeSetEditScope", s["editScope1"] )
 			self.assertTrue( inspection.canDisableEdit() )
 			self.assertEqual( inspection.nonDisableableReason(), "" )
 

python/GafferSceneUITest/TransformToolTest.py

@@ -333,5 +333,27 @@ def testDontEditUpstreamOfReference( self ) :
 		self.assertEqual( selection.warning(), "Transform is locked as it is inside \"reference\" which disallows edits to its children" )
 		self.assertFalse( selection.editable() )
 
+	def testDisallowEditingWithEditScopeNone( self ) :
+
+		script = Gaffer.ScriptNode()
+
+		script["plane"] = GafferScene.Plane()
+
+		script["editScope"] = Gaffer.EditScope()
+		script["editScope"].setup( script["plane"]["out"] )
+		script["editScope"]["in"].setInput( script["plane"]["out"] )
+
+		selection = GafferSceneUI.TransformTool.Selection( script["editScope"]["out"], "/plane", script.context(), script["editScope"] )
+		edit = selection.acquireTransformEdit()
+		self.assertTrue( script["editScope"].isAncestorOf( edit.translate ) )
+
+		selection = GafferSceneUI.TransformTool.Selection( script["editScope"]["out"], "/plane", script.context(), None )
+		self.assertFalse( selection.editable() )
+		self.assertEqual( selection.warning(), "Source is in an EditScope. Change scope to editScope to edit" )
+
+		selection = GafferSceneUI.TransformTool.Selection( script["editScope"]["out"], "/plane", script.context(), script["editScope"] )
+		self.assertTrue( selection.editable() )
+		self.assertEqual( selection.warning(), "" )
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferSceneUITest/TranslateToolTest.py

@@ -1043,26 +1043,20 @@ def testTransformInEditScopeButEditScopeOff( self ) :
 
 		GafferSceneUI.ScriptNodeAlgo.setSelectedPaths( script, IECore.PathMatcher( [ "/sphere" ] ) )
 
-		# We want the TranslateTool to pick up and use that edit
-		# even if we haven't told it to use that EditScope.
+		# We don't want the TranslateTool to pick up and use that edit
+		# as we haven't told it to use that EditScope.
 
 		tool = GafferSceneUI.TranslateTool( view )
 		tool["active"].setValue( True )
 
-		self.assertEqual( tool.handlesTransform(), imath.M44f().translate( imath.V3f( 1, 0, 0 ) ) )
 		self.assertEqual( len( tool.selection() ), 1 )
-		self.assertTrue( tool.selectionEditable() )
-		self.assertTrue( tool.selection()[0].editable() )
-		self.assertEqual( tool.selection()[0].acquireTransformEdit( createIfNecessary = False ), transformEdit )
+		self.assertFalse( tool.selectionEditable() )
+		self.assertFalse( tool.selection()[0].editable() )
 		self.assertEqual(
-			tool.selection()[0].editTarget(),
-			transformEdit.translate.ancestor( Gaffer.Spreadsheet.RowPlug )
+			tool.selection()[0].warning(),
+			"Source is in an EditScope. Change scope to editScope to edit"
 		)
 
-		tool.translate( imath.V3f( 0, 1, 0 ) )
-		self.assertEqual( tool.handlesTransform(), imath.M44f().translate( imath.V3f( 1, 1, 0 ) ) )
-		self.assertEqual( transformEdit.translate.getValue(), imath.V3f( 1, 1, 0 ) )
-
 	def testNonEditableSelections( self ) :
 
 		script = Gaffer.ScriptNode()

python/GafferUI/EditScopeUI.py

@@ -169,7 +169,7 @@ def getToolTip( self ) :
 
 		editScope = self.__editScope()
 		if editScope is None :
-			return "Edits will be made using the last relevant node, including nodes not in any EditScope."
+			return "Edits will be made using the last relevant node found outside of an edit scope.\n\nTo make an edit in an edit scope, choose it from the menu."
 
 		unusableReason = self.__unusableReason( editScope )
 		if unusableReason :

src/GafferSceneUI/Inspector.cpp

@@ -289,7 +289,21 @@ void Inspector::inspectHistoryWalk( const GafferScene::SceneAlgo::History *histo
 
 					// See if we can use it for editing
 
-					if( !result->m_editScope || node->ancestor<EditScope>() == result->m_editScope )
+					if( !result->m_editScope && node->ancestor<EditScope>() )
+					{
+						// 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
+						// inside _any_ EditScope.
+						result->m_editFunction = fmt::format(
+							"Source is in an EditScope. Change scope to {} to edit.",
+							node->ancestor<EditScope>()->relativeName( node->ancestor<EditScope>()->scriptNode() )
+						);
+						result->m_disableEditFunction = fmt::format(
+							"Source is in an EditScope. Change scope to {} to disable.",
+							node->ancestor<EditScope>()->relativeName( node->ancestor<EditScope>()->scriptNode() )
+						);
+					}
+					else if( !result->m_editScope || node->ancestor<EditScope>() == result->m_editScope )
 					{
 						const std::string nonEditableReason = ::nonEditableReason( result->m_source.get() );
 

src/GafferSceneUI/TransformTool.cpp

@@ -471,11 +471,22 @@ void TransformTool::Selection::initWalk( const GafferScene::SceneAlgo::History *
 		// First, check for a supported node in this history entry
 		initFromSceneNode( history );
 
-		// If we found a node to edit here and the user has requested a
-		// specific scope, check if the edit is in it.
-		if( m_upstreamScene && m_editScope )
+		if( m_upstreamScene )
 		{
-			editScopeFound = m_upstreamScene->ancestor<EditScope>() == m_editScope;
+			const auto upstreamEditScope = m_upstreamScene->ancestor<EditScope>();
+			if( m_editScope )
+			{
+				// If we found a node to edit here and the user has requested a
+				// specific scope, check if the edit is in it.
+				editScopeFound = upstreamEditScope == m_editScope;
+			}
+			else if( upstreamEditScope )
+			{
+				// We don't allow editing if the user hasn't requested a specific scope
+				// and the upstream edit is inside an EditScope.
+				m_warning = "Source is in an EditScope. Change scope to " + displayName( upstreamEditScope ) + " to edit";
+				m_editable = false;
+			}
 		}
 	}