PlugLayout : Prevent widgets being deleted when no longer accessories

Changes.md

@@ -8,6 +8,7 @@ Fixes
 
 - LevelSetOffset : Fixed crash when attempting to offset grid types other than `FloatGrid`.
 - AttributeTweaks, CustomAttributes, OptionTweaks, CustomOptions, OptionQuery : Fixed contexts used by "From Scene", "From Selected" and "From Affected" menu items. This fixes errors caused by missing context variables (such as script variables).
+- PlugLayout : Fixed `Internal C++ object already deleted` errors when a plug stops being laid out as an inline accessory, such as when its `layout:accessory` metadata was changed or removed, or when the plug it was grouped with was hidden or deleted (#6938).
 
 1.6.21.4 (relative to 1.6.21.3)
 ========

python/GafferUI/PlugLayout.py

@@ -309,6 +309,14 @@ def __updateLayout( self ) :
 		# sections.
 		rootSectionDepth = self.__rootSectionName.count( "." ) + 1 if self.__rootSectionName else 0
 		self.__rootSection.clear()
+
+		# Unparent existing accessory widgets from their previous _AccessoryRow.
+		# Accessories will be parented to their new row below, non-accessories
+		# must be unparented so Qt does not delete them along with the old row.
+		for widget in self.__widgets.values() :
+			if widget is not None and isinstance( widget.parent(), _AccessoryRow ) :
+				widget.parent().removeChild( widget )
+
 		for item in items :
 
 			if item not in self.__widgets :

python/GafferUITest/PlugLayoutTest.py

@@ -412,6 +412,67 @@ def iNameFilterFunction( plug ) :
 		self.assertTrue( l.plugValueWidget( n["f"] ).visible() )
 		self.assertTrue( l.customWidget( "test" ).visible() )
 
+	def testUpdateAccessoryMetadata( self ) :
+
+		n = Gaffer.Node()
+		n["a"] = Gaffer.IntPlug()
+		n["b"] = Gaffer.IntPlug()
+
+		Gaffer.Metadata.registerValue( n["b"], "layout:accessory", True )
+
+		l = GafferUI.PlugLayout( n )
+		a = l.plugValueWidget( n["a"] )
+		b = l.plugValueWidget( n["b"] )
+
+		for accessory in ( False, True ) :
+
+			Gaffer.Metadata.registerValue( n["b"], "layout:accessory", accessory )
+
+			self.assertIs( l.plugValueWidget( n["a"] ), a )
+			self.assertIs( l.plugValueWidget( n["b"] ), b )
+			self.assertTrue( l.isAncestorOf( a ) )
+			self.assertTrue( l.isAncestorOf( b ) )
+
+		Gaffer.Metadata.deregisterValue( n["b"], "layout:accessory" )
+
+		self.assertIs( l.plugValueWidget( n["a"] ), a )
+		self.assertIs( l.plugValueWidget( n["b"] ), b )
+		self.assertTrue( l.isAncestorOf( a ) )
+		self.assertTrue( l.isAncestorOf( b ) )
+
+	def testRemovePlugWithAccessory( self ) :
+
+		n = Gaffer.Node()
+		n["a"] = Gaffer.IntPlug()
+		n["b"] = Gaffer.IntPlug()
+
+		Gaffer.Metadata.registerValue( n["b"], "layout:accessory", True )
+
+		l = GafferUI.PlugLayout( n )
+		a = l.plugValueWidget( n["a"] )
+		b = l.plugValueWidget( n["b"] )
+
+		del( n["a"] )
+
+		self.assertIs( l.plugValueWidget( n["b"] ), b )
+		self.assertTrue( l.isAncestorOf( b ) )
+
+	def testHidePlugWithAccessory( self ) :
+
+		n = Gaffer.Node()
+		n["a"] = Gaffer.IntPlug()
+		n["b"] = Gaffer.IntPlug()
+
+		Gaffer.Metadata.registerValue( n["b"], "layout:accessory", True )
+
+		l = GafferUI.PlugLayout( n )
+		a = l.plugValueWidget( n["a"] )
+		b = l.plugValueWidget( n["b"] )
+
+		Gaffer.Metadata.registerValue( n["a"], "plugValueWidget:type", "" )
+
+		self.assertIs( l.plugValueWidget( n["b"] ), b )
+		self.assertTrue( l.isAncestorOf( b ) )
 
 if __name__ == "__main__":
 	unittest.main()