From 89b49af80bc62c289b8ae07657e705168ec0dd29 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 15 May 2024 16:55:32 +0100 Subject: [PATCH 1/9] AnnotationsUI : Add Ctrl + Enter shortcut --- Changes.md | 1 + python/GafferUI/AnnotationsUI.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/Changes.md b/Changes.md index ed4f458d796..bfafb115fec 100644 --- a/Changes.md +++ b/Changes.md @@ -6,6 +6,7 @@ Improvements - ColorChooser : Added channel names to identify sliders. - RenderPassEditor : Added "Select Affected Objects" popup menu item. +- Annotations : Added Ctrl + Enter keyboard shortcut to annotation dialogue. This applies the annotation and closes the dialogue. Fixes ----- diff --git a/python/GafferUI/AnnotationsUI.py b/python/GafferUI/AnnotationsUI.py index ee6b4241b8a..5c1bf926958 100644 --- a/python/GafferUI/AnnotationsUI.py +++ b/python/GafferUI/AnnotationsUI.py @@ -91,6 +91,9 @@ def __init__( self, node, name ) : self.__textWidget.textChangedSignal().connect( Gaffer.WeakMethod( self.__updateButtonStatus ), scoped = False ) + self.__textWidget.activatedSignal().connect( + Gaffer.WeakMethod( self.__textActivated ), scoped = False + ) if not template : self.__colorChooser = GafferUI.ColorChooser( @@ -147,3 +150,8 @@ def __makeAnnotation( self ) : ) else : return Gaffer.MetadataAlgo.Annotation( self.__textWidget.getText() ) + + def __textActivated( self, *unused ) : + + if self.__annotateButton.getEnabled() : + self.__annotateButton.clickedSignal()( self.__annotateButton ) From 3999073110c219ec6aac01847d8850a9857a478b Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 15 May 2024 11:07:00 +0100 Subject: [PATCH 2/9] AnnotationsGadget : Make global variable const --- src/GafferUI/AnnotationsGadget.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/GafferUI/AnnotationsGadget.cpp b/src/GafferUI/AnnotationsGadget.cpp index c3c71192917..92a7c9ebdd4 100644 --- a/src/GafferUI/AnnotationsGadget.cpp +++ b/src/GafferUI/AnnotationsGadget.cpp @@ -116,7 +116,8 @@ string wrap( const std::string &text, size_t maxLineLength ) return result; } -float g_offset = 0.5; + +const float g_offset = 0.5; } // namespace From f5d2129b2d104b6b0ed539d1028c8ee507670513 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Thu, 13 Jun 2024 15:06:03 +0100 Subject: [PATCH 3/9] AnnotationsGadget : Add `annotationText()` method This will facilitate testing as we add plug value substitutions in a future commit. --- Changes.md | 5 ++ include/GafferUI/AnnotationsGadget.h | 12 +++- python/GafferUITest/AnnotationsGadgetTest.py | 64 ++++++++++++++++++++ python/GafferUITest/__init__.py | 1 + src/GafferUI/AnnotationsGadget.cpp | 30 ++++++++- src/GafferUIModule/GraphGadgetBinding.cpp | 1 + 6 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 python/GafferUITest/AnnotationsGadgetTest.py diff --git a/Changes.md b/Changes.md index bfafb115fec..f344ba21841 100644 --- a/Changes.md +++ b/Changes.md @@ -14,6 +14,11 @@ 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. + 1.4.7.0 (relative to 1.4.6.0) ======= diff --git a/include/GafferUI/AnnotationsGadget.h b/include/GafferUI/AnnotationsGadget.h index dcc407d8d98..1346e4b8882 100644 --- a/include/GafferUI/AnnotationsGadget.h +++ b/include/GafferUI/AnnotationsGadget.h @@ -74,6 +74,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 : @@ -98,10 +102,16 @@ class GAFFERUI_API AnnotationsGadget : public Gadget void nodeMetadataChanged( IECore::TypeId nodeTypeId, IECore::InternedString key, Gaffer::Node *node ); void update() const; + struct StandardAnnotation : public Gaffer::MetadataAlgo::Annotation + { + StandardAnnotation( const Gaffer::MetadataAlgo::Annotation &a, IECore::InternedString name ) : Annotation( a ), name( name ) {} + IECore::InternedString name; + }; + struct Annotations { bool dirty = true; - std::vector standardAnnotations; + std::vector standardAnnotations; bool bookmarked = false; IECore::InternedString numericBookmark; bool renderable = false; diff --git a/python/GafferUITest/AnnotationsGadgetTest.py b/python/GafferUITest/AnnotationsGadgetTest.py new file mode 100644 index 00000000000..fcce91e9a81 --- /dev/null +++ b/python/GafferUITest/AnnotationsGadgetTest.py @@ -0,0 +1,64 @@ +########################################################################## +# +# 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 unittest + +import Gaffer +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"] ), "" ) + +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 92a7c9ebdd4..540a771306f 100644 --- a/src/GafferUI/AnnotationsGadget.cpp +++ b/src/GafferUI/AnnotationsGadget.cpp @@ -118,6 +118,7 @@ string wrap( const std::string &text, size_t maxLineLength ) } const float g_offset = 0.5; +const string g_emptyString; } // namespace @@ -162,6 +163,33 @@ 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(); + + const NodeGadget *nodeGadget = graphGadget()->nodeGadget( node ); + if( !nodeGadget ) + { + return g_emptyString; + } + + auto it = m_annotations.find( nodeGadget ); + if( it == m_annotations.end() ) + { + return g_emptyString; + } + + for( const auto &a : it->second.standardAnnotations ) + { + if( a.name == annotation ) + { + return a.text(); + } + } + + return g_emptyString; +} + bool AnnotationsGadget::acceptsParent( const GraphComponent *potentialParent ) const { return runTimeCast( potentialParent ); @@ -365,7 +393,7 @@ void AnnotationsGadget::update() const } annotations.standardAnnotations.push_back( - MetadataAlgo::getAnnotation( node, name, /* inheritTemplate = */ true ) + StandardAnnotation( MetadataAlgo::getAnnotation( node, name, /* inheritTemplate = */ true ), name ) ); // Word wrap. It might be preferable to do this during // rendering, but we have no way of querying the extent of 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() From 80f0e603d542f8530f15da9a5cd4f47757822426 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 14 Jun 2024 11:31:55 +0100 Subject: [PATCH 4/9] ParallelAlgoTest : Add `UIThreadCallHandler.receive()` method --- Changes.md | 1 + python/GafferTest/ParallelAlgoTest.py | 16 +++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index f344ba21841..2d2721d7099 100644 --- a/Changes.md +++ b/Changes.md @@ -18,6 +18,7 @@ API --- - AnnotationsGadget : Added `annotationText()` method. +- ParallelAlgoTest : Added `UIThreadCallHandler.receive()` method. 1.4.7.0 (relative to 1.4.6.0) ======= 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. From 9366a5ba2e11c4eb12cb703c1b55ae7692e2008a Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 14 Jun 2024 14:13:33 +0100 Subject: [PATCH 5/9] AnnotationsGadget : Fix crashes if gadget outlives GraphGadget This is highly unlikely in practice at this very moment. But in an upcoming commit we're going to be doing some threading shenanigans that means that it is possible if a GraphEditor is destroyed while the AnnotationsGadget does work on a background thread. The code is also simplified a little by having an `annotations()` method compared to an error-prone `graphGadget()` method. --- include/GafferUI/AnnotationsGadget.h | 5 +- python/GafferUITest/AnnotationsGadgetTest.py | 12 ++++ src/GafferUI/AnnotationsGadget.cpp | 66 +++++++++++--------- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/include/GafferUI/AnnotationsGadget.h b/include/GafferUI/AnnotationsGadget.h index 1346e4b8882..39da1db88a7 100644 --- a/include/GafferUI/AnnotationsGadget.h +++ b/include/GafferUI/AnnotationsGadget.h @@ -94,11 +94,12 @@ class GAFFERUI_API AnnotationsGadget : public Gadget private : - GraphGadget *graphGadget(); - const GraphGadget *graphGadget() const; + struct Annotations; void graphGadgetChildAdded( GraphComponent *child ); void graphGadgetChildRemoved( const GraphComponent *child ); + Annotations *annotations( const Gaffer::Node *node ); + const Annotations *annotations( const Gaffer::Node *node ) const; void nodeMetadataChanged( IECore::TypeId nodeTypeId, IECore::InternedString key, Gaffer::Node *node ); void update() const; diff --git a/python/GafferUITest/AnnotationsGadgetTest.py b/python/GafferUITest/AnnotationsGadgetTest.py index fcce91e9a81..65fd906b166 100644 --- a/python/GafferUITest/AnnotationsGadgetTest.py +++ b/python/GafferUITest/AnnotationsGadgetTest.py @@ -60,5 +60,17 @@ def testSimpleText( self ) : 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"] ), "" ) + if __name__ == "__main__": unittest.main() diff --git a/src/GafferUI/AnnotationsGadget.cpp b/src/GafferUI/AnnotationsGadget.cpp index 540a771306f..4aceced0204 100644 --- a/src/GafferUI/AnnotationsGadget.cpp +++ b/src/GafferUI/AnnotationsGadget.cpp @@ -166,24 +166,14 @@ const IECore::StringAlgo::MatchPattern &AnnotationsGadget::getVisibleAnnotations const std::string &AnnotationsGadget::annotationText( const Gaffer::Node *node, IECore::InternedString annotation ) const { const_cast( this )->update(); - - const NodeGadget *nodeGadget = graphGadget()->nodeGadget( node ); - if( !nodeGadget ) - { - return g_emptyString; - } - - auto it = m_annotations.find( nodeGadget ); - if( it == m_annotations.end() ) - { - return g_emptyString; - } - - for( const auto &a : it->second.standardAnnotations ) + if( const Annotations *nodeAnnotations = annotations( node ) ) { - if( a.name == annotation ) + for( const auto &a : nodeAnnotations->standardAnnotations ) { - return a.text(); + if( a.name == annotation ) + { + return a.text(); + } } } @@ -285,16 +275,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 ) ) @@ -313,6 +293,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 ) @@ -331,11 +339,9 @@ 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 ); } From 2283ca2febeb92ebc96a0dbf0327460aaf67485e Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 14 Jun 2024 11:58:41 +0100 Subject: [PATCH 6/9] AnnotationsGadget : Add `{plug}` syntax for substituting node values --- Changes.md | 4 +- include/GafferUI/AnnotationsGadget.h | 56 ++- python/GafferUITest/AnnotationsGadgetTest.py | 348 +++++++++++++++++ src/GafferUI/AnnotationsGadget.cpp | 378 ++++++++++++++++++- 4 files changed, 766 insertions(+), 20 deletions(-) diff --git a/Changes.md b/Changes.md index 2d2721d7099..993271fad06 100644 --- a/Changes.md +++ b/Changes.md @@ -6,7 +6,9 @@ Improvements - ColorChooser : Added channel names to identify sliders. - RenderPassEditor : Added "Select Affected Objects" popup menu item. -- Annotations : Added Ctrl + Enter keyboard shortcut to annotation dialogue. This applies the annotation and closes the dialogue. +- 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 ----- diff --git a/include/GafferUI/AnnotationsGadget.h b/include/GafferUI/AnnotationsGadget.h index 39da1db88a7..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" @@ -96,17 +97,61 @@ class GAFFERUI_API AnnotationsGadget : public Gadget 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 @@ -116,14 +161,19 @@ class GAFFERUI_API AnnotationsGadget : public Gadget 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/GafferUITest/AnnotationsGadgetTest.py b/python/GafferUITest/AnnotationsGadgetTest.py index 65fd906b166..ae722e447c5 100644 --- a/python/GafferUITest/AnnotationsGadgetTest.py +++ b/python/GafferUITest/AnnotationsGadgetTest.py @@ -34,9 +34,16 @@ # ########################################################################## +import inspect +import threading import unittest +import imath + +import IECore + import Gaffer +import GafferTest import GafferUI import GafferUITest @@ -72,5 +79,346 @@ def testExtendLifetimePastGraphGadget( self ) : 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/src/GafferUI/AnnotationsGadget.cpp b/src/GafferUI/AnnotationsGadget.cpp index 4aceced0204..44e8b7bb437 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; @@ -119,6 +124,139 @@ string wrap( const std::string &text, size_t maxLineLength ) 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 @@ -136,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() @@ -172,7 +313,7 @@ const std::string &AnnotationsGadget::annotationText( const Gaffer::Node *node, { if( a.name == annotation ) { - return a.text(); + return a.renderText; } } } @@ -208,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 ) { @@ -257,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 ); } } } @@ -347,18 +488,23 @@ void AnnotationsGadget::nodeMetadataChanged( IECore::TypeId nodeTypeId, IECore:: } } -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 ) { @@ -384,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 ) @@ -398,20 +547,217 @@ void AnnotationsGadget::update() const } } - annotations.standardAnnotations.push_back( - StandardAnnotation( MetadataAlgo::getAnnotation( node, name, /* inheritTemplate = */ true ), name ) - ); - // 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. + + ParallelAlgo::callOnUIThread( + [gadget = Ptr( this ), 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(); + } + } +} From c8e88e6480e7a709ad2dac6daf903f9350d351ec Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 15 May 2024 18:12:08 +0100 Subject: [PATCH 7/9] AnnotationsUI : Add autocomplete and highlighting for plug references --- python/GafferUI/AnnotationsUI.py | 72 +++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/python/GafferUI/AnnotationsUI.py b/python/GafferUI/AnnotationsUI.py index 5c1bf926958..a1a4268d4f2 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,9 +152,12 @@ 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 ) From e8a14175efe8894280991277fe178659361626b0 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Mon, 17 Jun 2024 15:30:21 +0100 Subject: [PATCH 8/9] AnnotationsGadget : Work around MSVC bug With the original code we were getting this compilation error : ``` C:\Users\John\dev\gaffer\src\GafferUI\AnnotationsGadget.cpp(694): error C2440: '': cannot convert from 'const GafferUI::AnnotationsGadget::schedulePlugValueSubstitutions:: *' to 'GafferUI::AnnotationsGadget::Ptr' ``` Looks like MSVC thought `this` was a pointer to the lambda and not to the AnnotationsGadget. --- src/GafferUI/AnnotationsGadget.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/GafferUI/AnnotationsGadget.cpp b/src/GafferUI/AnnotationsGadget.cpp index 44e8b7bb437..b11b36aa5ee 100644 --- a/src/GafferUI/AnnotationsGadget.cpp +++ b/src/GafferUI/AnnotationsGadget.cpp @@ -688,8 +688,12 @@ void AnnotationsGadget::schedulePlugValueSubstitutions( const Gaffer::Node *node // 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( this ), node = ConstNodePtr( node ), renderText = std::move( renderText ), cancelled] { + [gadget = Ptr( that ), node = ConstNodePtr( node ), renderText = std::move( renderText ), cancelled] { Annotations *annotations = gadget->annotations( node.get() ); if( !annotations ) From 854415c4edcd7836f94ee7255cb8636dc9ac07d6 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Wed, 19 Jun 2024 09:48:06 +0100 Subject: [PATCH 9/9] AnnotationsUI : Add context menu for inserting plug values --- python/GafferUI/AnnotationsUI.py | 41 +++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/python/GafferUI/AnnotationsUI.py b/python/GafferUI/AnnotationsUI.py index a1a4268d4f2..c1195c6d580 100644 --- a/python/GafferUI/AnnotationsUI.py +++ b/python/GafferUI/AnnotationsUI.py @@ -164,7 +164,9 @@ def __init__( self, node, name ) : 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 ), @@ -225,3 +227,40 @@ 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