Light Linking Editor

Changes.md

@@ -1,11 +1,27 @@
 1.7.x.x (relative to 1.7.0.0a10)
 =======
 
+Features
+--------
+
+- LightLinkingEditor : Added a new editor UI for inspecting and editing light links.
+
 Fixes
 -----
 
 - MenuBar : Made the main window menu extension button more visible. This button is shown when the window is not wide enough to show all menu items.
 - LightUI : Fixed `nodule:type` metadata lookups. Previously these ignored metadata registered to `light:{name}:{parameterName}`.
+- LightEditor : Fixed bug preventing the "Copy Path" menu item from appearing when the current selection contained locations not shown in the LightEditor.
+- PathListingWidget : Paths dragged from a PathListingWidget now preserve the order in which they are displayed.
+- SetExpressionAlgo : Fixed invalid set expressions returned by `exclude()` when the set expression to be excluded contains only whitespace [^1].
+- PlugLayout : `<layoutName>:width` metadata is now correctly reapplied to widgets with labels when a PlugLayout is rebuilt.
+
+API
+---
+
+- SetExpressionAlgo : Added `remove` [^1].
+
+[^1]: Improvement to a feature introduced in `1.7.0.0a1`, so should be omitted from final `1.7.0.0` release notes.
 
 1.7.0.0a10 (relative to 1.7.0.0a9)
 ==========

include/Gaffer/SetExpressionAlgo.h

@@ -91,5 +91,11 @@ GAFFER_API std::string include( const std::string &setExpression, const std::str
 /// the result simplified. Returns "" if `setExpression` is empty or would simplify
 /// to an empty expression.
 GAFFER_API std::string exclude( const std::string &setExpression, const std::string &exclusions );
+/// Returns a set expression with `removals` removed from `setExpression` and the
+/// result simplified. Unlike `exclude()`, which both removes and subtracts `exclusions`,
+/// `remove()` only removes the matching operations. For example `remove( "A B C", "B" )`
+/// returns `"A C"` rather than `"A C - B"`. Returns "" if `setExpression` is empty
+/// or would simplify to an empty expression.
+GAFFER_API std::string remove( const std::string &setExpression, const std::string &removals );
 
 } // namespace Gaffer::SetExpressionAlgo

python/GafferSceneUI/RenderPassEditor.py

@@ -187,7 +187,7 @@ def __optionColumnCreator( cls, optionName, section, columnName = None ) :
 		if section == "Favourites" :
 			optionLabel = Gaffer.Metadata.value( "option:" + optionName, "label" )
 		else :
-			optionLabel = Gaffer.Metadata.value( "option:" + optionName, f"columnLayout:label" ) or Gaffer.Metadata.value( "option:" + optionName, "label" )
+			optionLabel = Gaffer.Metadata.value( "option:" + optionName, "columnLayout:label" ) or Gaffer.Metadata.value( "option:" + optionName, "label" )
 
 		if not columnName :
 			columnName = optionLabel or optionName.split( ":" )[-1]

python/GafferSceneUI/SceneEditor.py

@@ -324,6 +324,9 @@ def _setFilterExpression( cls, filterValue, setFilterValue ) :
 # _SetFilterPlugValueWidget
 # =========================
 
+# Supported plug metadata :
+#
+#  - "setFilterPlugValueWidget:excludedSetNames" : A list of set names that shouldn't be shown in the menu.
 class _SetFilterPlugValueWidget( GafferUI.PlugValueWidget ) :
 
 	def __init__( self, plug, **kw ) :
@@ -341,6 +344,7 @@ def __init__( self, plug, **kw ) :
 
 		self.__lastNonDefaultValue = None
 		self.__availableSetNames = []
+		self.__excludedSetNames = set()
 
 	def _auxiliaryPlugs( self, plug ) :
 
@@ -376,13 +380,16 @@ def _updateFromValues( self, values, exception ) :
 		self.__button.setImage( "setFilter{}.png".format( "On" if value else "Off" ) )
 		self.__availableSetNames = values[0]["setNames"]
 
+	def _updateFromMetadata( self ) :
+
+		self.__excludedSetNames = set( Gaffer.Metadata.value( self.getPlug(), "setFilterPlugValueWidget:excludedSetNames" ) or [] )
+
 	def __setsMenuDefinition( self ) :
 
 		m = IECore.MenuDefinition()
 
-		availableSets = set( self.__availableSetNames )
-
-		builtInSets = { "__lights", "__lightFilters", "__cameras", "__coordinateSystems" }
+		availableSets = set( self.__availableSetNames ) - self.__excludedSetNames
+		builtInSets = { "__lights", "__lightFilters", "__cameras", "__coordinateSystems" } - self.__excludedSetNames
 		selectedSets = set( self.getPlug().getValue().split() )
 
 		m.append(
@@ -398,6 +405,11 @@ def __setsMenuDefinition( self ) :
 
 		m.append( "/EnabledDivider", { "divider" : True } )
 
+		if not ( availableSets | selectedSets | builtInSets ) :
+
+			m.append( "/No sets available", { "active" : False } )
+			return m
+
 		def item( setName ) :
 
 			updatedSets = set( selectedSets )
@@ -422,7 +434,7 @@ def item( setName ) :
 		for s in sorted( availableSets | selectedSets ) :
 			if s in builtInSets :
 				continue
-			if not haveDivider :
+			if builtInSets and not haveDivider :
 				m.append( "/BuiltInDivider", { "divider" : True } )
 				haveDivider = True
 			m.append( "/" + pathFn( s ), item( s ) )

python/GafferSceneUI/SceneHistoryUI.py

@@ -65,7 +65,7 @@ def connectToEditor( editor ) :
 
 	if isinstance( editor, GafferUI.Viewer ) :
 		editor.keyPressSignal().connect( __viewerKeyPress )
-	elif isinstance( editor, GafferSceneUI.HierarchyView ) or isinstance( editor, GafferSceneUI.LightEditor ) :
+	elif isinstance( editor, ( GafferSceneUI.HierarchyView, GafferSceneUI.LightEditor, GafferSceneUI.LightLinkingEditor ) ) :
 		editor.keyPressSignal().connect( __hierarchyViewKeyPress )
 	elif isinstance( editor, GafferUI.NodeEditor ) :
 		editor.keyPressSignal().connect( __nodeEditorKeyPress )

python/GafferSceneUI/_InspectorColumn.py

@@ -974,10 +974,9 @@ def __orderedSelection( pathListing ) :
 		for path in selection.paths() :
 			rows.setdefault( path, [] ).append( column )
 
-	matrix = []
-	orderedPaths = pathListing.visualOrder( IECore.PathMatcher( list( rows.keys() ) ) )
-	for path, columns in sorted( rows.items(), key = lambda item : orderedPaths.index( item[0] ) ) :
-		matrix.append( ( path, columns ) )
+	matrix = [
+		( path, rows[path] ) for path in pathListing.visualOrder( IECore.PathMatcher( list( rows.keys() ) ) )
+	]
 
 	return matrix
 

python/GafferSceneUI/__init__.py

@@ -53,6 +53,7 @@
 from .SetEditor import SetEditor
 from .RenderPassEditor import RenderPassEditor
 from .AttributeEditor import AttributeEditor
+from .LightLinkingEditor import LightLinkingEditor
 from . import SceneHistoryUI
 from . import EditScopeUI
 from . import _InspectorColumn

python/GafferSceneUITest/InspectorColumnTest.py

@@ -790,3 +790,45 @@ def testCancellation( self ) :
 
 		with self.assertRaises( IECore.Cancelled ) :
 			column.cellData( path, canceller )
+
+	def testInvalidPathsInSelection( self ) :
+
+		a = Gaffer.ApplicationRoot()
+		s = Gaffer.ScriptNode()
+		a["scripts"]["testScript"] = s
+		self.__testScene( s )
+
+		w = GafferUI.PathListingWidget(
+			GafferScene.ScenePath( s["parent"]["out"], Gaffer.Context(), "/" ),
+			columns = [
+				GafferSceneUI.Private.InspectorColumn( GafferSceneUI.Private.AttributeInspector( s["parent"]["out"], None, "test:string" ) ),
+			],
+			selectionMode = GafferUI.PathListingWidget.SelectionMode.Cells,
+			displayMode = GafferUI.PathListingWidget.DisplayMode.Tree
+		)
+
+		e = InspectorColumnTest.TestEditor( s )
+		e.addPathListing( w )
+
+		# Ensure a selection that contains paths not represented by this
+		# PathListingWidget does not error, and instead only returns data
+		# for the valid paths.
+
+		w.setSelection( [ IECore.PathMatcher( [ "/sphere", "/notAPath" ] ) ] )
+		_GafferUI._pathModelWaitForPendingUpdates( GafferUI._qtAddress( w._qtWidget().model() ) )
+
+		self.assertEqual(
+			GafferSceneUI._InspectorColumn._dataFromPathListingOrReason( w ),
+			IECore.StringData( "sphere" )
+		)
+
+		# A selection that only contains paths not represented by this
+		# PathListingWidget should be treated as no selection.
+
+		w.setSelection( [ IECore.PathMatcher( [ "/notAPath" ] ) ] )
+		_GafferUI._pathModelWaitForPendingUpdates( GafferUI._qtAddress( w._qtWidget().model() ) )
+
+		self.assertEqual(
+			GafferSceneUI._InspectorColumn._dataFromPathListingOrReason( w ),
+			"No selection"
+		)

python/GafferTest/SetExpressionAlgoTest.py

@@ -764,7 +764,86 @@ def testExclude( self ) :
 				testPaths.removePaths( Gaffer.SetExpressionAlgo.evaluateSetExpression( exclusions, s ) )
 				self.assertEqual( Gaffer.SetExpressionAlgo.evaluateSetExpression( excluded, s ), testPaths )
 
-	def testIncludeAndExcludeSelf( self ) :
+	def testRemove( self ) :
+
+		for base, removals, result in [
+			( "", "", "" ),
+			( "A", "", "A" ),
+			( "", "A", "" ),
+			( "A", "A", "" ),
+			( "A", "B", "A" ),
+			( "A A", "A", "" ),
+
+			( "A B", "A", "B" ),
+			( "A B", "B", "A" ),
+			( "A B", "A B", "" ),
+			( "A B", "B A", "" ),
+			( "A B B", "B", "A" ),
+			( "A B C", "B", "A C" ),
+			( "A B C", "A C", "B" ),
+			( "A B C", "A B C", "" ),
+			( "A B C", "D", "A B C" ),
+
+			( "/a /b", "/a", "/b" ),
+			( "A /b", "/b", "A" ),
+			( "A /b", "A", "/b" ),
+
+			# `-`, `in` and `containing` only remove from their left-hand side.
+			( "A - B", "A", "" ),
+			( "A - B", "B", "A - B" ),
+			( "A - B", "C", "A - B" ),
+			( "A B - C", "A", "B - C" ),
+			( "A B - C", "B", "A" ),
+			( "A B - C", "C", "A B - C" ),
+			( "(A B) - C", "A", "B - C" ),
+			( "(A B) - C", "B", "A - C" ),
+			( "(A B) - C", "C", "(A B) - C" ),
+			( "(A B) - C", "A B", "" ),
+			( "(A B) - (C D)", "A", "B - (C D)" ),
+			( "(A B) - (C D)", "C", "(A B) - (C D)" ),
+			( "(A B) - (C D)", "A B", "" ),
+
+			( "A in B", "A", "" ),
+			( "A in B", "B", "A in B" ),
+			( "A in B", "C", "A in B" ),
+			( "(A B) in C", "A", "B in C" ),
+			( "(A B) in C", "B", "A in C" ),
+			( "(A B) in C", "C", "(A B) in C" ),
+			( "(A B) in C", "A B", "" ),
+
+			( "A containing B", "A", "" ),
+			( "A containing B", "B", "A containing B" ),
+			( "(A B) containing C", "A", "B containing C" ),
+			( "(A B) containing C", "B", "A containing C" ),
+			( "(A B) containing C", "C", "(A B) containing C" ),
+			( "(A B) containing C", "A B", "" ),
+
+			( "A & B", "A", "" ),
+			( "A & B", "B", "" ),
+			( "A & B", "C", "A & B" ),
+			( "A & B", "A B", "" ),
+			( "A & B", "A & B", "" ),
+			( "A & B C", "A", "C" ),
+			( "A & B C", "C", "A & B" ),
+			( "A & B C", "A B", "C" ),
+			( "A & B C", "A & B", "C" ),
+
+			# Wildcard tokens are matched literally.
+			( "A A*", "A", "A*" ),
+			( "A A*", "A*", "A" ),
+			( "A*", "A", "A*" ),
+
+		] :
+			with self.subTest( base = base, removals = removals, result = result ) :
+
+				removed = Gaffer.SetExpressionAlgo.remove( base, removals )
+				self.assertEqual( removed, result )
+				# The new set expression should be already simplified.
+				self.assertEqual( removed, Gaffer.SetExpressionAlgo.simplify( removed ) )
+				# Removing `removals` a second time should result in no change to the expression.
+				self.assertEqual( removed, Gaffer.SetExpressionAlgo.remove( removed, removals ) )
+
+	def testIncludeExcludeAndRemoveSelf( self ) :
 
 		for expression in (
 			"",
@@ -801,3 +880,13 @@ def testIncludeAndExcludeSelf( self ) :
 			with self.subTest( expression = expression ) :
 				self.assertEqual( Gaffer.SetExpressionAlgo.exclude( expression, expression ), "" )
 				self.assertEqual( Gaffer.SetExpressionAlgo.include( expression, expression ), Gaffer.SetExpressionAlgo.simplify( expression ) )
+				self.assertEqual( Gaffer.SetExpressionAlgo.remove( expression, expression ), "" )
+
+	def testEmptyAndWhitespaceEdits( self ) :
+
+		for expression in ( "", "A", "A B", "A - B", "A & B", "A in B" ) :
+			for edit in ( "", " ", "\t", "\n", " \t\n " ) :
+				with self.subTest( expression = expression, edit = edit ) :
+					self.assertEqual( Gaffer.SetExpressionAlgo.include( expression, edit ), expression )
+					self.assertEqual( Gaffer.SetExpressionAlgo.exclude( expression, edit ), expression )
+					self.assertEqual( Gaffer.SetExpressionAlgo.remove( expression, edit ), expression )

python/GafferUI/PathListingWidget.py

@@ -775,7 +775,7 @@ def __dragBegin( self, widget, event ) :
 
 		if selection[0].match( str( path ) ) & IECore.PathMatcher.Result.ExactMatch :
 			GafferUI.Pointer.setCurrent( self.__dragPointer )
-			return IECore.StringVectorData( selection[0].paths() )
+			return IECore.StringVectorData( self.visualOrder( selection[0] ) )
 
 		index = self.__indexAt( event.line.p0 )
 		if index is not None :

python/GafferUI/PlugLayout.py

@@ -319,8 +319,10 @@ def __updateLayout( self ) :
 				self.__widgets[item] = widget
 			else :
 				widget = self.__widgets[item]
-				if self.__itemMetadataValue( item, "width" ) :
-					widget._qtWidget().setFixedWidth( self.__itemMetadataValue( item, "width" ) )
+				self.__setWidthFromMetadata(
+					widget.plugValueWidget() if isinstance( widget, GafferUI.PlugWidget ) else widget,
+					item
+				)
 
 			if widget is None :
 				continue

python/GafferUI/_StyleSheet.py

@@ -1314,6 +1314,7 @@ def styleColor( key ) :
 	*[gafferClass="GafferSceneUI.AttributeEditor"] QTreeView::item,
 	*[gafferClass="GafferSceneUI.SceneInspector"] QTreeView::item,
 	*[gafferClass="GafferSceneUI._HistoryWindow"] QTreeView::item,
+	*[gafferClass="GafferSceneUI.LightLinkingEditor"] QTreeView::item,
 	*[gafferClass="GafferSceneUI.SetEditor"] QTreeView::item {
 		height: 20px;
 		padding-top: 0px;

python/GafferUITest/PlugLayoutTest.py

@@ -411,3 +411,74 @@ def iNameFilterFunction( plug ) :
 		self.assertTrue( l.plugValueWidget( n["i2"] ).visible() )
 		self.assertTrue( l.plugValueWidget( n["f"] ).visible() )
 		self.assertTrue( l.customWidget( "test" ).visible() )
+
+	def testWidthMetadata( self ) :
+
+		n = Gaffer.Node()
+		n["withLabel"] = Gaffer.IntPlug()
+		n["withoutLabel"] = Gaffer.IntPlug()
+
+		Gaffer.Metadata.registerValue( n["withLabel"], "layout:width", 100 )
+		Gaffer.Metadata.registerValue( n["withoutLabel"], "layout:width", 100 )
+		Gaffer.Metadata.registerValue( n["withoutLabel"], "label", "" )
+
+		l = GafferUI.PlugLayout( n, orientation = GafferUI.ListContainer.Orientation.Horizontal )
+
+		withLabel = l.plugValueWidget( n["withLabel"] )
+		self.assertEqual( withLabel._qtWidget().minimumWidth(), 100 )
+		self.assertEqual( withLabel._qtWidget().maximumWidth(), 100 )
+
+		plugWidget = withLabel.ancestor( GafferUI.PlugWidget )
+		self.assertIsNotNone( plugWidget )
+		self.assertNotEqual( plugWidget._qtWidget().maximumWidth(), 100 )
+
+		withoutLabel = l.plugValueWidget( n["withoutLabel"] )
+		self.assertIsNone( withoutLabel.ancestor( GafferUI.PlugWidget ) )
+		self.assertEqual( withoutLabel._qtWidget().minimumWidth(), 100 )
+		self.assertEqual( withoutLabel._qtWidget().maximumWidth(), 100 )
+
+	def testWidthMetadataMaintainedByRelayout( self ) :
+
+		n = Gaffer.Node()
+		n["withLabel"] = Gaffer.IntPlug()
+		n["withoutLabel"] = Gaffer.IntPlug()
+		n["other"] = Gaffer.IntPlug()
+
+		Gaffer.Metadata.registerValue( n["withLabel"], "layout:width", 100 )
+		Gaffer.Metadata.registerValue( n["withoutLabel"], "label", "" )
+
+		l = GafferUI.PlugLayout( n, orientation = GafferUI.ListContainer.Orientation.Horizontal )
+
+		withLabel = l.plugValueWidget( n["withLabel"] )
+		withLabelPlugWidget = withLabel.ancestor( GafferUI.PlugWidget )
+		self.assertIsNotNone( withLabelPlugWidget )
+
+		self.assertEqual( withLabel._qtWidget().minimumWidth(), 100 )
+		self.assertEqual( withLabel._qtWidget().maximumWidth(), 100 )
+
+		plugWidgetMin = withLabelPlugWidget._qtWidget().minimumWidth()
+		plugWidgetMax = withLabelPlugWidget._qtWidget().maximumWidth()
+		self.assertLess( plugWidgetMin, 100 )
+		self.assertGreater( plugWidgetMax, 100 )
+
+		# A width change on any plug causes an update of the entire PlugLayout,
+		# reusing the existing widgets. Ensure widths are applied to the same
+		# widgets they were applied to originally.
+
+		Gaffer.Metadata.registerValue( n["other"], "layout:width", 50 )
+		Gaffer.Metadata.registerValue( n["withoutLabel"], "layout:width", 50 )
+
+		self.assertIs( l.plugValueWidget( n["withLabel"] ), withLabel )
+		self.assertEqual( withLabel._qtWidget().minimumWidth(), 100 )
+		self.assertEqual( withLabel._qtWidget().maximumWidth(), 100 )
+		self.assertEqual( withLabelPlugWidget._qtWidget().minimumWidth(), plugWidgetMin )
+		self.assertEqual( withLabelPlugWidget._qtWidget().maximumWidth(), plugWidgetMax )
+
+		withoutLabel = l.plugValueWidget( n["withoutLabel"] )
+		self.assertIsNone( withoutLabel.ancestor( GafferUI.PlugWidget ) )
+		self.assertEqual( withoutLabel._qtWidget().minimumWidth(), 50 )
+		self.assertEqual( withoutLabel._qtWidget().maximumWidth(), 50 )
+
+		other = l.plugValueWidget( n["other"] )
+		self.assertEqual( other._qtWidget().minimumWidth(), 50 )
+		self.assertEqual( other._qtWidget().maximumWidth(), 50 )