View : Add tools container

This holds all the Tools connected to the View, and provides more principled lifetime management between Views and Tools (formerly, `Tool::m_view` could be invalid if the View dies first). Parenting tools into the View is also a step towards the full serialisation of layout state discussed in GafferHQ#50. The eventual idea is that if the UI is a hierarchy of GraphComponents, then it can be serialised using the same mechanism used for node graphs.
johnhaddon · Feb 26, 2022 · 3fe8be2 · 3fe8be2
1 parent a0c46e7
commit 3fe8be2
Show file tree

Hide file tree

Showing 8 changed files with 119 additions and 13 deletions.
diff --git a/Changes.md b/Changes.md
@@ -15,6 +15,7 @@ API
 ---
 
 - Tool : Added support for subclassing in Python.
+- View : Added `tools()` method which returns a container of all connected Tools.
 - Container : Added constructor with a `name` argument.
 
 0.61.3.0 (relative to 0.61.2.0)

diff --git a/include/GafferUI/Tool.h b/include/GafferUI/Tool.h
@@ -40,6 +40,7 @@
 #include "GafferUI/Export.h"
 #include "GafferUI/TypeIds.h"
 
+#include "Gaffer/Container.h"
 #include "Gaffer/Node.h"
 #include "Gaffer/TypedPlug.h"
 
@@ -89,6 +90,11 @@ class GAFFERUI_API Tool : public Gaffer::Node
 		Gaffer::BoolPlug *activePlug();
 		const Gaffer::BoolPlug *activePlug() const;
 
+		/// The Tool constructor automatically parents the tool to
+		/// the`View::toolsContainer()`. After that, the tool may not be
+		/// reparented.
+		bool acceptsParent( const GraphComponent *potentialParent ) const override;
+
 		/// @name Factory
 		///////////////////////////////////////////////////////////////////
 		//@{
@@ -125,12 +131,16 @@ class GAFFERUI_API Tool : public Gaffer::Node
 
 	private :
 
-		View *m_view;
+		/// \todo Remove.
+		View *m_unused;
 
 		static size_t g_firstPlugIndex;
 
 };
 
+using ToolContainer = Gaffer::Container<Gaffer::Node, Tool>;
+IE_CORE_DECLAREPTR( ToolContainer );
+
 } // namespace GafferUI
 
 #endif // GAFFERUI_TOOL_H
diff --git a/include/GafferUI/TypeIds.h b/include/GafferUI/TypeIds.h
@@ -83,6 +83,7 @@ enum TypeId
 	AnimationGadgetTypeId = 110288,
 	AnnotationsGadgetTypeId = 110289,
 	GraphGadgetSetPositionsActionTypeId = 110290,
+	ToolContainerTypeId = 110291,
 
 	LastTypeId = 110450
 };

diff --git a/include/GafferUI/View.h b/include/GafferUI/View.h
@@ -38,6 +38,7 @@
 #ifndef GAFFERUI_VIEW_H
 #define GAFFERUI_VIEW_H
 
+#include "GafferUI/Tool.h"
 #include "GafferUI/ViewportGadget.h"
 
 #include "Gaffer/Node.h"
@@ -116,6 +117,11 @@ class GAFFERUI_API View : public Gaffer::Node
 		ViewportGadget *viewportGadget();
 		const ViewportGadget *viewportGadget() const;
 
+		/// All Tools connected to this View. Use `Tool::registeredTools()` to
+		/// query the available tools and `Tool::create()` to add a tool.
+		ToolContainer *tools();
+		const ToolContainer *tools() const;
+
 		/// @name Factory
 		///////////////////////////////////////////////////////////////////
 		//@{

diff --git a/python/GafferUITest/ToolTest.py b/python/GafferUITest/ToolTest.py
@@ -34,6 +34,8 @@
 #
 ##########################################################################
 
+import gc
+import six
 import unittest
 
 import IECore
@@ -45,27 +47,58 @@
 
 class ToolTest( GafferUITest.TestCase ) :
 
-	def testDerivingInPython( self ) :
+	class TestTool( GafferUI.Tool ) :
 
-		class TestTool( GafferUI.Tool ) :
+		def __init__( self, view, name = "TestTool" ) :
 
-			def __init__( self, view, name = "TestTool" ) :
+			GafferUI.Tool.__init__( self, view, name )
 
-				GafferUI.Tool.__init__( self, view, name )
+	IECore.registerRunTimeTyped( TestTool, typeName = "GafferUITest::TestTool" )
+	GafferUI.Tool.registerTool( "TestTool", GafferUITest.ViewTest.MyView, TestTool )
 
-		IECore.registerRunTimeTyped( TestTool, typeName = "GafferUITest::TestTool" )
-		GafferUI.Tool.registerTool( "TestTool", GafferUITest.ViewTest.MyView, TestTool )
+	def testDerivingInPython( self ) :
 
 		self.assertIn( "TestTool", GafferUI.Tool.registeredTools( GafferUITest.ViewTest.MyView ) )
 
 		view = GafferUITest.ViewTest.MyView()
 		tool = GafferUI.Tool.create( "TestTool", view )
-		self.assertIsInstance( tool, TestTool )
+		self.assertIsInstance( tool, self.TestTool )
 		self.assertIsInstance( tool, GafferUI.Tool )
 		self.assertTrue( tool.view() is view )
 
-		Gaffer.Metadata.registerValue( TestTool, "test", 10 )
+		Gaffer.Metadata.registerValue( self.TestTool, "test", 10 )
 		self.assertEqual( Gaffer.Metadata.value( tool, "test" ), 10 )
 
+	def testToolContainerParenting( self ) :
+
+		# When a tool is created, it is automatically parented to the View's
+		# tool container.
+		view1 = GafferUITest.ViewTest.MyView()
+		tool = GafferUI.Tool.create( "TestTool", view1 )
+		self.assertTrue( tool.parent().isSame( view1["tools"] ) )
+		self.assertTrue( tool.view().isSame( view1 ) )
+		self.assertTrue( tool.acceptsParent( tool.parent() ) )
+
+		# After that, it can't be reparented to another view. This simplifies
+		# tool implementation substantially.
+		view2 = GafferUITest.ViewTest.MyView()
+		self.assertFalse( tool.acceptsParent( view2["tools"] ) )
+
+		# Tool containers don't accept any other children.
+		self.assertFalse( view2["tools"].acceptsChild( Gaffer.Node() ) )
+
+	def testToolOutlivingView( self ) :
+
+		view = GafferUITest.ViewTest.MyView()
+		tool = GafferUI.Tool.create( "TestTool", view )
+		del view
+		while gc.collect() :
+			pass
+		IECore.RefCounted.collectGarbage()
+
+		self.assertIsNone( tool.parent() )
+		with six.assertRaisesRegex( self, RuntimeError, "View not found" ) :
+			tool.view()
+
 if __name__ == "__main__":
 	unittest.main()
diff --git a/src/GafferUI/Tool.cpp b/src/GafferUI/Tool.cpp
@@ -48,10 +48,14 @@ GAFFER_NODE_DEFINE_TYPE( Tool );
 size_t Tool::g_firstPlugIndex = 0;
 
 Tool::Tool( View *view, const std::string &name )
-	:	Node( name ), m_view( view )
+	:	Node( name ), m_unused( nullptr )
 {
 	storeIndexOfNextChild( g_firstPlugIndex );
 	addChild( new BoolPlug( "active", Plug::In, false ) );
+	view->tools()->addChild( this );
+
+	// Silence Clang warning "private field is not used".
+	(void)m_unused;
 }
 
 Tool::~Tool()
@@ -70,12 +74,40 @@ const Gaffer::BoolPlug *Tool::activePlug() const
 
 View *Tool::view()
 {
-	return m_view;
+	return const_cast<View *>( const_cast<const Tool *>( this )->view() );
 }
 
 const View *Tool::view() const
 {
-	return m_view;
+	auto v = ancestor<View>();
+	if( !v )
+	{
+		throw IECore::Exception( "View not found" );
+	}
+	return v;
+}
+
+bool Tool::acceptsParent( const GraphComponent *potentialParent ) const
+{
+	if( !Node::acceptsParent( potentialParent ) )
+	{
+		return false;
+	}
+
+	if( !potentialParent || potentialParent == parent() )
+	{
+		return true;
+	}
+
+	if( typeId() != staticTypeId() )
+	{
+		// Only accept initial parenting performed in our constructor, before
+		// derived class is initialised (and `typeId()` returns something else).
+		return false;
+	}
+
+	// Only accept parenting to ToolContainers.
+	return IECore::runTimeCast<const ToolContainer>( potentialParent );
 }
 
 //////////////////////////////////////////////////////////////////////////
@@ -130,3 +162,14 @@ void Tool::registeredTools( IECore::TypeId viewType, std::vector<std::string> &t
 		viewType = IECore::RunTimeTyped::baseTypeId( viewType );
 	} while( viewType != (IECore::TypeId)NodeTypeId && viewType != IECore::InvalidTypeId );
 }
+
+//////////////////////////////////////////////////////////////////////////
+// ToolContainer
+//////////////////////////////////////////////////////////////////////////
+
+namespace Gaffer
+{
+
+GAFFER_DECLARECONTAINERSPECIALISATIONS( ToolContainer, ToolContainerTypeId )
+
+} // namespace Gaffer
diff --git a/src/GafferUI/View.cpp b/src/GafferUI/View.cpp
@@ -58,7 +58,7 @@ View::View( const std::string &name, Gaffer::PlugPtr inPlug )
 	storeIndexOfNextChild( g_firstPlugIndex );
 	setChild( "in", inPlug );
 	addChild( new Plug( "editScope" ) );
-
+	addChild( new ToolContainer( "tools" ) );
 	setContext( new Context() );
 }
 
@@ -76,6 +76,16 @@ const Gaffer::Plug *View::editScopePlug() const
 	return getChild<Plug>( g_firstPlugIndex + 1 );
 }
 
+ToolContainer *View::tools()
+{
+	return getChild<ToolContainer>( g_firstPlugIndex + 2 );
+}
+
+const ToolContainer *View::tools() const
+{
+	return getChild<ToolContainer>( g_firstPlugIndex + 2 );
+}
+
 Gaffer::EditScope *View::editScope()
 {
 	Plug *p = editScopePlug()->getInput();

diff --git a/src/GafferUIModule/ToolBinding.cpp b/src/GafferUIModule/ToolBinding.cpp
@@ -100,4 +100,6 @@ void GafferUIModule::bindTool()
 		.def( "registeredTools", &registeredTools )
 		.staticmethod( "registeredTools" )
 	;
+
+	GafferBindings::NodeClass<ToolContainer>();
 }