From a3d242427a22ce28e8d3c09056fac30d08d05f17 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 3 Nov 2023 10:50:00 +0000 Subject: [PATCH] OpenColorIOTransformBinding : Add missing ScopedGILRelease This was causing hangs when opening old Gaffer files in `1.3.x`, if the user's default layout was such that the Viewer would start a compute as soon as the file was opened. In this case, `acquireDefaultConfigPlug()` was still holding the GIL while creating the plug, and creating the plug would request cancellation of the compute, and the compute couldn't cancel if it was waiting for the GIL. --- Changes.md | 1 + include/GafferImage/OpenColorIOConfigPlug.h | 2 ++ src/GafferImageModule/OpenColorIOTransformBinding.cpp | 11 ++++++++--- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Changes.md b/Changes.md index 51d3eba94d0..010477b90a8 100644 --- a/Changes.md +++ b/Changes.md @@ -30,6 +30,7 @@ Fixes - Dispatch app : Removed unnecessary and misleading "Execute" button. - SceneAlgo : Fixed computation of `ScenePlug.object` in networks with nodes derived from `ObjectProcessor`. These include : `CameraTweaks`, `ClosestPointSampler`, `CollectPrimitiveVariables`, `CopyPrimitiveVariables`, `CurveSampler`, `DeleteCurves`, `DeleteFaces`, `DeletePoints`, `MapOffset`, `MapProjection`, `MeshDistortion`, `MeshNormals`, `MeshSegments`, `MeshTangents`, `MeshToPoints`, `MeshType`, `Orientation`, `PointsType`, `PrimitiveSampler`, `PrimitiveVariables`, `ReverseWinding`, `ShufflePrimitiveVariables` and `UVSampler` (#5406). - Metadata : Fixed redundant copying of metadata when promoting plugs. +- OpenColorIO : Fixed hang when opening a script which didn't yet have the `openColorIO.config` plug. API --- diff --git a/include/GafferImage/OpenColorIOConfigPlug.h b/include/GafferImage/OpenColorIOConfigPlug.h index 4255aa5ce2e..e258dc68f64 100644 --- a/include/GafferImage/OpenColorIOConfigPlug.h +++ b/include/GafferImage/OpenColorIOConfigPlug.h @@ -85,4 +85,6 @@ class GAFFERIMAGE_API OpenColorIOConfigPlug final : public Gaffer::ValuePlug Gaffer::Signals::ScopedConnection m_plugSetConnection; }; +IE_CORE_DECLAREPTR( OpenColorIOConfigPlug ); + } // namespace GafferImage diff --git a/src/GafferImageModule/OpenColorIOTransformBinding.cpp b/src/GafferImageModule/OpenColorIOTransformBinding.cpp index 63d7d020f81..9c32fc0eb6f 100644 --- a/src/GafferImageModule/OpenColorIOTransformBinding.cpp +++ b/src/GafferImageModule/OpenColorIOTransformBinding.cpp @@ -71,6 +71,12 @@ boost::python::list supportedExtensions() return result; } +OpenColorIOConfigPlugPtr acquireDefaultConfigPlugWrapper( Gaffer::ScriptNode &scriptNode, bool createIfNecessary ) +{ + IECorePython::ScopedGILRelease gilRelease; + return OpenColorIOConfigPlug::acquireDefaultConfigPlug( &scriptNode, createIfNecessary ); +} + } // namespace void GafferImageModule::bindOpenColorIOTransform() @@ -121,9 +127,8 @@ void GafferImageModule::bindOpenColorIOTransform() ) ) .def( - "acquireDefaultConfigPlug", &OpenColorIOConfigPlug::acquireDefaultConfigPlug, - ( arg( "scriptNode" ), arg( "createIfNecessary" ) = true ), - boost::python::return_value_policy() + "acquireDefaultConfigPlug", &acquireDefaultConfigPlugWrapper, + ( arg( "scriptNode" ), arg( "createIfNecessary" ) = true ) ) .staticmethod( "acquireDefaultConfigPlug" ) ;