From 11b332fffb318dce406bb084bb1d9d3c8b71fce5 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 30 Jun 2023 15:48:57 +0100 Subject: [PATCH] BackgroundTask : Warn if ScriptNode can't be found Not being able to find it can lead to very hard to diagnose bugs (see 67aea86), so we need to know any time that happens. This means updating a few tests that weren't using a ScriptNode at all. Technically, several of the tests could have been updated to pass `subject = None` instead, because they're not relying on graph edits causing cancellation. But I think it's better to update them to show the more common real-world use case instead. --- Changes.md | 1 + python/GafferImageTest/MedianTest.py | 15 ++-- python/GafferImageTest/ResampleTest.py | 16 +++-- .../GafferSceneTest/RenderControllerTest.py | 28 ++++---- .../GafferSceneUITest/CropWindowToolTest.py | 28 ++++---- python/GafferSceneUITest/SceneGadgetTest.py | 70 ++++++++++--------- python/GafferTest/ValuePlugTest.py | 13 ++-- .../GafferUITest/BoolPlugValueWidgetTest.py | 14 ++-- .../GafferUITest/StringPlugValueWidgetTest.py | 10 +-- python/GafferVDBTest/MeshToLevelSetTest.py | 20 +++--- src/Gaffer/BackgroundTask.cpp | 10 ++- 11 files changed, 126 insertions(+), 99 deletions(-) diff --git a/Changes.md b/Changes.md index 284e756d453..77921afb0fc 100644 --- a/Changes.md +++ b/Changes.md @@ -233,6 +233,7 @@ API - Added support for creating a `PathMatcherDataPlug` from `PathMatcherData` in `createPlugFromData()`. - Added support for getting `PathMatcherData` from a `PathMatcherDataPlug` in `getValueAsData()`. - EditScopeAlgo : Added support for editing options. +- BackgroundTask : A warning is now emitted if the `subject` can not be associated with a ScriptNode for cancellation purposes. 1.2.8.0 (relative to 1.2.7.0) ======= diff --git a/python/GafferImageTest/MedianTest.py b/python/GafferImageTest/MedianTest.py index 9ebaf1c64ff..d3ccc462be9 100644 --- a/python/GafferImageTest/MedianTest.py +++ b/python/GafferImageTest/MedianTest.py @@ -145,13 +145,14 @@ def testDriverChannel( self ) : def testCancellation( self ) : - c = GafferImage.Constant() + script = Gaffer.ScriptNode() + script["c"] = GafferImage.Constant() - m = GafferImage.Median() - m["in"].setInput( c["out"] ) - m["radius"].setValue( imath.V2i( 2000 ) ) + script["m"] = GafferImage.Median() + script["m"]["in"].setInput( script["c"]["out"] ) + script["m"]["radius"].setValue( imath.V2i( 2000 ) ) - bt = Gaffer.ParallelAlgo.callOnBackgroundThread( m["out"], lambda : GafferImageTest.processTiles( m["out"] ) ) + bt = Gaffer.ParallelAlgo.callOnBackgroundThread( script["m"]["out"], lambda : GafferImageTest.processTiles( script["m"]["out"] ) ) # Give background tasks time to get into full swing time.sleep( 0.1 ) @@ -163,9 +164,9 @@ def testCancellation( self ) : # Check that we can do the same when using a master # channel. - m["masterChannel"].setValue( "R" ) + script["m"]["masterChannel"].setValue( "R" ) - bt = Gaffer.ParallelAlgo.callOnBackgroundThread( m["out"], lambda : GafferImageTest.processTiles( m["out"] ) ) + bt = Gaffer.ParallelAlgo.callOnBackgroundThread( script["m"]["out"], lambda : GafferImageTest.processTiles( script["m"]["out"] ) ) time.sleep( 0.1 ) t = time.time() diff --git a/python/GafferImageTest/ResampleTest.py b/python/GafferImageTest/ResampleTest.py index 0f8f3ce39f0..0acd3fe43cc 100644 --- a/python/GafferImageTest/ResampleTest.py +++ b/python/GafferImageTest/ResampleTest.py @@ -188,13 +188,15 @@ def testExpandDataWindow( self ) : def testCancellation( self ) : - c = GafferImage.Constant() + script = Gaffer.ScriptNode() - r = GafferImage.Resample() - r["in"].setInput( c["out"] ) - r["filterScale"].setValue( imath.V2f( 2000 ) ) + script["c"] = GafferImage.Constant() + + script["r"] = GafferImage.Resample() + script["r"]["in"].setInput( script["c"]["out"] ) + script["r"]["filterScale"].setValue( imath.V2f( 2000 ) ) - bt = Gaffer.ParallelAlgo.callOnBackgroundThread( r["out"], lambda : GafferImageTest.processTiles( r["out"] ) ) + bt = Gaffer.ParallelAlgo.callOnBackgroundThread( script["r"]["out"], lambda : GafferImageTest.processTiles( script["r"]["out"] ) ) # Give background tasks time to get into full swing time.sleep( 0.1 ) @@ -205,9 +207,9 @@ def testCancellation( self ) : self.assertLess( time.time() - t, acceptableCancellationDelay ) # Check that we can do the same when using a non-separable filter - r["filter"].setValue( "disk" ) + script["r"]["filter"].setValue( "disk" ) - bt = Gaffer.ParallelAlgo.callOnBackgroundThread( r["out"], lambda : GafferImageTest.processTiles( r["out"] ) ) + bt = Gaffer.ParallelAlgo.callOnBackgroundThread( script["r"]["out"], lambda : GafferImageTest.processTiles( script["r"]["out"] ) ) time.sleep( 0.1 ) t = time.time() diff --git a/python/GafferSceneTest/RenderControllerTest.py b/python/GafferSceneTest/RenderControllerTest.py index 235d0cb5669..e6f3b4b37e8 100644 --- a/python/GafferSceneTest/RenderControllerTest.py +++ b/python/GafferSceneTest/RenderControllerTest.py @@ -844,21 +844,23 @@ def testIDs( self ) : def testProgressCallback( self ) : - sphere = GafferScene.Sphere() - group = GafferScene.Group() - group["in"][0].setInput( sphere["out"] ) - group["in"][1].setInput( sphere["out"] ) - group["in"][2].setInput( sphere["out"] ) + script = Gaffer.ScriptNode() - allFilter = GafferScene.PathFilter() - allFilter["paths"].setValue( IECore.StringVectorData( [ "/*", "/*/*" ] ) ) + script["sphere"] = GafferScene.Sphere() + script["group"] = GafferScene.Group() + script["group"]["in"][0].setInput( script["sphere"]["out"] ) + script["group"]["in"][1].setInput( script["sphere"]["out"] ) + script["group"]["in"][2].setInput( script["sphere"]["out"] ) + + script["allFilter"] = GafferScene.PathFilter() + script["allFilter"]["paths"].setValue( IECore.StringVectorData( [ "/*", "/*/*" ] ) ) - transform = GafferScene.Transform() - transform["in"].setInput( group["out"] ) - transform["filter"].setInput( allFilter["out"] ) + script["transform"] = GafferScene.Transform() + script["transform"]["in"].setInput( script["group"]["out"] ) + script["transform"]["filter"].setInput( script["allFilter"]["out"] ) renderer = GafferScene.Private.IECoreScenePreview.CapturingRenderer() - controller = GafferScene.RenderController( transform["out"], Gaffer.Context(), renderer ) + controller = GafferScene.RenderController( script["transform"]["out"], Gaffer.Context(), renderer ) controller.setMinimumExpansionDepth( 2 ) statuses = [] @@ -882,7 +884,7 @@ def callback( status ) : # path we requested, and it's ancestors (not including the root, because # its transform hasn't changed). del statuses[:] - transform["transform"]["translate"]["x"].setValue( 1 ) + script["transform"]["transform"]["translate"]["x"].setValue( 1 ) controller.updateMatchingPaths( IECore.PathMatcher( [ "/group/sphere" ] ), callback ) self.assertEqual( statuses, [ Status.Running ] * 2 + [ Status.Completed ] ) @@ -890,7 +892,7 @@ def callback( status ) : # We expect updates to all locations (except the root, because its transform # hasn't changed). del statuses[:] - transform["transform"]["translate"]["x"].setValue( 2 ) + script["transform"]["transform"]["translate"]["x"].setValue( 2 ) task = controller.updateInBackground( callback, priorityPaths = IECore.PathMatcher( [ "/group/sphere" ] ) ) task.wait() self.assertEqual( statuses, [ Status.Running ] * 4 + [ Status.Completed ] ) diff --git a/python/GafferSceneUITest/CropWindowToolTest.py b/python/GafferSceneUITest/CropWindowToolTest.py index 43d6f879c51..cd9f1408033 100644 --- a/python/GafferSceneUITest/CropWindowToolTest.py +++ b/python/GafferSceneUITest/CropWindowToolTest.py @@ -46,9 +46,11 @@ class CropWindowToolTest( GafferUITest.TestCase ) : def testSceneViewStatus( self ) : - camera = GafferScene.Camera() + script = Gaffer.ScriptNode() + + script["camera"] = GafferScene.Camera() - view = GafferUI.View.create( camera["out"] ) + view = GafferUI.View.create( script["camera"]["out"] ) tool = GafferSceneUI.CropWindowTool( view ) tool["active"].setValue( True ) @@ -75,28 +77,28 @@ def testSceneViewStatus( self ) : # Editable - options = GafferScene.StandardOptions() - options["in"].setInput( camera["out"] ) - view["in"].setInput( options["out"] ) + script["options"] = GafferScene.StandardOptions() + script["options"]["in"].setInput( script["camera"]["out"] ) + view["in"].setInput( script["options"]["out"] ) self.waitForIdle( 1000 ) - self.assertEqual( tool.status(), "Info: Editing StandardOptions.options.renderCropWindow.value" ) + self.assertEqual( tool.status(), "Info: Editing options.options.renderCropWindow.value" ) # Locked value plug - Gaffer.MetadataAlgo.setReadOnly( options["options"]["renderCropWindow"]["value"], True ) + Gaffer.MetadataAlgo.setReadOnly( script["options"]["options"]["renderCropWindow"]["value"], True ) self.waitForIdle( 1000 ) - self.assertEqual( tool.status(), "Warning: StandardOptions.options.renderCropWindow.value is locked" ) + self.assertEqual( tool.status(), "Warning: options.options.renderCropWindow.value is locked" ) # Locked/off enabled plug - Gaffer.MetadataAlgo.setReadOnly( options["options"]["renderCropWindow"]["value"], False ) - options["options"]["renderCropWindow"]["enabled"].setValue( False ) - Gaffer.MetadataAlgo.setReadOnly( options["options"]["renderCropWindow"]["enabled"], True ) + Gaffer.MetadataAlgo.setReadOnly( script["options"]["options"]["renderCropWindow"]["value"], False ) + script["options"]["options"]["renderCropWindow"]["enabled"].setValue( False ) + Gaffer.MetadataAlgo.setReadOnly( script["options"]["options"]["renderCropWindow"]["enabled"], True ) self.waitForIdle( 1000 ) - self.assertEqual( tool.status(), "Warning: StandardOptions.options.renderCropWindow.value isn't editable" ) + self.assertEqual( tool.status(), "Warning: options.options.renderCropWindow.value isn't editable" ) # Check status across visible/invisible overlay transitions (this is # really testing one of the gnarly parts of the status implementation @@ -110,7 +112,7 @@ def testSceneViewStatus( self ) : view["camera"]["lookThroughEnabled"].setValue( True ) self.waitForIdle( 1000 ) - self.assertEqual( tool.status(), "Warning: StandardOptions.options.renderCropWindow.value isn't editable" ) + self.assertEqual( tool.status(), "Warning: options.options.renderCropWindow.value isn't editable" ) def testImageViewStatus( self ) : diff --git a/python/GafferSceneUITest/SceneGadgetTest.py b/python/GafferSceneUITest/SceneGadgetTest.py index 9b0eefd5548..efa2a34f842 100644 --- a/python/GafferSceneUITest/SceneGadgetTest.py +++ b/python/GafferSceneUITest/SceneGadgetTest.py @@ -371,23 +371,25 @@ def testExceptionsDuringCompute( self ) : def testObjectsAtBox( self ) : - plane = GafferScene.Plane() + script = Gaffer.ScriptNode() - sphere = GafferScene.Sphere() - sphere["radius"].setValue( 0.25 ) + script["plane"] = GafferScene.Plane() - instancer = GafferScene.Instancer() - instancer["in"].setInput( plane["out"] ) - instancer["prototypes"].setInput( sphere["out"] ) - instancer["parent"].setValue( "/plane" ) + script["sphere"] = GafferScene.Sphere() + script["sphere"]["radius"].setValue( 0.25 ) - subTree = GafferScene.SubTree() - subTree["in"].setInput( instancer["out"] ) - subTree["root"].setValue( "/plane" ) + script["instancer"] = GafferScene.Instancer() + script["instancer"]["in"].setInput( script["plane"]["out"] ) + script["instancer"]["prototypes"].setInput( script["sphere"]["out"] ) + script["instancer"]["parent"].setValue( "/plane" ) + + script["subTree"] = GafferScene.SubTree() + script["subTree"]["in"].setInput( script["instancer"]["out"] ) + script["subTree"]["root"].setValue( "/plane" ) sg = GafferSceneUI.SceneGadget() sg.setRenderer( self.renderer ) - sg.setScene( subTree["out"] ) + sg.setScene( script["subTree"]["out"] ) sg.setMinimumExpansionDepth( 100 ) with GafferUI.Window() as w : @@ -436,21 +438,19 @@ def testObjectsAtBox( self ) : def testObjectAtLine( self ) : - cubes = [] - names = ( "left", "center", "right" ) - for i in range( 3 ) : + script = Gaffer.ScriptNode() + script["group"] = GafferScene.Group() + + for i, name in enumerate( [ "left", "center", "right" ] ) : cube = GafferScene.Cube() cube["transform"]["translate"].setValue( imath.V3f( ( i - 1 ) * 2.0, 0.0, -2.5 ) ) - cube["name"].setValue( names[i] ) - cubes.append( cube ) - - group = GafferScene.Group() - for i, cube in enumerate( cubes ) : - group["in"][i].setInput( cube["out"] ) + cube["name"].setValue( name ) + script.addChild( cube ) + script["group"]["in"][i].setInput( cube["out"] ) sg = GafferSceneUI.SceneGadget() sg.setRenderer( self.renderer ) - sg.setScene( group["out"] ) + sg.setScene( script["group"]["out"] ) sg.setMinimumExpansionDepth( 100 ) with GafferUI.Window() as w : @@ -532,13 +532,15 @@ def testSetAndGetScene( self ) : def testBoundOfUnexpandedEmptyChildren( self ) : - group1 = GafferScene.Group() - group2 = GafferScene.Group() - group2["in"][0].setInput( group1["out"] ) + script = Gaffer.ScriptNode() + + script["group1"] = GafferScene.Group() + script["group2"] = GafferScene.Group() + script["group2"]["in"][0].setInput( script["group1"]["out"] ) sg = GafferSceneUI.SceneGadget() sg.setRenderer( self.renderer ) - sg.setScene( group2["out"] ) + sg.setScene( script["group2"]["out"] ) self.waitForRender( sg ) self.assertEqual( sg.bound(), imath.Box3f() ) @@ -563,18 +565,20 @@ def testSelectionMaskAccessors( self ) : def testSelectionMask( self ) : - plane = GafferScene.Plane() - plane["dimensions"].setValue( imath.V2f( 10 ) ) - plane["transform"]["translate"]["z"].setValue( 4 ) + script = Gaffer.ScriptNode() + + script["plane"] = GafferScene.Plane() + script["plane"]["dimensions"].setValue( imath.V2f( 10 ) ) + script["plane"]["transform"]["translate"]["z"].setValue( 4 ) - camera = GafferScene.Camera() - group = GafferScene.Group() - group["in"][0].setInput( plane["out"] ) - group["in"][1].setInput( camera["out"] ) + script["camera"] = GafferScene.Camera() + script["group"] = GafferScene.Group() + script["group"]["in"][0].setInput( script["plane"]["out"] ) + script["group"]["in"][1].setInput( script["camera"]["out"] ) sg = GafferSceneUI.SceneGadget() sg.setRenderer( self.renderer ) - sg.setScene( group["out"] ) + sg.setScene( script["group"]["out"] ) sg.setMinimumExpansionDepth( 100 ) with GafferUI.Window() as w : diff --git a/python/GafferTest/ValuePlugTest.py b/python/GafferTest/ValuePlugTest.py index cb0bb0d7525..abe401b6bc2 100644 --- a/python/GafferTest/ValuePlugTest.py +++ b/python/GafferTest/ValuePlugTest.py @@ -953,13 +953,14 @@ def computeCachePolicy( self, output ) : # in `LRUCachePolicy.TaskParallel.Handle.acquire`. ) : - node = InfiniteLoop( cachePolicy = cachePolicy ) + script = Gaffer.ScriptNode() + script["node"] = InfiniteLoop( cachePolicy = cachePolicy ) # Launch a compute in the background, and wait for it to start. - with node.computeStartedCondition : - backgroundTask1 = Gaffer.ParallelAlgo.callOnBackgroundThread( node["out"], lambda : node["out"].getValue() ) - node.computeStartedCondition.wait() + with script["node"].computeStartedCondition : + backgroundTask1 = Gaffer.ParallelAlgo.callOnBackgroundThread( script["node"]["out"], lambda : script["node"]["out"].getValue() ) + script["node"].computeStartedCondition.wait() # Launch a second compute in the background, wait for it to start, and # then make sure we can cancel it even though the compute is already in @@ -973,10 +974,10 @@ def getValueExpectingCancellation() : startedCondition.notify() with self.assertRaises( IECore.Cancelled ) : - node["out"].getValue() + script["node"]["out"].getValue() with startedCondition : - backgroundTask2 = Gaffer.ParallelAlgo.callOnBackgroundThread( node["out"], getValueExpectingCancellation ) + backgroundTask2 = Gaffer.ParallelAlgo.callOnBackgroundThread( script["node"]["out"], getValueExpectingCancellation ) startedCondition.wait() backgroundTask2.cancelAndWait() diff --git a/python/GafferUITest/BoolPlugValueWidgetTest.py b/python/GafferUITest/BoolPlugValueWidgetTest.py index abd4b5758a6..6c9651cc6e8 100644 --- a/python/GafferUITest/BoolPlugValueWidgetTest.py +++ b/python/GafferUITest/BoolPlugValueWidgetTest.py @@ -83,18 +83,20 @@ def testInitialValue( self ) : def testErrorHandling( self ) : - n = Gaffer.Node() - n["user"]["p"] = Gaffer.BoolPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) + script = Gaffer.ScriptNode() + + script["n"] = Gaffer.Node() + script["n"]["user"]["p"] = Gaffer.BoolPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic ) - w = GafferUI.BoolPlugValueWidget( n["user"]["p"] ) + w = GafferUI.BoolPlugValueWidget( script["n"]["user"]["p"] ) self.assertFalse( w.boolWidget().getErrored() ) - b = GafferTest.BadNode() - n["user"]["p"].setInput( b["out3"] ) + script["b"] = GafferTest.BadNode() + script["n"]["user"]["p"].setInput( script["b"]["out3"] ) GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) self.assertTrue( w.boolWidget().getErrored() ) - n["user"]["p"].setInput( None ) + script["n"]["user"]["p"].setInput( None ) GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) self.assertFalse( w.boolWidget().getErrored() ) diff --git a/python/GafferUITest/StringPlugValueWidgetTest.py b/python/GafferUITest/StringPlugValueWidgetTest.py index 099bcbfdf70..27bdba366ed 100644 --- a/python/GafferUITest/StringPlugValueWidgetTest.py +++ b/python/GafferUITest/StringPlugValueWidgetTest.py @@ -141,15 +141,17 @@ def testMixedValuesPreserved( self ) : def testExceptionHandling( self ) : - # Compute for `n["p"]` will error because it's an output plug + # Compute for `script["n"]["p"]` will error because it's an output plug # that ComputeNode knows nothing about. - n = Gaffer.ComputeNode() - n["p"] = Gaffer.StringPlug( direction = Gaffer.Plug.Direction.Out ) + script = Gaffer.ScriptNode() + + script["n"] = Gaffer.ComputeNode() + script["n"]["p"] = Gaffer.StringPlug( direction = Gaffer.Plug.Direction.Out ) # We want that to be reflected in the UI. - w = GafferUI.StringPlugValueWidget( n["p"] ) + w = GafferUI.StringPlugValueWidget( script["n"]["p"] ) GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertTrue( w.textWidget().getErrored() ) diff --git a/python/GafferVDBTest/MeshToLevelSetTest.py b/python/GafferVDBTest/MeshToLevelSetTest.py index e566d6fb93f..509b53c081f 100644 --- a/python/GafferVDBTest/MeshToLevelSetTest.py +++ b/python/GafferVDBTest/MeshToLevelSetTest.py @@ -135,19 +135,21 @@ def testCancellation( self ) : # Start a computation in the background, and # then cancel it. - sphere = GafferScene.Sphere() + script = Gaffer.ScriptNode() - meshToLevelSet = GafferVDB.MeshToLevelSet() - meshToLevelSet["in"].setInput( sphere["out"] ) - self.setFilter( meshToLevelSet, path = "/sphere" ) - meshToLevelSet["voxelSize"].setValue( 0.01 ) + script["sphere"] = GafferScene.Sphere() + + script["meshToLevelSet"] = GafferVDB.MeshToLevelSet() + script["meshToLevelSet"]["in"].setInput( script["sphere"]["out"] ) + self.setFilter( script["meshToLevelSet"], path = "/sphere" ) + script["meshToLevelSet"]["voxelSize"].setValue( 0.01 ) def computeObject() : - meshToLevelSet["out"].object( "/sphere" ) + script["meshToLevelSet"]["out"].object( "/sphere" ) backgroundTask = Gaffer.ParallelAlgo.callOnBackgroundThread( - meshToLevelSet["out"]["object"], computeObject + script["meshToLevelSet"]["out"]["object"], computeObject ) # Delay so that the computation actually starts, rather # than being avoided entirely. @@ -157,11 +159,11 @@ def computeObject() : # Get the value again. If cancellation has been managed properly, this # will do a fresh compute to get a full result, and not pull a half-finished # result out of the cache. - vdbAfterCancellation = meshToLevelSet["out"].object( "/sphere" ) + vdbAfterCancellation = script["meshToLevelSet"]["out"].object( "/sphere" ) # Compare against a result computed from scratch. Gaffer.ValuePlug.clearCache() - vdb = meshToLevelSet["out"].object( "/sphere" ) + vdb = script["meshToLevelSet"]["out"].object( "/sphere" ) self.assertEqual( vdbAfterCancellation.findGrid( "surface" ).activeVoxelCount(), diff --git a/src/Gaffer/BackgroundTask.cpp b/src/Gaffer/BackgroundTask.cpp index 8721e8f1015..3ae2c87ca22 100644 --- a/src/Gaffer/BackgroundTask.cpp +++ b/src/Gaffer/BackgroundTask.cpp @@ -48,6 +48,8 @@ #include "tbb/task_arena.h" +#include "fmt/format.h" + using namespace IECore; using namespace Gaffer; @@ -151,7 +153,13 @@ struct BackgroundTask::TaskData : public boost::noncopyable BackgroundTask::BackgroundTask( const Plug *subject, const Function &function ) : m_function( function ), m_taskData( std::make_shared( &m_function ) ) { - activeTasks().insert( ActiveTask{ this, scriptNode( subject ) } ); + const ScriptNode *s = scriptNode( subject ); + if( subject && !s ) + { + IECore::msg( IECore::Msg::Level::Warning, "BackgroundTask", fmt::format( "Unable to find ScriptNode for {}", subject->fullName() ) ); + } + + activeTasks().insert( ActiveTask{ this, s } ); // Enqueue task into current arena. tbb::task_arena( tbb::task_arena::attach() ).enqueue(