From 1df32d41df37df731d17ef4048b62d356d134d02 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 24 Jun 2024 11:34:42 +0100 Subject: [PATCH] ColorChooser : Don't emit signals with inconsistent internal state We want all our ducks in a row before being exposed to the arbitrary code execution represented by connected slots. I haven't observed any problems from the old code in practice, but noticed the pre-exising bug while reviewing #5909 and thought it probably worth addressing while we're looking at this part of the codebase. --- Changes.md | 1 + python/GafferUI/ColorChooser.py | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index 863edc089a..9010173701 100644 --- a/Changes.md +++ b/Changes.md @@ -18,6 +18,7 @@ Fixes - Cycles : Fixed rendering to the Catalogue using the batch Render node (#5905). Note that rendering a mixture of Catalogue and file outputs is still not supported, and in this case any file outputs will be ignored. - CodeWidget : Fixed bug that could prevent changes from being committed while the completion menu was visible. - Loop : Fixed handling of empty `indexVariable`. This now disables the Loop instead of creating an unnamed context variable. +- ColorChooser : Fixed emission of `colorChangedSignal()` while the widget was in an inconsistent internal state. API --- diff --git a/python/GafferUI/ColorChooser.py b/python/GafferUI/ColorChooser.py index 0d641cbc82..18ed5cfaae 100644 --- a/python/GafferUI/ColorChooser.py +++ b/python/GafferUI/ColorChooser.py @@ -257,15 +257,12 @@ def __setColorInternal( self, color, reason, hsv = False ) : GafferUI.NumericWidget.ValueChangedReason.DragEnd, ) - previousColor = self.__colorHSV if hsv else self.__color + colorChanged = color != ( self.__colorHSV if hsv else self.__color ) - if color != previousColor or dragBeginOrEnd : - # we never optimise away drag begin or end, because it's important - # that they emit in pairs. + if colorChanged : colorRGB = color.hsv2rgb() if hsv else color self.__color = colorRGB self.__colorSwatch.setColor( colorRGB ) - self.__colorChangedSignal( self, reason ) hsv = color if hsv else color.rgb2hsv() @@ -281,6 +278,11 @@ def __setColorInternal( self, color, reason, hsv = False ) : # in NumericWidget. self.__updateUIFromColor() + if colorChanged or dragBeginOrEnd : + # We never optimise away drag begin or end, because it's important + # that they emit in pairs. + self.__colorChangedSignal( self, reason ) + def __updateUIFromColor( self ) : with Gaffer.Signals.BlockedConnection( self.__componentValueChangedConnections ) :