Merge pull request #5354 from johnhaddon/ocioScriptVariables

Per-script OpenColorIO config, working space and display transform

Changes.md

@@ -17,6 +17,7 @@ Improvements
 - ImageReader, ImageWriter : The `colorSpace` menu is now filtered using the `file-io` category, if the current OpenColorIO config provides it.
 - OpenColorIO :
   - Updated default config to ACES Studio 1.3.
+  - Added `openColorIO` plug to ScriptNode, allowing the OpenColorIO config, working space, variables and display transform to be customised on a per-script basis.
   - Improved colorspace menus :
     - Organised colorspaces into submenus by family.
     - Removed unwanted title-casing, so that names are now displayed verbatim.
@@ -30,6 +31,8 @@ Improvements
   - OpenColorIOTransform :
     - Improved performance.
     - Improved detection of no-op transforms, such as when converting between colorspace aliases like `scene_linear` and `ACEScg`.
+  - ColorSpace : Defaulted the input and output space to the current working space.
+  - DisplayTransform : Defaulted the input space to the current working space, and the display and view to the defaults defined by the current OpenColorIO config.
 - Seeds :
   - Renamed to Scatter.
   - Added sampling of primitive variables from the source mesh onto the scattered points, controlled using the new `primitiveVariables` plug.
@@ -61,6 +64,9 @@ Fixes
 - ObjectSource, Group : Prevented the creation of locations with invalid names - `..`, `...` or anything containing `/` or a filter wildcard.
 - BranchCreator : Prevented the use of `...` and other filter wildcards in the `destination`.
 - TranslateTool : Fixed dragging in a plane parallel to an orthographic view. Translation in that case now behaves the same as dragging an axis.
+- Window : Fixed handling of `**kw` constructor arguments. These were being passed to the `QWidget` constructor where they caused errors, instead of being passed to the ContainerWidget base class.
+- PresetsPlugValueWidget : Fixed label update for context-sensitive presets.
+- PlugValueWidget : Fixed value update when auxiliary plugs depend on the context but the primary plugs do not.
 
 API
 ---
@@ -75,9 +81,10 @@ API
 - TestCase : Added `ignoreMessage()` method, to register messages that should not be treated as test failures.
 - OpenColorIOTransform : Automated image pass-throughs when the `transform()` method returns a no-op. Derived classes no longer need to implement their own pass-through.
 - OpenColorIOTransformUI :
-  - Added `noneLabel` argument to `colorSpacePresetNames()`.
   - Added support for `openColorIO:categories` and `openColorIO:includeRoles` metadata to `colorSpacePresetNames()`. These may be registered on a per-plug basis to control the colorspaces shown for that plug.
-- OpenColorIOAlgo : Added a new namespace that allows the OpenColorIO config to be defined via the Gaffer context.
+  - Added support for `openColorIO:extraPresetNames` and `openColorIO:extraPresetValues` metadata to add presets not defined by the OpenColorIO config.
+- OpenColorIOAlgo : Added a new namespace that allows the OpenColorIO config and working space to be defined via the Gaffer context.
+- OpenColorIOConfigPlug : Added a new plug type to aid in configuring the OpenColorIO context for a ScriptNode.
 - ImageReader/ImageWriter : Added a `config` argument to the `DefaultColorSpaceFunction` definition. This is passed the OpenColorIO config currently being used by the node.
 - ValuePlugs : Added Python bindings for `ValueType` type alias.
 - Color4fVectorDataPlug : Added a plug type for storing arrays of `Color4f`.
@@ -86,18 +93,22 @@ API
 - PlugLayout :
   - Added support for `layout:minimumWidth` metadata.
   - Tabs are now hidden if all their child plugs are hidden by `layout:visibilityActivator` metadata.
-- PlugValueWidget : Added `typeMetadata` argument to `create()` to use an alternative key to `plugValueWidget:type` when looking up widget type.
+- PlugValueWidget :
+  - Added `typeMetadata` argument to `create()` to use an alternative key to `plugValueWidget:type` when looking up widget type.
+  - Added `_valuesDependOnContext()` method which may be overridden by derived classes.
 - PresetsPlugValueWidget : Added `presetsPlugValueWidget:customWidgetType` metadata, to allow the type for the custom widget to be specified.
 - TabbedContainer : Added `setTabVisible()` and `getTabVisible()` methods.
 - Removed use of `RTLD_GLOBAL` for loading Python modules.
 - SceneAlgo : Added `validateName()` function.
 - Sampler : Added `visitPixels()` method, which provides an optimised method for accessing all pixels in a region.
 - Handle::AngularDrag : Added `isLinearDrag()` method.
+- Widget : Added per-widget control over colour display transforms via new `setDisplayTransform()`, `getDisplayTransform()` and `displayTransform()` methods.
 
 Breaking Changes
 ----------------
 
 - Appleseed : Removed Appleseed support. We suggest Cycles as an actively maintained open-source alternative.
+- ColorSwatch, ColorChooser, ColorChooserDialogue : Removed `useDisplayTransform` constructor argument. Use `Widget.setDisplayTransform()` instead.
 - GraphComponent : Changed slot signature for `nameChangedSignal()`.
 - GLWidget :
   - A GL context is no longer available in `_resize()`.
@@ -132,7 +143,13 @@ Breaking Changes
   - Removed support for deprecated `layout:widgetType` metadata. Use `plugValueWidget:type` metadata instead.
   - Removed `useTypeOnly` argument from `create()` function. Pass `typeMetadata = None` instead.
 - MeshTangents : Changed the edge used by `Mode::FirstEdge`.
-- Handle::AngularDrag : Fix mismatch between comment and implementation regarding the axis for zero rotation. The constructor arguments `axis0` and `axis1` were changed to `normal` and `axis0` respectively.
+- Handle::AngularDrag : Fixed mismatch between comment and implementation regarding the axis for zero rotation. The constructor arguments `axis0` and `axis1` were changed to `normal` and `axis0` respectively.
+- Preferences : Removed `displayColorSpace` plug. Use the ScriptNode's `openColorIO` plug instead.
+- GafferUI.DisplayTransform : Removed. Use `Widget.setDisplayTransform()` instead.
+- ColorSpace : The `inputSpace` and `outputSpace` default values are now interpreted as the working space rather than as invalid spaces. This means that a node where only one space has been specified is no longer a pass-through as it was before.
+- DisplayTransform :
+  - The `inputSpace` default value is now interpreted as the working space rather than as an invalid space. This means that a node without `inputSpace` specified is no longer a pass-through as it was before.
+  - The `display` and `view` default values are now interpreted as the default defined by the current OpenColorIO config, rather than as invalid values. This means that a node without `display` or `view` specified is no longer a pass-through as it was before.
 
 Build
 -----

include/GafferImage/OpenColorIOAlgo.h

@@ -60,6 +60,11 @@ GAFFERIMAGE_API void setConfig( Gaffer::Context *context, const std::string &con
 GAFFERIMAGE_API void setConfig( Gaffer::Context::EditableScope &context, const std::string *configFileName );
 GAFFERIMAGE_API const std::string &getConfig( const Gaffer::Context *context );
 
+/// Sets the colour space in which GafferImage stores colours for processing. Defaults to the `scene_linear` role.
+GAFFERIMAGE_API void setWorkingSpace( Gaffer::Context *context, const std::string &colorSpace );
+GAFFERIMAGE_API void setWorkingSpace( Gaffer::Context::EditableScope &context, const std::string *colorSpace );
+GAFFERIMAGE_API const std::string &getWorkingSpace( const Gaffer::Context *context );
+
 /// Adds an OCIO "string var" to be used in `context`. Note that OCIO also calls these
 /// "environment vars" and "context vars" but they're all the same thing.
 GAFFERIMAGE_API void addVariable( Gaffer::Context *context, const std::string &name, const std::string &value );

include/GafferImage/OpenColorIOConfigPlug.h

@@ -0,0 +1,88 @@
+//////////////////////////////////////////////////////////////////////////
+//
+//  Copyright (c) 2023, 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.
+//
+//////////////////////////////////////////////////////////////////////////
+
+#pragma once
+
+#include "GafferImage/Export.h"
+#include "GafferImage/TypeIds.h"
+
+#include "Gaffer/Context.h"
+#include "Gaffer/ScriptNode.h"
+#include "Gaffer/StringPlug.h"
+
+#include "OpenColorIO/OpenColorTypes.h"
+
+namespace GafferImage
+{
+/// Plug to provide user-level control over the default OCIO config for a ScriptNode.
+/// Would typically be used by calling `acquireDefaultConfigPlug()` from a startup file.
+class GAFFERIMAGE_API OpenColorIOConfigPlug final : public Gaffer::ValuePlug
+{
+
+	public :
+
+		GAFFER_PLUG_DECLARE_TYPE( GafferImage::OpenColorIOConfigPlug, OpenColorIOConfigPlugTypeId, Gaffer::ValuePlug );
+
+		explicit OpenColorIOConfigPlug( const std::string &name = defaultName<OpenColorIOConfigPlug>(), Direction direction = In, unsigned flags = Default );
+
+		Gaffer::StringPlug *configPlug();
+		const Gaffer::StringPlug *configPlug() const;
+
+		Gaffer::StringPlug *workingSpacePlug();
+		const Gaffer::StringPlug *workingSpacePlug() const;
+
+		Gaffer::ValuePlug *variablesPlug();
+		const Gaffer::ValuePlug *variablesPlug() const;
+
+		Gaffer::StringPlug *displayTransformPlug();
+		const Gaffer::ValuePlug *displayTransformPlug() const;
+
+		bool acceptsChild( const GraphComponent *potentialChild ) const override;
+		Gaffer::PlugPtr createCounterpart( const std::string &name, Direction direction ) const override;
+
+		static OpenColorIOConfigPlug *acquireDefaultConfigPlug( Gaffer::ScriptNode *scriptNode, bool createIfNecessary = true );
+
+	protected :
+
+		void parentChanged( Gaffer::GraphComponent *oldParent ) override;
+
+	private :
+
+		void plugSet( Gaffer::Plug *plug );
+		Gaffer::Signals::ScopedConnection m_plugSetConnection;
+};
+
+} // namespace GafferImage

include/GafferImage/OpenColorIOContext.h

@@ -65,6 +65,15 @@ class GAFFERIMAGE_API OpenColorIOContext : public Gaffer::ContextProcessor
 		Gaffer::StringPlug *configValuePlug();
 		const Gaffer::StringPlug *configValuePlug() const;
 
+		Gaffer::ValuePlug *workingSpacePlug();
+		const Gaffer::ValuePlug *workingSpacePlug() const;
+
+		Gaffer::BoolPlug *workingSpaceEnabledPlug();
+		const Gaffer::BoolPlug *workingSpaceEnabledPlug() const;
+
+		Gaffer::StringPlug *workingSpaceValuePlug();
+		const Gaffer::StringPlug *workingSpaceValuePlug() const;
+
 		Gaffer::ValuePlug *variablesPlug();
 		const Gaffer::ValuePlug *variablesPlug() const;
 

include/GafferImage/TypeIds.h

@@ -72,7 +72,7 @@ enum TypeId
 	SelectViewTypeId = 110777,
 	DeleteViewsTypeId = 110778,
 	CopyViewsTypeId = 110779,
-	CatmullRomFilterTypeId = 110780, // Obsolete - available for reuse
+	OpenColorIOConfigPlugTypeId = 110780,
 	SincFilterTypeId = 110781, // Obsolete - available for reuse
 	LanczosFilterTypeId = 110782, // Obsolete - available for reuse
 	ImageStatsTypeId = 110783,

python/GafferImageTest/ColorSpaceTest.py

@@ -374,5 +374,42 @@ def testRolePassThrough( self ) :
 		self.assertImagesEqual( colorSpace["out"], reader["out"] )
 		self.assertImageHashesEqual( colorSpace["out"], reader["out"] )
 
+	def testEmptyColorSpaceIsSameAsWorkingSpace( self ) :
+
+		checker = GafferImage.Checkerboard()
+
+		colorSpace1 = GafferImage.ColorSpace()
+		colorSpace1["in"].setInput( checker["out"] )
+		colorSpace1["inputSpace"].setValue( "scene_linear" )
+		colorSpace1["outputSpace"].setValue( "color_picking" )
+
+		self.assertNotEqual(
+			colorSpace1["out"].channelData( "R", imath.V2i( 0 ) ),
+			colorSpace1["in"].channelData( "R", imath.V2i( 0 ) )
+		)
+
+		colorSpace2 = GafferImage.ColorSpace()
+		colorSpace2["in"].setInput( checker["out"] )
+		self.assertEqual( colorSpace2["inputSpace"].getValue(), "" )
+		colorSpace2["outputSpace"].setValue( "color_picking" )
+
+		self.assertImagesEqual( colorSpace2["out"], colorSpace1["out"] )
+
+	def testChangingWorkingSpace( self ) :
+
+		checker = GafferImage.Checkerboard()
+
+		colorSpace = GafferImage.ColorSpace()
+		colorSpace["in"].setInput( checker["out"] )
+		colorSpace["outputSpace"].setValue( "color_picking" )
+
+		with Gaffer.Context() as context :
+
+			GafferImage.OpenColorIOAlgo.setWorkingSpace( context, "scene_linear" )
+			tile = colorSpace["out"].channelData( "R", imath.V2i( 0 ) )
+
+			GafferImage.OpenColorIOAlgo.setWorkingSpace( context, "color_picking" )
+			self.assertNotEqual( colorSpace["out"].channelData( "R", imath.V2i( 0 ) ), tile )
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferImageTest/DisplayTransformTest.py

@@ -40,6 +40,8 @@
 import subprocess
 import imath
 
+import PyOpenColorIO
+
 import IECore
 
 import Gaffer
@@ -91,7 +93,6 @@ def testHashPassThrough( self ) :
 		self.assertImageHashesEqual( n["out"], o["out"] )
 		self.assertImagesEqual( n["out"], o["out"] )
 
-		o["inputColorSpace"].setValue( "scene_linear" )
 		o["display"].setValue( "sRGB - Display" )
 		o["view"].setValue( "ACES 1.0 - SDR Video" )
 
@@ -106,16 +107,6 @@ def testHashPassThrough( self ) :
 		self.assertEqual( n["out"]["metadata"].getValue(), o["out"]["metadata"].getValue() )
 		self.assertEqual( n["out"]['channelNames'].hash(), o["out"]['channelNames'].hash() )
 
-		o["enabled"].setValue( True )
-
-		o["inputColorSpace"].setValue( "" )
-		self.assertImageHashesEqual( n["out"], o["out"] )
-		self.assertImagesEqual( n["out"], o["out"] )
-		self.assertEqual( n["out"]['format'].hash(), o["out"]['format'].hash() )
-		self.assertEqual( n["out"]['dataWindow'].hash(), o["out"]['dataWindow'].hash() )
-		self.assertEqual( n["out"]["metadata"].getValue(), o["out"]["metadata"].getValue() )
-		self.assertEqual( n["out"]['channelNames'].hash(), o["out"]['channelNames'].hash() )
-
 	def testImageHashPassThrough( self ) :
 
 		i = GafferImage.ImageReader()
@@ -237,5 +228,51 @@ def testContext( self ) :
 		# check override produce expected output
 		self.assertImagesEqual( actual["out"], expected["out"], ignoreMetadata = True )
 
+	def testChangingWorkingSpace( self ) :
+
+		checker = GafferImage.Checkerboard()
+
+		displayTransform = GafferImage.DisplayTransform()
+		displayTransform["in"].setInput( checker["out"] )
+		displayTransform["display"].setValue( "sRGB - Display" )
+		displayTransform["view"].setValue( "ACES 1.0 - SDR Video" )
+
+		with Gaffer.Context() as context :
+
+			GafferImage.OpenColorIOAlgo.setWorkingSpace( context, "scene_linear" )
+			tile = displayTransform["out"].channelData( "R", imath.V2i( 0 ) )
+
+			GafferImage.OpenColorIOAlgo.setWorkingSpace( context, "color_picking" )
+			self.assertNotEqual( displayTransform["out"].channelData( "R", imath.V2i( 0 ) ), tile )
+
+	def testDisplayAndViewDefaultToConfig( self ) :
+
+		checker = GafferImage.Checkerboard()
+
+		# Test default config
+
+		defaultDisplayTransform = GafferImage.DisplayTransform()
+		defaultDisplayTransform["in"].setInput( checker["out"] )
+
+		config = GafferImage.OpenColorIOAlgo.currentConfig()
+		explicitDisplayTransform = GafferImage.DisplayTransform()
+		explicitDisplayTransform["in"].setInput( checker["out"] )
+		explicitDisplayTransform["display"].setValue( config.getDefaultDisplay() )
+		explicitDisplayTransform["view"].setValue( config.getDefaultView( config.getDefaultDisplay() ) )
+
+		self.assertImagesEqual( defaultDisplayTransform["out"], explicitDisplayTransform["out"] )
+
+		# Test alternative config
+
+		configPath = Gaffer.rootPath() / "openColorIO" / "config.ocio"
+		config = PyOpenColorIO.Config.CreateFromFile( str( configPath ) )
+
+		explicitDisplayTransform["display"].setValue( config.getDefaultDisplay() )
+		explicitDisplayTransform["view"].setValue( config.getDefaultView( config.getDefaultDisplay() ) )
+
+		with Gaffer.Context() as context :
+			GafferImage.OpenColorIOAlgo.setConfig( context, str( configPath ) )
+			self.assertImagesEqual( defaultDisplayTransform["out"], explicitDisplayTransform["out"] )
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferImageTest/OpenColorIOAlgoTest.py

@@ -114,6 +114,14 @@ def testCurrentConfigAndContextHash( self ) :
 
 		self.assertEqual( len( { h1, h2, h3 } ), 3 )
 
+	def testWorkingSpace( self ) :
+
+		c = Gaffer.Context()
+		self.assertEqual( GafferImage.OpenColorIOAlgo.getWorkingSpace( c ), PyOpenColorIO.ROLE_SCENE_LINEAR )
+
+		GafferImage.OpenColorIOAlgo.setWorkingSpace( c, "test" )
+		self.assertEqual( GafferImage.OpenColorIOAlgo.getWorkingSpace( c ), "test" )
+
 	def __assertContextsEqual( self, a, b ) :
 
 		self.assertEqual( a.getSearchPath(), b.getSearchPath() )