Merge pull request #5721 from johnhaddon/srgbFiasco

Image/ImageGadget : Avoid `IECoreImage.ImageReader`

Author: johnhaddon

Changes.md

@@ -9,11 +9,23 @@ Fixes
   - Fixed hangs and crashes when using non-default session modes such as SVM shading.
   - Fixed failure to render background light in batch renders (#5234).
   - Fixed failure to update when reverting a background shader to previous values.
+- GafferUI :
+  - Fixed `Color space 'sRGB' could not be found` errors when running with certain custom OCIO configs (#5695).
+  - Fixed icon colours when running with an ACES OCIO config.
 
 Breaking Changes
 ----------------
 
 - CyclesOptions : Changed `hairShape` default value to "ribbon", to match Cycles' and Blender's own defaults.
+- Pointer :
+  - Removed `Pointer( const ImagePrimitive * )` constructor.
+  - Removed `image()` method.
+
+API
+---
+
+- ImageGadget : Removed `textureLoader()` method.
+- Pointer : Added `fileName()` method.
 
 [^1]: To be omitted from final release notes for 1.4.0.0.
 

SConstruct

@@ -1058,7 +1058,7 @@ libraries = {
 	"GafferUI" : {
 		"envAppends" : {
 			## \todo Stop linking against `Iex`. It is only necessary on Windows Imath 2 builds.
-			"LIBS" : [ "Gaffer", "Iex$IMATH_LIB_SUFFIX", "IECoreGL$CORTEX_LIB_SUFFIX", "IECoreImage$CORTEX_LIB_SUFFIX", "IECoreScene$CORTEX_LIB_SUFFIX" ],
+			"LIBS" : [ "Gaffer", "Iex$IMATH_LIB_SUFFIX", "IECoreGL$CORTEX_LIB_SUFFIX", "IECoreImage$CORTEX_LIB_SUFFIX", "IECoreScene$CORTEX_LIB_SUFFIX", "OpenImageIO$OIIO_LIB_SUFFIX", "OpenImageIO_Util$OIIO_LIB_SUFFIX" ],
 		},
 		"pythonEnvAppends" : {
 			"LIBS" : [ "IECoreImage$CORTEX_LIB_SUFFIX", "IECoreScene$CORTEX_LIB_SUFFIX", "IECoreGL$CORTEX_LIB_SUFFIX", "GafferUI", "GafferBindings" ],

include/GafferUI/ImageGadget.h

@@ -53,6 +53,9 @@ IE_CORE_FORWARDDECLARE( Texture )
 namespace GafferUI
 {
 
+/// Gadget suitable for displaying icons and the like. For
+/// high-performance display of images generated by ImageNode,
+/// see `GafferImageUI::ImageGadget`.
 class GAFFERUI_API ImageGadget : public Gadget
 {
 
@@ -62,22 +65,29 @@ class GAFFERUI_API ImageGadget : public Gadget
 		/// the GAFFERUI_IMAGE_PATHS environment variable.
 		/// Throws if the file cannot be loaded.
 		explicit ImageGadget( const std::string &fileName );
-		/// A copy of the image is taken.
+		/// \deprecated
 		ImageGadget( const IECoreImage::ConstImagePrimitivePtr image );
 		~ImageGadget() override;
 
 		GAFFER_GRAPHCOMPONENT_DECLARE_TYPE( GafferUI::ImageGadget, ImageGadgetTypeId, Gadget );
 
 		Imath::Box3f bound() const override;
 
-		/// Returns the texture loader used for converting images
-		/// on disk into textures for rendering. This is exposed
-		/// publicly so that other code can share the same texture
-		/// cache.
-		static IECoreGL::TextureLoader *textureLoader();
-		/// Loads a texture using the `textureLoader()` and applies
-		/// the default ImageGadget texture parameters.
-		static IECoreGL::ConstTexturePtr loadTexture( const std::string &fileName );
+		struct GAFFERUI_API TextureParameters
+		{
+			GLint minFilter = GL_LINEAR_MIPMAP_LINEAR;
+			GLint magFilter = GL_LINEAR;
+			float lodBias = -1.0;
+			GLint wrapS = GL_CLAMP_TO_BORDER;
+			GLint wrapT = GL_CLAMP_TO_BORDER;
+			static TextureParameters defaultParameters() { return {}; }
+		};
+
+		/// Loads a texture suitable for rendering with `StandardStyle::renderImage()`,
+		/// searching for it on the paths defined by the `GAFFERUI_IMAGE_PATHS` environment
+		/// variable. Throws if the file cannot be loaded. Uses an internal cache, so the
+		/// returned texture may be shared with other code, and is therefore const.
+		static IECoreGL::ConstTexturePtr loadTexture( const std::string &fileName, const TextureParameters &parameters = TextureParameters::defaultParameters() );
 
 	protected :
 

include/GafferUI/Pointer.h

@@ -56,13 +56,11 @@ class GAFFERUI_API Pointer : public IECore::RefCounted
 
 		IE_CORE_DECLAREMEMBERPTR( Pointer )
 
-		/// A copy of the image is taken.
-		explicit Pointer( const IECoreImage::ImagePrimitive *image, const Imath::V2i &hotspot = Imath::V2i( -1 ) );
 		/// Images are loaded from the paths specified by the
 		/// GAFFERUI_IMAGE_PATHS environment variable.
 		Pointer( const std::string &fileName, const Imath::V2i &hotspot = Imath::V2i( -1 ) );
 
-		const IECoreImage::ImagePrimitive *image() const;
+		const std::string &fileName() const;
 		const Imath::V2i &hotspot() const;
 
 		/// Sets the current pointer. Passing null resets the
@@ -83,7 +81,7 @@ class GAFFERUI_API Pointer : public IECore::RefCounted
 
 	private :
 
-		IECoreImage::ConstImagePrimitivePtr m_image;
+		std::string m_fileName;
 		Imath::V2i m_hotspot;
 
 };

python/GafferUI/Image.py

@@ -171,6 +171,10 @@ def _qtIcon( self, highlighted = False ) :
 		icon.addPixmap( self._qtPixmapDisabled(), QtGui.QIcon.Disabled )
 		return icon
 
+	## \todo Deprecate and remove - we want to phase out ImagePrimitive.
+	# Although we don't use this function or the `Image( ImagePrimitive )`
+	# constructor in Gaffer itself, we can't do this immediately because some
+	# external code currently depends on it.
 	@staticmethod
 	def _qtPixmapFromImagePrimitive( image ) :
 
@@ -238,14 +242,7 @@ def __cacheGetter( cls, fileName ) :
 		if not resolvedFileName :
 			raise Exception( "Unable to find file \"%s\"" % fileName )
 
-		reader = IECore.Reader.create( resolvedFileName )
-
-		image = reader.read()
-		if not isinstance( image, IECoreImage.ImagePrimitive ) :
-			raise Exception( "File \"%s\" is not an image file" % resolvedFileName )
-
-		result = cls._qtPixmapFromImagePrimitive( image )
-
+		result = QtGui.QPixmap( resolvedFileName )
 		cost = result.width() * result.height() * ( 4 if result.hasAlpha() else 3 )
 
 		return ( result, cost )

python/GafferUI/_Pointer.py

@@ -55,7 +55,7 @@ def __pointerChanged() :
 			application.restoreOverrideCursor()
 			__cursorOverridden = False
 	else :
-		pixmap = GafferUI.Image._qtPixmapFromImagePrimitive( pointer.image() )
+		pixmap = GafferUI.Image._qtPixmapFromFile( pointer.fileName() )
 		cursor = QtGui.QCursor( pixmap, pointer.hotspot().x, pointer.hotspot().y )
 		if __cursorOverridden :
 			application.changeOverrideCursor( cursor )

python/GafferUITest/ImageGadgetTest.py

@@ -35,6 +35,7 @@
 ##########################################################################
 
 import os
+import pathlib
 import unittest
 import imath
 
@@ -64,16 +65,19 @@ def testMissingFiles( self ) :
 
 		self.assertRaises( Exception, GafferUI.ImageGadget, "iDonNotExist" )
 
-	def testTextureLoader( self ) :
+	def testAllImages( self ) :
 
-		# must access an attribute from IECoreGL to force import
-		# before calling textureLoader(), because it is imported
-		# lazily by GafferUI.
-		import IECoreGL
-		IECoreGL.TextureLoader
+		with GafferUI.Window() as window :
+			gadgetWidget = GafferUI.GadgetWidget()
 
-		l = GafferUI.ImageGadget.textureLoader()
-		self.assertTrue( isinstance( l, IECoreGL.TextureLoader ) )
+		window.setVisible( True )
+
+		for path in IECore.SearchPath( os.environ["GAFFERUI_IMAGE_PATHS"] ).paths :
+			for image in pathlib.Path( path ).glob( "*.png" ) :
+				imageGadget = GafferUI.ImageGadget( str( image ) )
+				gadgetWidget.getViewportGadget().setPrimaryChild( imageGadget )
+				gadgetWidget.getViewportGadget().frame( imageGadget.bound() )
+				self.waitForIdle( 100 )
 
 if __name__ == "__main__":
 	unittest.main()

src/GafferUI/AnnotationsGadget.cpp

@@ -74,37 +74,15 @@ Box2f nodeFrame( const NodeGadget *nodeGadget )
 }
 
 
-IECoreGL::Texture *bookmarkTexture()
+const IECoreGL::Texture *bookmarkTexture()
 {
-	static IECoreGL::TexturePtr bookmarkTexture;
-
-	if( !bookmarkTexture )
-	{
-		bookmarkTexture = ImageGadget::textureLoader()->load( "bookmarkStar2.png" );
-
-		IECoreGL::Texture::ScopedBinding binding( *bookmarkTexture );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_LINEAR );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_BORDER );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_BORDER );
-	}
+	static IECoreGL::ConstTexturePtr bookmarkTexture = ImageGadget::loadTexture( "bookmarkStar2.png" );
 	return bookmarkTexture.get();
 }
 
-IECoreGL::Texture *numericBookmarkTexture()
+const IECoreGL::Texture *numericBookmarkTexture()
 {
-	static IECoreGL::TexturePtr numericBookmarkTexture;
-
-	if( !numericBookmarkTexture )
-	{
-		numericBookmarkTexture = ImageGadget::textureLoader()->load( "bookmarkStar.png" );
-
-		IECoreGL::Texture::ScopedBinding binding( *numericBookmarkTexture );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_LINEAR );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_BORDER );
-		glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_BORDER );
-	}
+	static IECoreGL::ConstTexturePtr numericBookmarkTexture = ImageGadget::loadTexture( "bookmarkStar.png" );
 	return numericBookmarkTexture.get();
 }