From 4cfc9feccc7c933c58d13ac65c047dfe32d6f35f Mon Sep 17 00:00:00 2001 From: Murray Stevenson <50844517+murraystevenson@users.noreply.github.com> Date: Wed, 26 Aug 2026 12:28:31 -0700 Subject: [PATCH] PlugLayout : Prevent widgets being deleted when no longer accessories In situations where visible widgets are updated to no longer be shown under an _AccessoryRow, we see a `Internal C++ object (PySide6.QtWidgets.QWidget) already deleted` error and the Node Editor fails to show the node UI. This is a result of us trying to reuse a widget that was destroyed by Qt when its previous parent `_AccessoryRow` was removed from the updated layout. To prevent this, we now unparent all widgets from their old `_AccessoryRow`s before updating the layout. `_AccessoryRow`s are recreated as necessary on layout update, so the widgets needing one are parented to their new row, and the widgets transitioning from accessory to regular stay alive as they're no longer parented to a row pending deletion. Fixes #6938 --- Changes.md | 1 + python/GafferUI/PlugLayout.py | 8 ++++ python/GafferUITest/PlugLayoutTest.py | 61 +++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) diff --git a/Changes.md b/Changes.md index 43a3756fbcb..6a06954e5e7 100644 --- a/Changes.md +++ b/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) ======== diff --git a/python/GafferUI/PlugLayout.py b/python/GafferUI/PlugLayout.py index 7282139cdd8..8339b19aec7 100644 --- a/python/GafferUI/PlugLayout.py +++ b/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 : diff --git a/python/GafferUITest/PlugLayoutTest.py b/python/GafferUITest/PlugLayoutTest.py index edfd8e635e7..b46730f35cc 100644 --- a/python/GafferUITest/PlugLayoutTest.py +++ b/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()