LightTool : Remove selection() and selectionChangedSignal()

These aren't really doing anything : - `selection()` is just a wrapper around ContextAlgo, and wasn't even used consistently for that purpose. - Nothing uses `selectionChangedSignal()` except the unit tests.
GafferHQ · Aug 30, 2024 · 78ad77c · 78ad77c
1 parent ba7309b
commit 78ad77c
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 94 deletions.
diff --git a/Changes.md b/Changes.md
@@ -84,6 +84,7 @@ Breaking Changes
 - View :
   - Changed constructor arguments for View and all subclasses. A ScriptNode must now be passed.
   - Changed `ViewCreator` signature.
+- LightTool : Removed `selection()` and `selectionChangedSignal()`.
 
 Build
 -----

diff --git a/include/GafferSceneUI/LightTool.h b/include/GafferSceneUI/LightTool.h
@@ -65,11 +65,6 @@ class GAFFERSCENEUI_API LightTool : public GafferSceneUI::SelectionTool
 
 		GAFFER_NODE_DECLARE_TYPE( GafferSceneUI::LightTool, LightToolTypeId, SelectionTool );
 
-		const IECore::PathMatcher selection() const;
-
-		using SelectionChangedSignal = Gaffer::Signals::Signal<void (LightTool &)>;
-		SelectionChangedSignal &selectionChangedSignal();
-
 	private :
 
 		GafferScene::ScenePlug *scenePlug();
@@ -96,8 +91,6 @@ class GAFFERSCENEUI_API LightTool : public GafferSceneUI::SelectionTool
 
 		bool m_priorityPathsDirty;
 
-		SelectionChangedSignal m_selectionChangedSignal;
-
 		bool m_dragging;
 
 		Gaffer::Signals::ScopedConnection m_contextChangedConnection;

diff --git a/python/GafferSceneUITest/LightToolTest.py b/python/GafferSceneUITest/LightToolTest.py
@@ -58,35 +58,6 @@ def setUp( self ) :
 		Gaffer.Metadata.registerValue( "light:testLight", "coneAngleParameter", "coneAngle" )
 		Gaffer.Metadata.registerValue( "light:testLight", "penumbraAngleParameter", "penumbraAngle" )
 
-	def testSelection( self ) :
-
-		script = Gaffer.ScriptNode()
-
-		script["light1"] = GafferSceneTest.TestLight()
-		script["light2"] = GafferSceneTest.TestLight()
-
-		script["group"] = GafferScene.Group()
-		script["group"]["in"][0].setInput( script["light1"]["out"] )
-		script["group"]["in"][1].setInput( script["light2"]["out"] )
-
-		view = GafferSceneUI.SceneView( script )
-		view["in"].setInput( script["group"]["out"] )
-
-		tool = GafferSceneUI.LightTool( view )
-		tool["active"].setValue( True )
-
-		self.assertTrue( tool.selection().isEmpty() )
-
-		for selection in [ [ "/group/light" ], ["/group/light", "/group/light1" ], [ "/group/light"] ] :
-			with self.subTest( selection, selection = selection ) :
-				GafferSceneUI.ContextAlgo.setSelectedPaths( view.getContext(), IECore.PathMatcher( selection ) )
-				s = tool.selection()
-				self.assertEqual( len( s.paths() ), len( selection ) )
-
-		GafferSceneUI.ContextAlgo.setSelectedPaths( view.getContext(), IECore.PathMatcher( [] ) )
-		s = tool.selection()
-		self.assertTrue( s.isEmpty() )
-
 	def testSpotLightHandleVisibility( self ) :
 
 		script = Gaffer.ScriptNode()
@@ -126,22 +97,6 @@ def testSpotLightHandleVisibility( self ) :
 		# and not visible with a non-spotlight selection. Currently handles come in as
 		# `GraphComponent` which prevents testing that.
 
-	def testSelectionChangedSignal( self ) :
-
-		script = Gaffer.ScriptNode()
-		script["light"] = GafferSceneTest.TestLight()
-
-		view = GafferSceneUI.SceneView( script )
-		view["in"].setInput( script["light"]["out"] )
-
-		tool = GafferSceneUI.LightTool( view )
-		tool["active"].setValue( True )
-
-		cs = GafferTest.CapturingSlot( tool.selectionChangedSignal() )
-		GafferSceneUI.ContextAlgo.setSelectedPaths( view.getContext(), IECore.PathMatcher( [ "/light" ]  ) )
-		self.assertTrue( len( cs ) )
-		self.assertEqual( cs[0][0], tool )
-
 	def testDeleteNodeCrash( self ) :
 
 		# Make a spotlight and get the LightTool to edit it.

diff --git a/src/GafferSceneUI/LightTool.cpp b/src/GafferSceneUI/LightTool.cpp
@@ -3037,16 +3037,6 @@ LightTool::~LightTool()
 
 }
 
-const PathMatcher LightTool::selection() const
-{
-	return ContextAlgo::getSelectedPaths( view()->getContext() );
-}
-
-LightTool::SelectionChangedSignal &LightTool::selectionChangedSignal()
-{
-	return m_selectionChangedSignal;
-}
-
 ScenePlug *LightTool::scenePlug()
 {
 	return getChild<ScenePlug>( g_firstPlugIndex );
@@ -3075,7 +3065,6 @@ void LightTool::contextChanged( const InternedString &name )
 		m_handleInspectionsDirty = true;
 		m_handleTransformsDirty = true;
 		m_priorityPathsDirty = true;
-		selectionChangedSignal()( *this );
 	}
 }
 
@@ -3102,7 +3091,7 @@ void LightTool::updateHandleInspections()
 
 	m_inspectorsDirtiedConnection.clear();
 
-	const PathMatcher selection = this->selection();
+	const PathMatcher selection = ContextAlgo::getSelectedPaths( view()->getContext() );
 	if( selection.isEmpty() )
 	{
 		for( auto &c : m_handles->children() )
@@ -3176,7 +3165,7 @@ void LightTool::updateHandleTransforms( float rasterScale )
 		return;
 	}
 
-	const PathMatcher selection = this->selection();
+	const PathMatcher selection = ContextAlgo::getSelectedPaths( view()->getContext() );
 	if( selection.isEmpty() )
 	{
 		return;
@@ -3225,10 +3214,6 @@ void LightTool::plugDirtied( const Plug *plug )
 		( plug->ancestor<View>() && plug == view()->editScopePlug() )
 	)
 	{
-		if( !m_dragging )
-		{
-			selectionChangedSignal()( *this );
-		}
 		m_handleInspectionsDirty = true;
 		m_priorityPathsDirty = true;
 	}
@@ -3280,14 +3265,7 @@ void LightTool::preRender()
 		{
 			m_priorityPathsDirty = false;
 			auto sceneGadget = static_cast<SceneGadget *>( view()->viewportGadget()->getPrimaryChild() );
-			if( !selection().isEmpty() )
-			{
-				sceneGadget->setPriorityPaths( ContextAlgo::getSelectedPaths( view()->getContext() ) );
-			}
-			else
-			{
-				sceneGadget->setPriorityPaths( IECore::PathMatcher() );
-			}
+			sceneGadget->setPriorityPaths( ContextAlgo::getSelectedPaths( view()->getContext() ) );
 		}
 	}
 
@@ -3341,8 +3319,6 @@ bool LightTool::dragEnd( Gadget *gadget )
 {
 	m_dragging = false;
 	m_mergeGroupId++;
-	selectionChangedSignal()( *this );
-
 	auto handle = runTimeCast<LightToolHandle>( gadget );
 	handle->handleDragEnd();
 

diff --git a/src/GafferSceneUIModule/ToolBinding.cpp b/src/GafferSceneUIModule/ToolBinding.cpp
@@ -109,12 +109,6 @@ boost::python::list selection( const TransformTool &tool )
 	return result;
 }
 
-IECore::PathMatcher lightToolSelection( const LightTool &tool )
-{
-	IECorePython::ScopedGILRelease gilRelease;
-	return tool.selection();
-}
-
 bool selectionEditable( const TransformTool &tool )
 {
 	IECorePython::ScopedGILRelease gilRelease;
@@ -302,15 +296,9 @@ void GafferSceneUIModule::bindTools()
 		.def( init<SceneView *>() )
 	;
 
-	{
-		scope s = GafferBindings::NodeClass<LightTool>( nullptr, no_init )
-			.def( init<SceneView *>() )
-			.def( "selection", &lightToolSelection )
-			.def( "selectionChangedSignal", &LightTool::selectionChangedSignal, return_internal_reference<1>() )
-		;
-
-		GafferBindings::SignalClass<LightTool::SelectionChangedSignal, GafferBindings::DefaultSignalCaller<LightTool::SelectionChangedSignal>, SelectionChangedSlotCaller<LightTool>>( "SelectionChangedSignal" );
-	}
+	GafferBindings::NodeClass<LightTool>( nullptr, no_init )
+		.def( init<SceneView *>() )
+	;
 
 	{
 		scope s = GafferBindings::NodeClass<LightPositionTool>( nullptr, no_init )