Merge pull request #5991 from johnhaddon/catalogueDescriptionHeight

Small Catalogue improvements
GafferHQ · Aug 7, 2024 · f60e1ea · f60e1ea
2 parents bce99ca + a34d35a
commit f60e1ea
Show file tree

Hide file tree

Showing 4 changed files with 130 additions and 63 deletions.
diff --git a/Changes.md b/Changes.md
@@ -13,6 +13,7 @@ Improvements
   - Green : Render
   - Blue : Proxy
   - Red : Guide
+- Catalogue : Added a handle for controlling the relative sizes of the listing and image property widgets.
 
 Fixes
 -----
@@ -21,6 +22,10 @@ Fixes
 - WidgetAlgo : Fixed issue preventing `grab()` from capturing popup menus on Windows.
 - ShowURL : Fixed opening of "file://" URLs on Windows (#5861).
 - Shader : Fixed "Revert to Defaults" menu item in NodeEditor (#1862).
+- Catalogue :
+  - Fixed bug which allowed locked Catalogues to be edited.
+  - Fixed NodeEditor update when the first image is added or the last image is removed.
+- NameWidget : Fixed bug which allowed plugs on locked nodes to be renamed.
 
 API
 ---

diff --git a/python/GafferImageUI/CatalogueUI.py b/python/GafferImageUI/CatalogueUI.py
@@ -721,73 +721,82 @@ class ImageListing( GafferUI.PlugValueWidget ) :
 
 	def __init__( self, plug, **kw ) :
 
-		self.__column = GafferUI.ListContainer( spacing = 4 )
+		splitContainer = GafferUI.SplitContainer()
 
-		GafferUI.PlugValueWidget.__init__( self, self.__column, plug, **kw )
+		GafferUI.PlugValueWidget.__init__( self, splitContainer, plug, **kw )
 
-		with self.__column :
+		with splitContainer :
 
-			self.__pathListing = GafferUI.PathListingWidget(
-				_ImagesPath( self.__images(), [] ),
-				columns = self.__listingColumns(),
-				selectionMode = GafferUI.PathListingWidget.SelectionMode.Rows,
-				sortable = False,
-				horizontalScrollMode = GafferUI.ScrollMode.Automatic
-			)
-			self.__pathListing.setDragPointer( "" )
-			self.__pathListing.setHeaderVisible( True )
-			self.__pathListing.selectionChangedSignal().connect(
-				Gaffer.WeakMethod( self.__pathListingSelectionChanged ), scoped = False
-			)
-			self.__pathListing.dragEnterSignal().connect(
-				Gaffer.WeakMethod( self.__pathListingDragEnter ), scoped = False
-			)
-			self.__pathListing.dragLeaveSignal().connect(
-				Gaffer.WeakMethod( self.__pathListingDragLeave ), scoped = False
-			)
-			self.__pathListing.dragMoveSignal().connect(
-				Gaffer.WeakMethod( self.__pathListingDragMove ), scoped = False
-			)
-			self.__pathListing.dropSignal().connect(
-				Gaffer.WeakMethod( self.__pathListingDrop ), scoped = False
-			)
-			self.keyPressSignal().connect( Gaffer.WeakMethod( self.__keyPress ), scoped = False )
+			with GafferUI.ListContainer( spacing = 4 ) :
 
-			with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Horizontal, spacing = 4 ) as self.__buttonRow :
+				self.__pathListing = GafferUI.PathListingWidget(
+					_ImagesPath( self.__images(), [] ),
+					columns = self.__listingColumns(),
+					selectionMode = GafferUI.PathListingWidget.SelectionMode.Rows,
+					sortable = False,
+					horizontalScrollMode = GafferUI.ScrollMode.Automatic
+				)
+				self.__pathListing.setDragPointer( "" )
+				self.__pathListing.setHeaderVisible( True )
+				self.__pathListing.selectionChangedSignal().connect(
+					Gaffer.WeakMethod( self.__pathListingSelectionChanged ), scoped = False
+				)
+				self.__pathListing.dragEnterSignal().connect(
+					Gaffer.WeakMethod( self.__pathListingDragEnter ), scoped = False
+				)
+				self.__pathListing.dragLeaveSignal().connect(
+					Gaffer.WeakMethod( self.__pathListingDragLeave ), scoped = False
+				)
+				self.__pathListing.dragMoveSignal().connect(
+					Gaffer.WeakMethod( self.__pathListingDragMove ), scoped = False
+				)
+				self.__pathListing.dropSignal().connect(
+					Gaffer.WeakMethod( self.__pathListingDrop ), scoped = False
+				)
+				self.__pathListing.getPath().pathChangedSignal().connect(
+					Gaffer.WeakMethod( self.__pathChanged ), scoped = False
+				)
+				self.keyPressSignal().connect( Gaffer.WeakMethod( self.__keyPress ), scoped = False )
+
+				with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Horizontal, spacing = 4 ) as self.__buttonRow :
+
+					addButton = GafferUI.Button( image = "pathChooser.png", hasFrame = False, toolTip = "Load image" )
+					addButton.clickedSignal().connect( Gaffer.WeakMethod( self.__addClicked ), scoped = False )
 
-				addButton = GafferUI.Button( image = "pathChooser.png", hasFrame = False, toolTip = "Load image" )
-				addButton.clickedSignal().connect( Gaffer.WeakMethod( self.__addClicked ), scoped = False )
+					self.__duplicateButton = GafferUI.Button( image = "duplicate.png", hasFrame = False, toolTip = "Duplicate selected image, hold <kbd>alt</kbd> to view copy. [<kbd>Ctrl-D</kbd>]" )
+					self.__duplicateButton.setEnabled( False )
+					self.__duplicateButton.clickedSignal().connect( Gaffer.WeakMethod( self.__duplicateClicked ), scoped = False )
 
-				self.__duplicateButton = GafferUI.Button( image = "duplicate.png", hasFrame = False, toolTip = "Duplicate selected image, hold <kbd>alt</kbd> to view copy. [<kbd>Ctrl-D</kbd>]" )
-				self.__duplicateButton.setEnabled( False )
-				self.__duplicateButton.clickedSignal().connect( Gaffer.WeakMethod( self.__duplicateClicked ), scoped = False )
+					self.__exportButton = GafferUI.Button( image = "export.png", hasFrame = False, toolTip = "Export selected image" )
+					self.__exportButton.setEnabled( False )
+					self.__exportButton.clickedSignal().connect( Gaffer.WeakMethod( self.__exportClicked ), scoped = False )
 
-				self.__exportButton = GafferUI.Button( image = "export.png", hasFrame = False, toolTip = "Export selected image" )
-				self.__exportButton.setEnabled( False )
-				self.__exportButton.clickedSignal().connect( Gaffer.WeakMethod( self.__exportClicked ), scoped = False )
+					self.__extractButton = GafferUI.Button( image = "extract.png", hasFrame = False, toolTip = "Create CatalogueSelect node for selected image" )
+					self.__extractButton.setEnabled( False )
+					self.__extractButton.clickedSignal().connect( Gaffer.WeakMethod( self.__extractClicked ), scoped = False )
 
-				self.__extractButton = GafferUI.Button( image = "extract.png", hasFrame = False, toolTip = "Create CatalogueSelect node for selected image" )
-				self.__extractButton.setEnabled( False )
-				self.__extractButton.clickedSignal().connect( Gaffer.WeakMethod( self.__extractClicked ), scoped = False )
+					GafferUI.Spacer( imath.V2i( 0 ), parenting = { "expand" : True } )
 
-				GafferUI.Spacer( imath.V2i( 0 ), parenting = { "expand" : True } )
+					self.__removeButton = GafferUI.Button( image = "delete.png", hasFrame = False, toolTip = "Remove selected image [<kbd>Delete</kbd>]" )
+					self.__removeButton.setEnabled( False )
+					self.__removeButton.clickedSignal().connect( Gaffer.WeakMethod( self.__removeClicked ), scoped = False )
 
-				self.__removeButton = GafferUI.Button( image = "delete.png", hasFrame = False, toolTip = "Remove selected image [<kbd>Delete</kbd>]" )
-				self.__removeButton.setEnabled( False )
-				self.__removeButton.clickedSignal().connect( Gaffer.WeakMethod( self.__removeClicked ), scoped = False )
+				GafferUI.Spacer( size = imath.V2i( 2 ) )
 
-			GafferUI.Divider()
+			with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Vertical, spacing = 4 ) :
 
-			with GafferUI.Collapsible( label = "Image Properties", collapsed = False ) :
+				GafferUI.Spacer( size = imath.V2i( 2 ) )
 
-				with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Vertical, spacing = 4 ) :
+				GafferUI.Label( "<h4>Image Properties</h4>" )
 
-					with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Horizontal, spacing = 4 ) :
-						GafferUI.Label( "Name" )
-						self.__nameWidget = GafferUI.NameWidget( graphComponent = None )
+				GafferUI.Spacer( size = imath.V2i( 2 ) )
 
-					GafferUI.Label( "Description" )
-					self.__descriptionWidget = GafferUI.MultiLineStringPlugValueWidget( plug = None )
+				with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Horizontal, spacing = 4 ) :
+					GafferUI.Label( "Name" )
+					self.__nameWidget = GafferUI.NameWidget( graphComponent = None )
+
+				GafferUI.Label( "Description" )
+				self.__descriptionWidget = GafferUI.MultiLineStringPlugValueWidget( plug = None )
 
 		self.__mergeGroupId = 0
 
@@ -824,7 +833,19 @@ def _updateFromValues( self, values, exception ) :
 			self.__descriptionWidget.setPlug( None )
 			self.__nameWidget.setGraphComponent( None )
 
-		self.__column.setEnabled( self._editable() )
+	def _updateFromEditable( self ) :
+
+		self.__pathListing.setEnabled( self._editable() )
+		self.__buttonRow.setEnabled( self._editable() )
+		# No need to manage editability of `self.__nameWidget` and
+		# `self.__descriptionWidget` because they deal with that
+		# internally.
+
+	def __pathChanged( self, path ) :
+
+		# `_updateFromValues()` is sensitive to the contents of the image path so
+		# we need to do an update when it changes.
+		self._requestUpdateFromValues()
 
 	def __plugMetadataValueChanged( self, plug, key, reason ) :
 

diff --git a/python/GafferUI/NameWidget.py b/python/GafferUI/NameWidget.py
@@ -51,7 +51,7 @@ def __init__( self, graphComponent, **kw ) :
 
 		self._qtWidget().setValidator( _Validator( self._qtWidget() ) )
 
-		self.__graphComponent = None
+		self.__graphComponent = False # Sentinel that forces `setGraphComponent()` to update
 		self.setGraphComponent( graphComponent )
 
 		self.editingFinishedSignal().connect( Gaffer.WeakMethod( self.__setName ), scoped = False )
@@ -62,23 +62,23 @@ def setGraphComponent( self, graphComponent ) :
 			return
 
 		self.__graphComponent = graphComponent
+		self.__nameChangedConnection = None
+		self.__nodeMetadataChangedConnection = None
+		self.__plugMetadataChangedConnection = None
+
 		if self.__graphComponent is not None :
 			self.__nameChangedConnection = self.__graphComponent.nameChangedSignal().connect(
 				Gaffer.WeakMethod( self.__setText ), scoped = True
 			)
-			if isinstance( self.__graphComponent, Gaffer.Node ) :
-				self.__metadataChangedConnection = Gaffer.Metadata.nodeValueChangedSignal().connect(
+			if isinstance( self.__graphComponent, ( Gaffer.Node, Gaffer.Plug ) ) :
+				self.__nodeMetadataChangedConnection = Gaffer.Metadata.nodeValueChangedSignal().connect(
 					Gaffer.WeakMethod( self.__nodeMetadataChanged ), scoped = True
 				)
-			elif isinstance( self.__graphComponent, Gaffer.Plug ) :
-				self.__metadataChangedConnection = Gaffer.Metadata.plugValueChangedSignal( self.__graphComponent.node() ).connect(
+
+			if isinstance( self.__graphComponent, Gaffer.Plug ) :
+				self.__plugMetadataChangedConnection = Gaffer.Metadata.plugValueChangedSignal( self.__graphComponent.node() ).connect(
 					Gaffer.WeakMethod( self.__plugMetadataChanged ), scoped = True
 				)
-			else :
-				self.__metadataChangedConnection = None
-		else :
-			self.__nameChangedConnection = None
-			self.__metadataChangedConnection = None
 
 		self.__setText()
 		self.__updateEditability()

diff --git a/python/GafferUITest/NameWidgetTest.py b/python/GafferUITest/NameWidgetTest.py
@@ -70,5 +70,46 @@ def testSetGraphComponentToNone( self ) :
 		self.assertEqual( w.getText(), "" )
 		self.assertTrue( w.getGraphComponent() is None )
 
+	def testEditability( self ) :
+
+		node = Gaffer.Node()
+		node["user"]["p1"] = Gaffer.IntPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
+		node["user"]["p2"] = Gaffer.IntPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
+
+		nodeNameWidget = GafferUI.NameWidget( node )
+		plugNameWidget = GafferUI.NameWidget( node["user"]["p1"] )
+
+		self.assertTrue( nodeNameWidget.getEditable() )
+		self.assertTrue( plugNameWidget.getEditable() )
+
+		Gaffer.Metadata.registerValue( node, "readOnly", True )
+
+		self.assertFalse( nodeNameWidget.getEditable() )
+		self.assertFalse( plugNameWidget.getEditable() )
+
+		Gaffer.Metadata.registerValue( node, "readOnly", False )
+
+		self.assertTrue( nodeNameWidget.getEditable() )
+		self.assertTrue( plugNameWidget.getEditable() )
+
+		Gaffer.Metadata.registerValue( node["user"]["p2"], "readOnly", True )
+
+		self.assertTrue( nodeNameWidget.getEditable() )
+		self.assertTrue( plugNameWidget.getEditable() )
+
+		Gaffer.Metadata.registerValue( node["user"]["p1"], "readOnly", True )
+
+		self.assertTrue( nodeNameWidget.getEditable() )
+		self.assertFalse( plugNameWidget.getEditable() )
+
+		nodeNameWidget.setGraphComponent( None )
+		self.assertFalse( nodeNameWidget.getEditable() )
+
+		plugNameWidget.setGraphComponent( None )
+		self.assertFalse( plugNameWidget.getEditable() )
+
+		noneWidget = GafferUI.NameWidget( graphComponent = None )
+		self.assertFalse( noneWidget.getEditable() )
+
 if __name__ == "__main__":
 	unittest.main()