diff --git a/Changes.md b/Changes.md index ed4f458d796..993271fad06 100644 --- a/Changes.md +++ b/Changes.md @@ -6,6 +6,9 @@ Improvements - ColorChooser : Added channel names to identify sliders. - RenderPassEditor : Added "Select Affected Objects" popup menu item. +- Annotations : + - Added support for `{plug}` value substitutions in node annotations. + - Added Ctrl + Enter keyboard shortcut to annotation dialogue. This applies the annotation and closes the dialogue. Fixes ----- @@ -13,6 +16,12 @@ 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. +API +--- + +- AnnotationsGadget : Added `annotationText()` method. +- ParallelAlgoTest : Added `UIThreadCallHandler.receive()` method. + 1.4.7.0 (relative to 1.4.6.0) ======= diff --git a/include/GafferUI/AnnotationsGadget.h b/include/GafferUI/AnnotationsGadget.h index dcc407d8d98..f9419eab994 100644 --- a/include/GafferUI/AnnotationsGadget.h +++ b/include/GafferUI/AnnotationsGadget.h @@ -37,6 +37,7 @@ #pragma once #include "Gaffer/MetadataAlgo.h" +#include "Gaffer/ParallelAlgo.h" #include "GafferUI/Gadget.h" @@ -74,6 +75,10 @@ class GAFFERUI_API AnnotationsGadget : public Gadget void setVisibleAnnotations( const IECore::StringAlgo::MatchPattern &patterns ); const IECore::StringAlgo::MatchPattern &getVisibleAnnotations() const; + /// Returns the text currently being rendered for the specified + /// annotation. Only really intended for use in the unit tests. + const std::string &annotationText( const Gaffer::Node *node, IECore::InternedString annotation = "user" ) const; + bool acceptsParent( const GraphComponent *potentialParent ) const override; protected : @@ -90,29 +95,85 @@ class GAFFERUI_API AnnotationsGadget : public Gadget private : - GraphGadget *graphGadget(); - const GraphGadget *graphGadget() const; - + struct Annotations; + + // Update process + // ============== + // + // We query annotation metadata and store it ready for rendering in our + // `m_annotations` data structure. This occurs in synchronous, lazy and + // asynchronous phases as performance requirements dictate. + // + // In the first phase, these two methods ensure that `m_annotations` + // always has an entry for each NodeGadget being drawn by the + // GraphGadget. This is done synchronously with the addition and removal + // of children. void graphGadgetChildAdded( GraphComponent *child ); void graphGadgetChildRemoved( const GraphComponent *child ); + // These accessors can then be used to find the annotations (if any) + // for a node. + Annotations *annotations( const Gaffer::Node *node ); + const Annotations *annotations( const Gaffer::Node *node ) const; + // We then use `nodeMetadataChanged()` to dirty individual annotations + // when the metadata has changed. We don't query the metadata at this + // point, as it's fairly typical to receive many metadata edits at once + // and we want to batch the updates. We might not even be visible when + // the edits are made. void nodeMetadataChanged( IECore::TypeId nodeTypeId, IECore::InternedString key, Gaffer::Node *node ); - void update() const; + // We lazily call `update()` from `renderLayer()` to query all dirty + // metadata just in time for rendering. Such update are fairly + // infrequent because annotations are edited infrequently. + void update(); + // Some annotations use `{}` syntax to substitute in the values of + // plugs. For these we use `plugDirtied()` to check if the substitutions + // are affected and dirty them when necessary. Plugs are dirtied + // frequently and many don't affect the substitutions at all, so this is + // performed at a finer level of granularity than `update()`. + void plugDirtied( const Gaffer::Plug *plug, Annotations *annotations ); + // If the substitutions are from computed plugs, then we also need to + // update when the context changes. + void scriptContextChanged(); + // Some plug substitutions may depend on computes, in which case we must + // perform the substitutions in a BackgroundTask to avoid blocking the + // UI. This function schedules such a task, or if the values are not + // computes, does the substitutions directly on the UI thread. This is + // done on a per-node basis, so that slow updates for one node do not + // prevent other nodes updating rapidly. + void schedulePlugValueSubstitutions( const Gaffer::Node *node, Annotations *annotations ); + // These two functions do the actual work of calculating and applying + // substitutions. + std::unordered_map substitutedRenderText( const Gaffer::Node *node, const Annotations &annotations ); + void applySubstitutedRenderText( const std::unordered_map &renderText, Annotations &annotations ); + // When we are hidden, we want to cancel all background tasks. + void visibilityChanged(); + + struct StandardAnnotation : public Gaffer::MetadataAlgo::Annotation + { + StandardAnnotation( const Gaffer::MetadataAlgo::Annotation &a, IECore::InternedString name ) : Annotation( a ), name( name ) {} + IECore::InternedString name; + std::string renderText; + }; struct Annotations { bool dirty = true; - std::vector standardAnnotations; + std::vector standardAnnotations; bool bookmarked = false; IECore::InternedString numericBookmark; bool renderable = false; + bool hasPlugValueSubstitutions = false; + bool hasContextSensitiveSubstitutions = false; + Gaffer::Signals::ScopedConnection plugDirtiedConnection; + std::unique_ptr substitutionsTask; }; Gaffer::Signals::ScopedConnection m_graphGadgetChildAddedConnection; Gaffer::Signals::ScopedConnection m_graphGadgetChildRemovedConnection; + Gaffer::Signals::ScopedConnection m_scriptContextChangedConnection; using AnnotationsContainer = std::unordered_map; - mutable AnnotationsContainer m_annotations; - mutable bool m_dirty; + AnnotationsContainer m_annotations; + bool m_dirty; IECore::StringAlgo::MatchPattern m_visibleAnnotations; diff --git a/python/GafferTest/ParallelAlgoTest.py b/python/GafferTest/ParallelAlgoTest.py index afd00882343..4553c42026f 100644 --- a/python/GafferTest/ParallelAlgoTest.py +++ b/python/GafferTest/ParallelAlgoTest.py @@ -74,16 +74,22 @@ def __callOnUIThread( self, f ) : self.__queue.put( f ) - # Waits for a single use of `callOnUIThread()`, raising - # a test failure if none arises before `timeout` seconds. - def assertCalled( self, timeout = 30.0 ) : + # Waits for a single use of `callOnUIThread()` and returns the functor + # that was passed. It is the caller's responsibility to call the + # functor. Raises a test failure if no call arises before `timeout` + # seconds. + def receive( self, timeout = 30.0 ) : try : - f = self.__queue.get( block = True, timeout = timeout ) + return self.__queue.get( block = True, timeout = timeout ) except queue.Empty : raise AssertionError( "UIThread call not made within {} seconds".format( timeout ) ) - f() + # Waits for and handles a single use of `callOnUIThread()`, raising a + # test failure if none arises before `timeout` seconds. + def assertCalled( self, timeout = 30.0 ) : + + self.receive( timeout )() # Asserts that no further uses of `callOnUIThread()` will # be made with this handler. This is checked on context exit. diff --git a/python/GafferUI/AnnotationsUI.py b/python/GafferUI/AnnotationsUI.py index ee6b4241b8a..c1195c6d580 100644 --- a/python/GafferUI/AnnotationsUI.py +++ b/python/GafferUI/AnnotationsUI.py @@ -35,6 +35,7 @@ ########################################################################## import functools +import re import imath import IECore @@ -71,6 +72,72 @@ def __annotate( node, name, menu ) : dialogue = __AnnotationsDialogue( node, name ) dialogue.wait( parentWindow = menu.ancestor( GafferUI.Window ) ) +class _AnnotationsHighlighter( GafferUI.CodeWidget.Highlighter ) : + + __substitutionRe = re.compile( r"(\{[^}]+\})" ) + + def __init__( self, node ) : + + GafferUI.CodeWidget.Highlighter.__init__( self ) + self.__node = node + + def highlights( self, line, previousHighlightType ) : + + result = [] + + l = 0 + for token in self.__substitutionRe.split( line ) : + if ( + len( token ) > 2 and + token[0] == "{" and token[-1] == "}" and + isinstance( self.__node.descendant( token[1:-1] ), Gaffer.ValuePlug ) + ) : + result.append( + self.Highlight( l, l + len( token ), self.Type.Keyword ) + ) + l += len( token ) + + return result + +class _AnnotationsCompleter( GafferUI.CodeWidget.Completer ) : + + __incompleteSubstitutionRe = re.compile( r"\{([^.}][^}]*$)" ) + + def __init__( self, node ) : + + GafferUI.CodeWidget.Completer.__init__( self ) + self.__node = node + + def completions( self, text ) : + + m = self.__incompleteSubstitutionRe.search( text ) + if m is None : + return [] + + parentPath, _, childName = m.group( 1 ).rpartition( "." ) + parent = self.__node.descendant( parentPath ) if parentPath else self.__node + if parent is None : + return [] + + result = [] + for plug in Gaffer.Plug.Range( parent ) : + if not hasattr( plug, "getValue" ) and not len( plug ) : + continue + if plug.getName().startswith( childName ) : + childPath = plug.relativeName( self.__node ) + result.append( + self.Completion( + "{prefix}{{{childPath}{closingBrace}".format( + prefix = text[:m.start()], + childPath = childPath, + closingBrace = "}" if hasattr( plug, "getValue" ) else "" + ), + label = childPath + ) + ) + + return result + class __AnnotationsDialogue( GafferUI.Dialogue ) : def __init__( self, node, name ) : @@ -85,13 +152,21 @@ def __init__( self, node, name ) : with GafferUI.ListContainer( GafferUI.ListContainer.Orientation.Vertical, spacing = 4 ) as layout : - self.__textWidget = GafferUI.MultiLineTextWidget( + self.__textWidget = GafferUI.CodeWidget( text = annotation.text() if annotation else "", + placeholderText = "Tip : Use {plugName} to include plug values", ) + self.__textWidget.setHighlighter( _AnnotationsHighlighter( node ) ) + self.__textWidget.setCompleter( _AnnotationsCompleter( node ) ) self.__textWidget.textChangedSignal().connect( Gaffer.WeakMethod( self.__updateButtonStatus ), scoped = False ) - + self.__textWidget.activatedSignal().connect( + Gaffer.WeakMethod( self.__textActivated ), scoped = False + ) + self.__textWidget.contextMenuSignal().connect( + Gaffer.WeakMethod( self.__textWidgetContextMenu ), scoped = False + ) if not template : self.__colorChooser = GafferUI.ColorChooser( annotation.color() if annotation else imath.Color3f( 0.15, 0.26, 0.26 ), @@ -147,3 +222,45 @@ def __makeAnnotation( self ) : ) else : return Gaffer.MetadataAlgo.Annotation( self.__textWidget.getText() ) + + def __textActivated( self, *unused ) : + + if self.__annotateButton.getEnabled() : + self.__annotateButton.clickedSignal()( self.__annotateButton ) + + def __textWidgetContextMenu( self, *unused ) : + + menuDefinition = IECore.MenuDefinition() + + def menuLabel( name ) : + + if "_" in name : + name = IECore.CamelCase.fromSpaced( name.replace( "_", " " ) ) + return IECore.CamelCase.toSpaced( name ) + + def walkPlugs( graphComponent ) : + + if graphComponent.getName().startswith( "__" ) : + return + + if isinstance( graphComponent, Gaffer.ValuePlug ) and hasattr( graphComponent, "getValue" ) : + relativeName = graphComponent.relativeName( self.__node ) + menuDefinition.append( + "/Insert Plug Value/{}".format( "/".join( menuLabel( n ) for n in relativeName.split( "." ) ) ), + { + "command" : functools.partial( Gaffer.WeakMethod( self.__textWidget.insertText ), f"{{{relativeName}}}" ), + } + ) + else : + for plug in Gaffer.Plug.InputRange( graphComponent ) : + walkPlugs( plug ) + + walkPlugs( self.__node ) + + if not menuDefinition.size() : + menuDefinition.append( "/Insert Plug Value/No plugs available", { "active" : False } ) + + self.__popupMenu = GafferUI.Menu( menuDefinition ) + self.__popupMenu.popup( parent = self ) + + return True diff --git a/python/GafferUITest/AnnotationsGadgetTest.py b/python/GafferUITest/AnnotationsGadgetTest.py new file mode 100644 index 00000000000..ae722e447c5 --- /dev/null +++ b/python/GafferUITest/AnnotationsGadgetTest.py @@ -0,0 +1,424 @@ +########################################################################## +# +# Copyright (c) 2024, Cinesite VFX Ltd. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above +# copyright notice, this list of conditions and the following +# disclaimer. +# +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following +# disclaimer in the documentation and/or other materials provided with +# the distribution. +# +# * Neither the name of John Haddon nor the names of +# any other contributors to this software may be used to endorse or +# promote products derived from this software without specific prior +# written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +# IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +# PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +########################################################################## + +import inspect +import threading +import unittest + +import imath + +import IECore + +import Gaffer +import GafferTest +import GafferUI +import GafferUITest + +class AnnotationsGadgetTest( GafferUITest.TestCase ) : + + def testSimpleText( self ) : + + script = Gaffer.ScriptNode() + script["node"] = Gaffer.Node() + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + self.assertEqual( gadget.annotationText( script["node"] ), "" ) + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test" ) ) + self.assertEqual( gadget.annotationText( script["node"] ), "test" ) + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test2" ) ) + self.assertEqual( gadget.annotationText( script["node"] ), "test2" ) + + Gaffer.MetadataAlgo.removeAnnotation( script["node"], "user" ) + self.assertEqual( gadget.annotationText( script["node"] ), "" ) + + def testExtendLifetimePastGraphGadget( self ) : + + script = Gaffer.ScriptNode() + script["node"] = Gaffer.Node() + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + del graphGadget + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test" ) ) + self.assertEqual( gadget.annotationText( script["node"] ), "" ) + + def testSubstitutedText( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + self.assertEqual( gadget.annotationText( script["node"] ), "" ) + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test : {op1}" ) ) + self.assertEqual( gadget.annotationText( script["node"] ), "test : 0" ) + + script["node"]["op1"].setValue( 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "test : 1" ) + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{}" ) ) + self.assertEqual( gadget.annotationText( script["node"] ), "" ) + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{notAPlug}" ) ) + self.assertEqual( gadget.annotationText( script["node"] ), "" ) + + def testSubstitutionsByPlugType( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{plug}" ) ) + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + + for plugType, value, substitution in [ + ( Gaffer.BoolPlug, True, "On" ), + ( Gaffer.BoolPlug, False, "Off" ), + ( Gaffer.IntPlug, 1, "1" ), + ( Gaffer.FloatPlug, 1.1, "1.1" ), + ( Gaffer.V2iPlug, imath.V2i( 1, 2 ), "1, 2" ), + ( Gaffer.Color3fPlug, imath.Color3f( 1, 2, 3 ), "1, 2, 3" ), + ( Gaffer.StringVectorDataPlug, IECore.StringVectorData( [ "1", "2", "3" ] ), "1, 2, 3" ), + ( Gaffer.IntVectorDataPlug, IECore.IntVectorData( [ 1, 2, 3 ] ), "1, 2, 3" ), + ] : + + script["node"]["plug"] = plugType( defaultValue = value ) + self.assertEqual( gadget.annotationText( script["node"] ), substitution ) + + def testInitialSubstitutedText( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test : {op1}" ) ) + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + self.assertEqual( gadget.annotationText( script["node"] ), "test : 0" ) + + def testComputedText( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + script["errorNode"] = GafferTest.BadNode() + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test : {sum}" ) ) + + errorCondition = threading.Condition() + def error( *unused ) : + with errorCondition : + errorCondition.notify() + script["errorNode"].errorSignal().connect( error, scoped = False ) + + with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + + # But if we wait for the background update we should get some updated text. + callHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "test : 0" ) + + # Same applies when the plug is dirtied. We expect a placeholder first. + script["node"]["op1"].setValue( 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + + # Then we get the real value when the computation is done. + callHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "test : 1" ) + + # And when the plug is dirtied by an upstream change we again expect + # placeholder text at first. + script["node"]["op1"].setInput( script["errorNode"]["out3"] ) + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + + # But this time we don't expect to get updated text, because the computation + # will error. + with errorCondition : + errorCondition.wait() + + # Handle UI thread calls made by StandardNodeGadget to show errors, + # and assert that there are no more calls. + callHandler.assertCalled() + callHandler.assertCalled() + + callHandler.assertDone() + + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + + def testComputedTextCancellation( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + script["node2"] = GafferTest.AddNode() + + AnnotationsGadgetTest.expressionStartedCondition = threading.Condition() + AnnotationsGadgetTest.expressionContinueCondition = threading.Condition() + + script["expression"] = Gaffer.Expression() + script["expression"].setExpression( inspect.cleandoc( + """ + # Let the test know the expression has started running. + import GafferUITest + with GafferUITest.AnnotationsGadgetTest.expressionStartedCondition : + GafferUITest.AnnotationsGadgetTest.expressionStartedCondition.notify() + + # And loop checking for cancellation until the test + # allows us to continue. + while True : + IECore.Canceller.check( context.canceller() ) + with GafferUITest.AnnotationsGadgetTest.expressionContinueCondition : + if GafferUITest.AnnotationsGadgetTest.expressionContinueCondition.wait( timeout = 0.1 ) : + break + + parent["node"]["op1"] = parent["node"]["op2"] + """ + ) ) + + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) + + with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : + + graphGadget = GafferUI.GraphGadget( script ) + viewportGadget = GafferUI.ViewportGadget( graphGadget ) + gadget = graphGadget["__annotations"] + + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + + # Wait for compute to start, and make a graph edit to cancel it. + with AnnotationsGadgetTest.expressionStartedCondition : + AnnotationsGadgetTest.expressionStartedCondition.wait() + + script["node"]["op2"].setValue( 2 ) + + # We expect a call on the UI thread to re-dirty the annotation. + + renderRequests = GafferTest.CapturingSlot( viewportGadget.renderRequestSignal() ) + callHandler.assertCalled() + self.assertEqual( len( renderRequests ), 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + + # A new background task should have been launched to compute the + # text again. If we let the expression run to completion then we + # should get the final text. + with AnnotationsGadgetTest.expressionStartedCondition : + AnnotationsGadgetTest.expressionStartedCondition.wait() + + with GafferUITest.AnnotationsGadgetTest.expressionContinueCondition : + GafferUITest.AnnotationsGadgetTest.expressionContinueCondition.notify() + + callHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "4" ) + + # Try one more time. But this time do the cancellation by + # modifying a completely unrelated plug. + + script["node"]["op2"].setValue( 3 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + with AnnotationsGadgetTest.expressionStartedCondition : + AnnotationsGadgetTest.expressionStartedCondition.wait() + + script["node2"]["op1"].setValue( 1 ) # Cancels + + renderRequests = GafferTest.CapturingSlot( viewportGadget.renderRequestSignal() ) + callHandler.assertCalled() + self.assertEqual( len( renderRequests ), 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + + with AnnotationsGadgetTest.expressionStartedCondition : + AnnotationsGadgetTest.expressionStartedCondition.wait() + + with GafferUITest.AnnotationsGadgetTest.expressionContinueCondition : + GafferUITest.AnnotationsGadgetTest.expressionContinueCondition.notify() + + callHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "6" ) + + callHandler.assertDone() + + def testContextSensitiveText( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.FrameNode() + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{output}" ) ) + + with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + + # But if we wait for the background update we should get some updated text. + callHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "1" ) + + # Same applies when the context is changed. We expect a placeholder first. + script.context().setFrame( 10 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + + # Then we get the real value when the computation is done. + callHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "10" ) + + callHandler.assertDone() + + def testSubstitutedTextRenderRequests( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + viewportGadget = GafferUI.ViewportGadget( graphGadget ) + renderRequests = GafferTest.CapturingSlot( viewportGadget.renderRequestSignal() ) + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test" ) ) + self.assertEqual( len( renderRequests ), 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "test" ) + + script["node"]["op1"].setValue( 1 ) + self.assertEqual( len( renderRequests ), 1 ) # No new request, because no substitutions + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test : {op1}" ) ) + self.assertEqual( len( renderRequests ), 2 ) # New request, because annotations changed + self.assertEqual( gadget.annotationText( script["node"] ), "test : 1" ) + + script["node"]["op1"].setValue( 2 ) + self.assertEqual( len( renderRequests ), 3 ) # New request, because substitution changed + + script["node"]["op2"].setValue( 2 ) + self.assertEqual( len( renderRequests ), 3 ) # No new request, because plug doesn't affect substitution + + script.context().setFrame( 10 ) + self.assertEqual( len( renderRequests ), 4 ) # One new request, from unrelated code in GraphGadget + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "test : {sum}" ) ) + self.assertEqual( len( renderRequests ), 5 ) # New request, because annotation changed + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + + script.context().setFrame( 11 ) + # 2 new requests, one because substitution depends on compute, and one from unrelated code in GraphGadget. + self.assertEqual( len( renderRequests ), 7 ) + + def testDestroyGadgetWhileBackgroundThreadRuns( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) + + with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + + # Value must be computed in background, so initially we expect a placeholder. + self.assertEqual( gadget.annotationText( script["node"], "user" ), "---" ) + + # Wait for the UI thread call that will be scheduled to update the gadget. + call = callHandler.receive() + # But then delete our references to the gadget _before_ we execute + # the call. This simulates a user removing a GraphEditor while the + # AnnotationsGadget is still updating. If we don't handle lifetimes + # well, then this could crash. + del graphGadget, gadget + call() + + callHandler.assertDone() + + def testRemoveNodeWhileBackgroundThreadRuns( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + + Gaffer.MetadataAlgo.addAnnotation( script["node"], "test", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) + + with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"], "test" ), "---" ) + + # Wait for the UI thread call that will be scheduled to update the gadget. + call = callHandler.receive() + # But then delete the node _before_ we execute the call. This + # simulates a user deleting a node while the AnnotationsGadget is + # still updating. If we don't handle lifetimes well, then this could + # crash. + del script["node"] + call() + + callHandler.assertDone() + + def testRemoveAnnotationWhileBackgroundThreadRuns( self ) : + + script = Gaffer.ScriptNode() + script["node"] = GafferTest.AddNode() + + numAnnotations = 100 + for i in range( 0, numAnnotations ) : + Gaffer.MetadataAlgo.addAnnotation( script["node"], f"test{i}", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) + + with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : + + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget["__annotations"] + + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"], "test0" ), "---" ) + + # Remove annotations while the background task runs. + for i in range( 0, numAnnotations ) : + Gaffer.MetadataAlgo.removeAnnotation( script["node"], f"test{i}" ) + self.assertEqual( gadget.annotationText( script["node"], "test0" ), "" ) + + # And wait for the task to complete. + callHandler.assertCalled() + +if __name__ == "__main__": + unittest.main() diff --git a/python/GafferUITest/__init__.py b/python/GafferUITest/__init__.py index b57c06f8f1e..bf3cac41534 100644 --- a/python/GafferUITest/__init__.py +++ b/python/GafferUITest/__init__.py @@ -131,6 +131,7 @@ from .LabelPlugValueWidgetTest import LabelPlugValueWidgetTest from .PythonEditorTest import PythonEditorTest from .BoxIOUITest import BoxIOUITest +from .AnnotationsGadgetTest import AnnotationsGadgetTest if __name__ == "__main__": unittest.main() diff --git a/src/GafferUI/AnnotationsGadget.cpp b/src/GafferUI/AnnotationsGadget.cpp index c3c71192917..b11b36aa5ee 100644 --- a/src/GafferUI/AnnotationsGadget.cpp +++ b/src/GafferUI/AnnotationsGadget.cpp @@ -44,12 +44,17 @@ #include "Gaffer/Metadata.h" #include "Gaffer/MetadataAlgo.h" +#include "Gaffer/PlugAlgo.h" +#include "Gaffer/Process.h" #include "Gaffer/ScriptNode.h" +#include "Gaffer/StringPlug.h" #include "boost/algorithm/string/predicate.hpp" #include "boost/bind/bind.hpp" #include "boost/bind/placeholders.hpp" +#include + using namespace GafferUI; using namespace Gaffer; using namespace IECore; @@ -116,7 +121,142 @@ string wrap( const std::string &text, size_t maxLineLength ) return result; } -float g_offset = 0.5; + +const float g_offset = 0.5; +const string g_emptyString; +const int g_maxLineLength = 60; + +template +string formatVectorDataPlugValue( const PlugType *plug ) +{ + auto value = plug->getValue(); + + string result; + for( const auto &v : value->readable() ) + { + if( !result.empty() ) + { + result += ", "; + } + result += fmt::format( "{}", v ); + } + + return result; +} + +/// \todo Should this be made public in PlugAlgo, and how does it relate +/// to `SpreadsheetUI.formatValue()`? It would be really nice if this could +/// support preset names too, but those are only available in Python at +/// present. +std::string formatPlugValue( const ValuePlug *plug ) +{ + switch( (int)plug->typeId() ) + { + case BoolPlugTypeId : + return static_cast( plug )->getValue() ? "On" : "Off"; + case IntPlugTypeId : + return std::to_string( static_cast( plug )->getValue() ); + case FloatPlugTypeId : + return fmt::format( "{}", static_cast( plug )->getValue() ); + case StringPlugTypeId : + return static_cast( plug )->getValue(); + case IntVectorDataPlugTypeId : + return formatVectorDataPlugValue( static_cast( plug ) ); + case FloatVectorDataPlugTypeId : + return formatVectorDataPlugValue( static_cast( plug ) ); + case StringVectorDataPlugTypeId : + return formatVectorDataPlugValue( static_cast( plug ) ); + default : { + string result; + for( const auto &child : ValuePlug::Range( *plug ) ) + { + if( !result.empty() ) + { + result += ", "; + } + result += formatPlugValue( child.get() ); + } + return result; + } + } +} + +const std::regex g_plugValueSubstitutionRegex( R"(\{([^}]*)\})" ); + +bool hasPlugValueSubstitutions( const string &text ) +{ + return std::regex_search( text, g_plugValueSubstitutionRegex ); +} + +bool affectsPlugValueSubstitutions( const Plug *plug, const string &text ) +{ + const string name = plug->relativeName( plug->node() ); + sregex_iterator matchIt( text.begin(), text.end(), g_plugValueSubstitutionRegex ); + sregex_iterator matchEnd; + for( ; matchIt != matchEnd; ++matchIt ) + { + if( matchIt->str( 1 ) == name ) + { + return true; + } + } + + return false; +} + +const Plug *plugValueSubstitutionsBackgroundTaskSubject( const std::string &text, const Node *node ) +{ + sregex_iterator matchIt( text.begin(), text.end(), g_plugValueSubstitutionRegex ); + sregex_iterator matchEnd; + for( ; matchIt != matchEnd; ++matchIt ) + { + const string plugPath = matchIt->str( 1 ); + if( auto plug = node->descendant( plugPath ) ) + { + if( PlugAlgo::dependsOnCompute( plug ) ) + { + return plug; + } + } + } + return nullptr; +} + +string substitutePlugValues( const string &text, const Node *node ) +{ + sregex_iterator matchIt( text.begin(), text.end(), g_plugValueSubstitutionRegex ); + sregex_iterator matchEnd; + + if( matchIt == matchEnd ) + { + // No matches + return text; + } + + string result; + ssub_match suffix; + for( ; matchIt != matchEnd; ++matchIt ) + { + // Add any unmatched text from before this match. + result.insert( result.end(), matchIt->prefix().first, matchIt->prefix().second ); + + const string plugPath = matchIt->str( 1 ); + if( !node ) + { + result += "---"; + } + else if( auto plug = node->descendant( plugPath ) ) + { + result += formatPlugValue( plug ); + } + + suffix = matchIt->suffix(); + } + // The suffix for one match is the same as the prefix for the next + // match. So we only need to add the suffix from the last match. + result.insert( result.end(), suffix.first, suffix.second ); + return result; +} } // namespace @@ -134,6 +274,9 @@ AnnotationsGadget::AnnotationsGadget() Metadata::nodeValueChangedSignal().connect( boost::bind( &AnnotationsGadget::nodeMetadataChanged, this, ::_1, ::_2, ::_3 ) ); + visibilityChangedSignal().connect( + boost::bind( &AnnotationsGadget::visibilityChanged, this ) + ); } AnnotationsGadget::~AnnotationsGadget() @@ -161,6 +304,23 @@ const IECore::StringAlgo::MatchPattern &AnnotationsGadget::getVisibleAnnotations return m_visibleAnnotations; } +const std::string &AnnotationsGadget::annotationText( const Gaffer::Node *node, IECore::InternedString annotation ) const +{ + const_cast( this )->update(); + if( const Annotations *nodeAnnotations = annotations( node ) ) + { + for( const auto &a : nodeAnnotations->standardAnnotations ) + { + if( a.name == annotation ) + { + return a.renderText; + } + } + } + + return g_emptyString; +} + bool AnnotationsGadget::acceptsParent( const GraphComponent *potentialParent ) const { return runTimeCast( potentialParent ); @@ -189,11 +349,11 @@ void AnnotationsGadget::renderLayer( Layer layer, const Style *style, RenderReas return; } - update(); + const_cast( this )->update(); - for( auto &ga : m_annotations ) + for( const auto &ga : m_annotations ) { - Annotations &annotations = ga.second; + const Annotations &annotations = ga.second; assert( !annotations.dirty ); if( !annotations.renderable ) { @@ -238,7 +398,7 @@ void AnnotationsGadget::renderLayer( Layer layer, const Style *style, RenderReas for( const auto &a : annotations.standardAnnotations ) { - annotationOrigin = style->renderAnnotation( annotationOrigin, a.text(), Style::NormalState, a.colorData ? &a.color() : nullptr ); + annotationOrigin = style->renderAnnotation( annotationOrigin, a.renderText, Style::NormalState, a.colorData ? &a.color() : nullptr ); } } } @@ -256,16 +416,6 @@ Imath::Box3f AnnotationsGadget::renderBound() const return b; } -GraphGadget *AnnotationsGadget::graphGadget() -{ - return parent(); -} - -const GraphGadget *AnnotationsGadget::graphGadget() const -{ - return parent(); -} - void AnnotationsGadget::graphGadgetChildAdded( GraphComponent *child ) { if( NodeGadget *nodeGadget = runTimeCast( child ) ) @@ -284,6 +434,34 @@ void AnnotationsGadget::graphGadgetChildRemoved( const GraphComponent *child ) } } +AnnotationsGadget::Annotations *AnnotationsGadget::annotations( const Gaffer::Node *node ) +{ + return const_cast( const_cast( this )->annotations( node ) ); +} + +const AnnotationsGadget::Annotations *AnnotationsGadget::annotations( const Gaffer::Node *node ) const +{ + const GraphGadget *graphGadget = parent(); + if( !graphGadget ) + { + return nullptr; + } + + const NodeGadget *nodeGadget = graphGadget->nodeGadget( node ); + if( !nodeGadget ) + { + return nullptr; + } + + auto it = m_annotations.find( nodeGadget ); + if( it == m_annotations.end() ) + { + return nullptr; + } + + return &it->second; +} + void AnnotationsGadget::nodeMetadataChanged( IECore::TypeId nodeTypeId, IECore::InternedString key, Gaffer::Node *node ) { if( !node ) @@ -302,28 +480,31 @@ void AnnotationsGadget::nodeMetadataChanged( IECore::TypeId nodeTypeId, IECore:: return; } - if( auto gadget = graphGadget()->nodeGadget( node ) ) + if( Annotations *a = annotations( node ) ) { - auto it = m_annotations.find( gadget ); - assert( it != m_annotations.end() ); - it->second.dirty = true; + a->dirty = true; m_dirty = true; dirty( DirtyType::Render ); } } -void AnnotationsGadget::update() const +void AnnotationsGadget::update() { - if( !m_dirty ) + if( !m_dirty || !parent() ) { return; } + Context *context = parent()->getRoot()->scriptNode()->context(); + Context::Scope scopedContext( context ); + vector templates; MetadataAlgo::annotationTemplates( templates ); std::sort( templates.begin(), templates.end() ); const bool untemplatedVisible = StringAlgo::matchMultiple( untemplatedAnnotations, m_visibleAnnotations ); + bool dependsOnContext = false; + vector names; for( auto &ga : m_annotations ) { @@ -349,7 +530,10 @@ void AnnotationsGadget::update() const annotations.numericBookmark = InternedString(); } + annotations.substitutionsTask.reset(); // Stop task before clearing the data it accesses. + annotations.hasPlugValueSubstitutions = false; annotations.standardAnnotations.clear(); + names.clear(); MetadataAlgo::annotations( node, names ); for( const auto &name : names ) @@ -363,20 +547,221 @@ void AnnotationsGadget::update() const } } - annotations.standardAnnotations.push_back( - MetadataAlgo::getAnnotation( node, name, /* inheritTemplate = */ true ) - ); - // Word wrap. It might be preferable to do this during - // rendering, but we have no way of querying the extent of - // `Style::renderWrappedText()`. - annotations.standardAnnotations.back().textData = new StringData( - wrap( annotations.standardAnnotations.back().text(), 60 ) - ); + StandardAnnotation a( MetadataAlgo::getAnnotation( node, name, /* inheritTemplate = */ true ), name ); + if( !hasPlugValueSubstitutions( a.text() ) ) + { + a.renderText = wrap( a.text(), g_maxLineLength ); + } + else + { + // We'll update `renderText` later in `schedulePlugValueSubstitutions()`. + annotations.hasPlugValueSubstitutions = true; + if( plugValueSubstitutionsBackgroundTaskSubject( a.text(), node ) ) + { + annotations.hasContextSensitiveSubstitutions = true; + dependsOnContext = true; + } + } + annotations.standardAnnotations.push_back( a ); } annotations.renderable |= (bool)annotations.standardAnnotations.size(); + if( annotations.hasPlugValueSubstitutions ) + { + annotations.plugDirtiedConnection = const_cast( node )->plugDirtiedSignal().connect( + boost::bind( &AnnotationsGadget::plugDirtied, this, ::_1, &annotations ) + ); + schedulePlugValueSubstitutions( node, &annotations ); + } + else + { + annotations.plugDirtiedConnection.disconnect(); + } + annotations.dirty = false; } + if( dependsOnContext ) + { + m_scriptContextChangedConnection = context->changedSignal().connect( + boost::bind( &AnnotationsGadget::scriptContextChanged, this ) + ); + } + else + { + m_scriptContextChangedConnection.disconnect(); + } + m_dirty = false; } + +void AnnotationsGadget::plugDirtied( const Gaffer::Plug *plug, Annotations *annotations ) +{ + assert( annotations->hasPlugValueSubstitutions ); + for( const auto &annotation : annotations->standardAnnotations ) + { + if( affectsPlugValueSubstitutions( plug, annotation.text() ) ) + { + annotations->dirty = true; + m_dirty = true; + dirty( DirtyType::Render ); + break; + } + } +} + +void AnnotationsGadget::scriptContextChanged() +{ + bool dirtied = false; + for( auto &[nodeGadget, annotation] : m_annotations ) + { + if( annotation.hasContextSensitiveSubstitutions ) + { + annotation.dirty = true; + dirtied = true; + } + } + + if( dirtied ) + { + m_dirty = true; + dirty( DirtyType::Render ); + } +} + +void AnnotationsGadget::schedulePlugValueSubstitutions( const Gaffer::Node *node, Annotations *annotations ) +{ + const Plug *backgroundTaskSubject = nullptr; + for( const auto &annotation : annotations->standardAnnotations ) + { + backgroundTaskSubject = plugValueSubstitutionsBackgroundTaskSubject( annotation.text(), node ); + if( backgroundTaskSubject ) + { + break; + } + } + + if( !backgroundTaskSubject ) + { + applySubstitutedRenderText( substitutedRenderText( node, *annotations ), *annotations ); + return; + } + + // At least one annotation depends on a computed plug value. Evaluate all + // substitutions in a background task, so that we don't lock up the UI if + // the computation is slow. Before we launch background task, substitute + // in `---` placeholders to give folks a hint as to what is happening. + + applySubstitutedRenderText( substitutedRenderText( /* node = */ nullptr, *annotations ), *annotations ); + + annotations->substitutionsTask = ParallelAlgo::callOnBackgroundThread( + + // `this`, `node` and `annotations` are safe to capture because they are + // guaranteed to outlive the BackgroundTask, due to `annotations` owning + // the task, and waiting for it in its destructor. + backgroundTaskSubject, [this, node, annotations] () { + + // Get new render text for each annotation. Note : We can not access + // the Metatada API from a BackgroundTask, as it doesn't participate + // in cancellation. + + bool cancelled = false; + std::unordered_map renderText; + try + { + renderText = substitutedRenderText( node, *annotations ); + } + catch( const IECore::Cancelled & ) + { + cancelled = true; + } + + if( !cancelled && renderText.empty() ) + { + return; + } + + // Schedule an update on the UI thread. + // + // Note : As soon as the background task returns, we have lost the + // guarantee on the lifetime of `this`, `node` and `annotations` - + // any could be destroyed before we get on to the UI thread. So + // maintain ownership and look up `annotations` again. + + // Alias for `this` to work around MSVC bug that prevents capturing + // `this` again in a nested lambda. + AnnotationsGadget *that = this; + + ParallelAlgo::callOnUIThread( + [gadget = Ptr( that ), node = ConstNodePtr( node ), renderText = std::move( renderText ), cancelled] { + + Annotations *annotations = gadget->annotations( node.get() ); + if( !annotations ) + { + return; + } + + gadget->applySubstitutedRenderText( renderText, *annotations ); + if( cancelled ) + { + // Dirty, so that we relaunch background task on next redraw. + annotations->dirty = true; + gadget->m_dirty = true; + } + gadget->dirty( DirtyType::Render ); + } + ); + + } + ); +} + +std::unordered_map AnnotationsGadget::substitutedRenderText( const Gaffer::Node *node, const Annotations &annotations ) +{ + std::unordered_map result; + + for( auto &annotation : annotations.standardAnnotations ) + { + try + { + const string newRenderText = wrap( substitutePlugValues( annotation.text(), node ), g_maxLineLength ); + if( newRenderText != annotation.renderText ) + { + result[annotation.name] = newRenderText; + } + } + catch( const ProcessException & ) + { + // Computation error. Ignore, so we keep original `---` placeholder + // text. + } + } + + return result; +} + +void AnnotationsGadget::applySubstitutedRenderText( const std::unordered_map &renderText, Annotations &annotations ) +{ + for( auto &annotation : annotations.standardAnnotations ) + { + auto it = renderText.find( annotation.name ); + if( it != renderText.end() ) + { + annotation.renderText = it->second; + } + } +} + +void AnnotationsGadget::visibilityChanged() +{ + if( !visible() ) + { + for( auto &[nodeGadget, annotation] : m_annotations ) + { + // Cancel background task. If not yet complete, it will catch this + // cancellation and dirty the annotation so that a new update starts + // when we are next visible. + annotation.substitutionsTask.reset(); + } + } +} diff --git a/src/GafferUIModule/GraphGadgetBinding.cpp b/src/GafferUIModule/GraphGadgetBinding.cpp index 65ed01e704b..8be04f972f7 100644 --- a/src/GafferUIModule/GraphGadgetBinding.cpp +++ b/src/GafferUIModule/GraphGadgetBinding.cpp @@ -298,6 +298,7 @@ void GafferUIModule::bindGraphGadget() .def_readonly( "untemplatedAnnotations", &AnnotationsGadget::untemplatedAnnotations ) .def( "setVisibleAnnotations", &AnnotationsGadget::setVisibleAnnotations ) .def( "getVisibleAnnotations", &AnnotationsGadget::getVisibleAnnotations, return_value_policy() ) + .def( "annotationText", &AnnotationsGadget::annotationText, return_value_policy(), ( arg( "node" ), arg( "annotation" ) = "user" ) ) ; IECorePython::RunTimeTypedClass()