Skip to content

Commit

Permalink
Merge branch '1.2_maintenance' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
johnhaddon committed Jul 7, 2023
2 parents 554f226 + 30e9747 commit 305f464
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 5 deletions.
8 changes: 8 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,14 @@ Build
1.2.x.x (relative to 1.2.9.0)
=======

Fixes
-----

- Instancer : Fixed bug that could cause the `scene:path` context variable to be leaked into the evaluation of `propotypes.set` in rare circumstances.
- NumericPlugValueWidget (#5335) :
- Fixed bug causing the cursor position to be reset to the end if the number of digits in the plug value changed while incrementing/decrementing with the keyboard up/down arrow keys.
- Fixed bug causing the cursor position to be reset to the end when incrementing an animated plug.
- Fixed intermittent `---` values when drag-changing an animated plug value.

1.2.9.0 (relative to 1.2.8.0)
=======
Expand Down
30 changes: 30 additions & 0 deletions python/GafferSceneTest/InstancerTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2225,6 +2225,36 @@ def testRootPerVertexWithEmptyPoints( self ) :
self.assertEqual( instancer["out"].childNames( "/points/instances" ), IECore.InternedStringVectorData() )
self.assertSceneValid( instancer["out"] )

def testNoScenePathInPrototypeSetContext( self ) :

plane = GafferScene.Plane()
planeFilter = GafferScene.PathFilter()
planeFilter["paths"].setValue( IECore.StringVectorData( [ "/plane" ] ) )

cube = GafferScene.Cube()
cube["sets"].setValue( "setA" )

instancer = GafferScene.Instancer()
instancer["in"].setInput( plane["out"] )
instancer["prototypes"].setInput( cube["out"] )
instancer["filter"].setInput( planeFilter["out"] )

instancer["user"]["contextVariableEnabler"] = Gaffer.BoolPlug( defaultValue = False, flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )

instancer["contextVariables"].addChild( GafferScene.Instancer.ContextVariablePlug() )
instancer["contextVariables"][0]["name"].setValue( "test" )
# Although the input has a value of `False`, this is sufficient to trick
# `Instancer::hashBranchSet()` into thinking there will be context
# variables, which causes it to call `setCollaboratePlug()->hash()`,
# which then accurately concludes that actually there are no context
# variables. It then hits a very rare code path which wasn't well tested
# until now.
instancer["contextVariables"][0]["enabled"].setInput( instancer["user"]["contextVariableEnabler"] )

# We're not asserting anything here, but the ContextSanitiser installed by `SceneTestCase.setUp()`
# is checking that we're not leaking `scene:path` upstream.
instancer["out"].set( "setA" )

def runTestContextSetPerf( self, useContexts, parallelEvaluate ):

plane = GafferScene.Plane()
Expand Down
8 changes: 8 additions & 0 deletions python/GafferUI/NumericPlugValueWidget.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ def getToolTip( self ) :

def _updateFromValues( self, values, exception ) :

if len( values ) == 0 and exception is None and self.getPlugs() :
# Placeholder update for a pending background compute. If we're animated,
# we don't want to use a `---` placeholder because it messes up editing for
# the user. Animation is quick enough to compute that there is no need
# for a placeholder anyway, so we'll just wait for the final update.
if all( Gaffer.Animation.isAnimated( p ) for p in self.getPlugs() ) :
return

# Update value and error state.

value = sole( values )
Expand Down
8 changes: 4 additions & 4 deletions python/GafferUI/NumericWidget.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,10 @@ def __setValueInternal( self, value, reason ) :
text = self.__valueToString( value )
dragBeginOrEnd = reason in ( self.ValueChangedReason.DragBegin, self.ValueChangedReason.DragEnd )

if text == self.getText() and not dragBeginOrEnd :
# early out if the text hasn't changed. we never early out if the reason is
# drag start or drag end, as we want to maintain matching pairs so things
# make sense to client code.
if self.getText() and self.__numericType( text ) == self.getValue() and not dragBeginOrEnd :
# early out if the value, after text formatting, hasn't changed. we never
# early out if the reason is drag start or drag end, as we want to maintain
# matching pairs so things make sense to client code.
return

self.setText( text )
Expand Down
4 changes: 3 additions & 1 deletion src/GafferScene/Instancer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1344,7 +1344,7 @@ void Instancer::hash( const Gaffer::ValuePlug *output, const Gaffer::Context *co
// requested were actually invalid, then we can use the same fast approximate hash used in
// hashBranchSet.
//
// This is a little bit weird, because in this scenario, this plug will never be evaluated:
// This is a little bit weird, because in this scenario, this plug will never be computed :
// computeBranchSet checks the accurate hasContextVariables before evaluating setCollaboratePlug.
// But hashBranchSet is evaluating us, so we have to give a hash that will work for it.
//
Expand All @@ -1353,6 +1353,8 @@ void Instancer::hash( const Gaffer::ValuePlug *output, const Gaffer::Context *co
// if point locations change, unlike the engineHash which includes all changes
engineHash( sourcePath, context, h );
prototypeChildNamesHash( sourcePath, context, h );
Context::EditableScope scope( context );
scope.remove( ScenePlug::scenePathContextName );
prototypesPlug()->setPlug()->hash( h );
namePlug()->hash( h );
return;
Expand Down

0 comments on commit 305f464

Please sign in to comment.