Skip to content

Commit b10cb1a

Browse files
murraystevensonmurraystevenson
authored
Merge pull request #6770 from GafferHQ/globalsSanitiser
GlobalsSanitiser
2 parents 827ea10 + 77ed2df commit b10cb1a

File tree

9 files changed

+321
-6
lines changed

9 files changed

+321
-6
lines changed

‎Changes.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,22 @@ Improvements
77
- RenderPassEditor :
88
- Added RenderMan XPU support for Gaffer's inbuilt render pass types.
99
- Improved performance of render adaptors when computing the scene globals.
10+
- SceneTestCase : Added GlobalsSanitiser.
1011

1112
Fixes
1213
-----
1314

1415
- NodeEditor : Fixed "Revert to Defaults" to handle ganged plugs, and other plugs where a subset of children have input connections. In this case, the subset without inputs now revert correctly to their default values.
1516
- ShaderTweaks : Fixed context handling in "From Affected" and "From Selected" menu items.
1617
- RenderMan : Fixed `R10043 {WARNING} inputMaterial, unknown or mismatched input parameter of PxrSurface`.
18+
- SceneTestCase, ImageTestCase : Sanitisers are no longer installed when testing performance, since they add additional overhead.
1719

1820
API
1921
---
2022

2123
- PlugCreationWidget : Added `context()` method.
24+
- TestRunner : Added `PerformanceTestMethod.isDecorated()` for detection of performance test methods.
25+
- GlobalsSanitiser : Added monitor for detecting `ScenePlug.globals` computations depending on other aspects of the scene.
2226

2327
1.6.9.1 (relative to 1.6.9.0)
2428
=======

‎include/GafferSceneTest/GlobalsSanitiser.h‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
//////////////////////////////////////////////////////////////////////////
2+
//
3+
// Copyright (c) 2026, Cinesite VFX Ltd. All rights reserved.
4+
//
5+
// Redistribution and use in source and binary forms, with or without
6+
// modification, are permitted provided that the following conditions are
7+
// met:
8+
//
9+
// * Redistributions of source code must retain the above
10+
// copyright notice, this list of conditions and the following
11+
// disclaimer.
12+
//
13+
// * Redistributions in binary form must reproduce the above
14+
// copyright notice, this list of conditions and the following
15+
// disclaimer in the documentation and/or other materials provided with
16+
// the distribution.
17+
//
18+
// * Neither the name of John Haddon nor the names of
19+
// any other contributors to this software may be used to endorse or
20+
// promote products derived from this software without specific prior
21+
// written permission.
22+
//
23+
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
24+
// IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
25+
// THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
26+
// PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
27+
// CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
28+
// EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
29+
// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
30+
// PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
31+
// LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
32+
// NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
33+
// SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
34+
//
35+
//////////////////////////////////////////////////////////////////////////
36+
37+
#pragma once
38+
39+
#include "GafferSceneTest/Export.h"
40+
41+
#include "Gaffer/Monitor.h"
42+
#include "Gaffer/TypedObjectPlug.h"
43+
44+
#include "tbb/concurrent_unordered_set.h"
45+
#include "tbb/concurrent_unordered_map.h"
46+
47+
namespace GafferSceneTest
48+
{
49+
50+
/// A monitor which warns if the scene globals depend on some other
51+
/// aspect of the scene. Our rule is that the globals must be fast to
52+
/// compute, so should not depend on the rest of the scene, because that
53+
/// could be arbitrarily complex.
54+
class GAFFERSCENETEST_API GlobalsSanitiser : public Gaffer::Monitor
55+
{
56+
57+
public :
58+
59+
GlobalsSanitiser();
60+
61+
IE_CORE_DECLAREMEMBERPTR( GlobalsSanitiser )
62+
63+
protected :
64+
65+
void processStarted( const Gaffer::Process *process ) override;
66+
void processFinished( const Gaffer::Process *process ) override;
67+
68+
private :
69+
70+
// Maps from a process to the closest `ScenePlug.globals` that depends on it.
71+
using DependentGlobalsMap = tbb::concurrent_unordered_map<const Gaffer::Process *, const Gaffer::CompoundObjectPlug *>;
72+
DependentGlobalsMap m_dependentGlobalsMap;
73+
74+
// First is the upstream plug where the problem was detected. Second
75+
// is the downstream globals plug which depended on it.
76+
using Warning = std::pair<Gaffer::ConstPlugPtr, Gaffer::ConstCompoundObjectPlugPtr>;
77+
using WarningSet = tbb::concurrent_unordered_set<Warning>;
78+
// Used to avoid outputting duplicate warnings.
79+
WarningSet m_warningsEmitted;
80+
81+
void warn( const Gaffer::Process &process, const Gaffer::CompoundObjectPlug *dependentGlobals );
82+
83+
};
84+
85+
IE_CORE_DECLAREPTR( GlobalsSanitiser )
86+
87+
} // namespace GafferSceneTest

‎python/GafferImageTest/ImageTestCase.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,13 @@ def setUp( self ) :
5151

5252
GafferTest.TestCase.setUp( self )
5353

54-
sanitiser = GafferImageTest.ContextSanitiser()
55-
sanitiser.__enter__()
56-
self.addCleanup( sanitiser.__exit__, None, None, None )
54+
# Only install sanitisers when we're not testing performance, as they do
55+
# have some overhead.
56+
if not GafferTest.TestRunner.PerformanceTestMethod.isDecorated( getattr( self, self._testMethodName ) ) :
57+
58+
sanitiser = GafferImageTest.ContextSanitiser()
59+
sanitiser.__enter__()
60+
self.addCleanup( sanitiser.__exit__, None, None, None )
5761

5862
def tearDown( self ) :
5963

‎python/GafferSceneTest/GlobalsSanitiserTest.py‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
##########################################################################
2+
#
3+
# Copyright (c) 2026, Cinesite VFX Ltd. All rights reserved.
4+
#
5+
# Redistribution and use in source and binary forms, with or without
6+
# modification, are permitted provided that the following conditions are
7+
# met:
8+
#
9+
# * Redistributions of source code must retain the above
10+
# copyright notice, this list of conditions and the following
11+
# disclaimer.
12+
#
13+
# * Redistributions in binary form must reproduce the above
14+
# copyright notice, this list of conditions and the following
15+
# disclaimer in the documentation and/or other materials provided with
16+
# the distribution.
17+
#
18+
# * Neither the name of John Haddon nor the names of
19+
# any other contributors to this software may be used to endorse or
20+
# promote products derived from this software without specific prior
21+
# written permission.
22+
#
23+
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
24+
# IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
25+
# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
26+
# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
27+
# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
28+
# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
29+
# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
30+
# PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
31+
# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
32+
# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
33+
# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
34+
#
35+
##########################################################################
36+
37+
import IECore
38+
39+
import Gaffer
40+
import GafferScene
41+
import GafferSceneTest
42+
43+
class GlobalsSanitiserTest( GafferSceneTest.SceneTestCase ) :
44+
45+
def test( self ) :
46+
47+
plane = GafferScene.Plane()
48+
49+
attributeQuery = GafferScene.AttributeQuery()
50+
attributeQuery.setup( Gaffer.BoolPlug() )
51+
attributeQuery["scene"].setInput( plane["out"] )
52+
attributeQuery["location"].setValue( "/plane" )
53+
attributeQuery["attribute"].setValue( "test" )
54+
55+
options1 = GafferScene.StandardOptions( "options1" )
56+
options1["options"]["render:camera"]["enabled"].setInput( attributeQuery["value"] )
57+
58+
options2 = GafferScene.StandardOptions( "options2" )
59+
options2["in"].setInput( options1["out"] )
60+
61+
# A GlobalsSanitiser is automatically hooked up by SceneTestCase.setUp, so
62+
# we don't need to explicitly set one up
63+
with IECore.CapturingMessageHandler() as mh :
64+
options2["out"].globals()
65+
66+
for message in mh.messages :
67+
self.assertEqual( message.level, mh.Level.Warning )
68+
self.assertEqual( message.context, "GlobalsSanitiser" )
69+
70+
self.assertEqual(
71+
[ m.message for m in mh.messages ],
72+
[
73+
"Globals options1.out.globals depends on Plane.out.exists",
74+
"Globals options1.out.globals depends on Plane.out.attributes",
75+
]
76+
)
77+
78+
if __name__ == "__main__":
79+
unittest.main()

‎python/GafferSceneTest/SceneTestCase.py‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,17 @@ def setUp( self ) :
5454

5555
GafferImageTest.ImageTestCase.setUp( self )
5656

57-
sanitiser = GafferSceneTest.ContextSanitiser()
58-
sanitiser.__enter__()
59-
self.addCleanup( sanitiser.__exit__, None, None, None )
57+
# Only install sanitisers when we're not testing performance, as they do
58+
# have some overhead.
59+
if not GafferTest.TestRunner.PerformanceTestMethod.isDecorated( getattr( self, self._testMethodName ) ) :
60+
61+
sanitiser = GafferSceneTest.ContextSanitiser()
62+
sanitiser.__enter__()
63+
self.addCleanup( sanitiser.__exit__, None, None, None )
64+
65+
globalsSanitiser = GafferSceneTest.GlobalsSanitiser()
66+
globalsSanitiser.__enter__()
67+
self.addCleanup( globalsSanitiser.__exit__, None, None, None )
6068

6169
def tearDown( self ) :
6270

‎python/GafferSceneTest/__init__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@
194194
from .CatalogueTest import CatalogueTest
195195
from .CatalogueSelectTest import CatalogueSelectTest
196196
from .CameraQueryTest import CameraQueryTest
197+
from .GlobalsSanitiserTest import GlobalsSanitiserTest
197198

198199
from .IECoreScenePreviewTest import *
199200
from .IECoreGLPreviewTest import *

‎python/GafferTest/TestRunner.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ def __init__( self, repeat = 3, acceptableDifference = 0.01 ) :
101101
def __call__( self, method ) :
102102

103103
method = TestRunner.CategorisedTestMethod.__call__( self, method )
104+
method._isPerformanceTestMethod = True
104105

105106
@functools.wraps( method )
106107
def wrapper( *args, **kw ) :
@@ -131,6 +132,16 @@ def wrapper( *args, **kw ) :
131132

132133
return wrapper
133134

135+
# Returns True if `method` has been decorated with
136+
# PerformanceTestMethod.
137+
@staticmethod
138+
def isDecorated( method ) :
139+
140+
while method is not None :
141+
if getattr( method, "_isPerformanceTestMethod", False ) :
142+
return True
143+
method = getattr( method, "__wrapped__", None )
144+
134145
# Context manager used to time only specific blocks
135146
# within a PerformanceTestMethod.
136147
class PerformanceScope( object ) :

‎src/GafferSceneTest/GlobalsSanitiser.cpp‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
//////////////////////////////////////////////////////////////////////////
2+
//
3+
// Copyright (c) 2026, Cinesite VFX Ltd. All rights reserved.
4+
//
5+
// Redistribution and use in source and binary forms, with or without
6+
// modification, are permitted provided that the following conditions are
7+
// met:
8+
//
9+
// * Redistributions of source code must retain the above
10+
// copyright notice, this list of conditions and the following
11+
// disclaimer.
12+
//
13+
// * Redistributions in binary form must reproduce the above
14+
// copyright notice, this list of conditions and the following
15+
// disclaimer in the documentation and/or other materials provided with
16+
// the distribution.
17+
//
18+
// * Neither the name of John Haddon nor the names of
19+
// any other contributors to this software may be used to endorse or
20+
// promote products derived from this software without specific prior
21+
// written permission.
22+
//
23+
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
24+
// IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
25+
// THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
26+
// PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
27+
// CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
28+
// EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
29+
// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
30+
// PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
31+
// LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
32+
// NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
33+
// SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
34+
//
35+
//////////////////////////////////////////////////////////////////////////
36+
37+
#include "GafferSceneTest/GlobalsSanitiser.h"
38+
39+
#include "GafferScene/ScenePlug.h"
40+
41+
#include "Gaffer/Process.h"
42+
#include "Gaffer/ScriptNode.h"
43+
44+
#include "IECore/MessageHandler.h"
45+
46+
#include "fmt/format.h"
47+
48+
using namespace IECore;
49+
using namespace Gaffer;
50+
using namespace GafferScene;
51+
using namespace GafferSceneTest;
52+
53+
GlobalsSanitiser::GlobalsSanitiser()
54+
{
55+
}
56+
57+
void GlobalsSanitiser::processStarted( const Gaffer::Process *process )
58+
{
59+
const CompoundObjectPlug *dependentGlobals = nullptr;
60+
auto it = m_dependentGlobalsMap.find( process->parent() );
61+
if( it != m_dependentGlobalsMap.end() )
62+
{
63+
// If some globals were dependent on our parent process, then
64+
// they are dependent on us too.
65+
dependentGlobals = it->second;
66+
}
67+
68+
if( const ScenePlug *scene = process->plug()->parent<ScenePlug>() )
69+
{
70+
if( process->plug() == scene->globalsPlug() )
71+
{
72+
dependentGlobals = scene->globalsPlug();
73+
}
74+
else if( dependentGlobals )
75+
{
76+
warn( *process, dependentGlobals );
77+
// No point issuing further warnings for upstream processes.
78+
dependentGlobals = nullptr;
79+
}
80+
}
81+
82+
if( dependentGlobals )
83+
{
84+
m_dependentGlobalsMap[process] = dependentGlobals;
85+
}
86+
}
87+
88+
void GlobalsSanitiser::processFinished( const Gaffer::Process *process )
89+
{
90+
auto it = m_dependentGlobalsMap.find( process );
91+
if( it != m_dependentGlobalsMap.end() )
92+
{
93+
// We can't erase in a thread-safe manner, so just store null.
94+
// This does mean we accumulate entries, but we only intend to
95+
// use the sanitiser for short bursts anyway.
96+
it->second = nullptr;
97+
}
98+
}
99+
100+
void GlobalsSanitiser::warn( const Gaffer::Process &process, const Gaffer::CompoundObjectPlug *dependentGlobals )
101+
{
102+
const Warning warning( process.plug(), dependentGlobals );
103+
if( !m_warningsEmitted.insert( warning ).second )
104+
{
105+
return;
106+
}
107+
108+
IECore::msg(
109+
IECore::Msg::Warning, "GlobalsSanitiser",
110+
fmt::format(
111+
"Globals {} depends on {}",
112+
dependentGlobals->relativeName( dependentGlobals->ancestor<ScriptNode>() ),
113+
process.plug()->relativeName( process.plug()->ancestor<ScriptNode>() )
114+
)
115+
);
116+
}

‎src/GafferSceneTestModule/GafferSceneTestModule.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939

4040
#include "GafferSceneTest/ContextSanitiser.h"
4141
#include "GafferSceneTest/CompoundObjectSource.h"
42+
#include "GafferSceneTest/GlobalsSanitiser.h"
4243
#include "GafferSceneTest/ScenePlugTest.h"
4344
#include "GafferSceneTest/TestLight.h"
4445
#include "GafferSceneTest/TestLightFilter.h"
@@ -65,6 +66,10 @@ BOOST_PYTHON_MODULE( _GafferSceneTest )
6566
.def( init<>() )
6667
;
6768

69+
IECorePython::RefCountedClass<GlobalsSanitiser, Gaffer::Monitor>( "GlobalsSanitiser" )
70+
.def( init<>() )
71+
;
72+
6873
GafferBindings::DependencyNodeClass<CompoundObjectSource>();
6974
GafferBindings::NodeClass<TestShader>();
7075
GafferBindings::NodeClass<TestLight>()

0 commit comments

Comments
 (0)