From cfc076fe456856aacf54972ab02772c56f68a612 Mon Sep 17 00:00:00 2001 From: ivanimanishi Date: Thu, 20 Aug 2026 13:35:59 -0700 Subject: [PATCH] ImageReader, TextureLoader : Remove special case for png colorspaces OIIO 3 doesn't have "linear" built-in anymore, and it now needs to come from the config. This is causing errors when trying to open png files, as often there isn't a colorspace specifically called "linear". The original reasons for hard-coding that colorspace don't seem to apply anymore, neither ImageEngine nor gaffer are loading UI icons that way, so it seems reasonable to remove it, and treat png files just like any other image. --- Changes | 5 +++++ src/IECoreGL/TextureLoader.cpp | 24 ++++-------------------- src/IECoreImage/ImageReader.cpp | 20 ++------------------ 3 files changed, 11 insertions(+), 38 deletions(-) diff --git a/Changes b/Changes index a19ab128ff..db9e4b3425 100644 --- a/Changes +++ b/Changes @@ -8,6 +8,11 @@ Improvements - TypeId : Added `format_as` overload, so that TypeIds can be passed to `fmt::format()`. - PrimitiveVariable : Added `format_as` overload for `Interpolation`, so that it can be passed to `fmt::format()`. +Fixes +----- + +- ImageReader, TextureLoader : Removed special case for colorspaces when opening pngs. + Build ----- diff --git a/src/IECoreGL/TextureLoader.cpp b/src/IECoreGL/TextureLoader.cpp index df719176b9..aeeb4d5059 100644 --- a/src/IECoreGL/TextureLoader.cpp +++ b/src/IECoreGL/TextureLoader.cpp @@ -145,30 +145,14 @@ TexturePtr TextureLoader::load( const std::string &name, int maximumResolution ) } // This logic feels pretty broken - why do we ask the current color config's - // display transform to decide what colorspace a file is stored in? Why special - // case just png. But I've currently copied this logic from ImageReader in the + // display transform to decide what colorspace a file is stored in? + // But I've currently copied this logic from ImageReader in the // name of backwards compatibility std::string linearColorSpace; std::string currentColorSpace; OIIO::string_view fileFormat = imageBuf.file_format_name(); - if( fileFormat == "png" ) - { - // The most common use for loading PNGs via Cortex is for icons in Gaffer. - // If we were to use the OCIO config to guess the colorspaces as below, we - // would get it spectacularly wrong. For instance, with an ACES config the - // resulting icons are so washed out as to be illegible. Instead, we hardcode - // the rudimentary colour spaces much more likely to be associated with a PNG. - // These are supported by OIIO regardless of what OCIO config is in use. - /// \todo Should this apply to other formats too? Can we somehow fix - /// `OpenImageIOAlgo::colorSpace` instead? - linearColorSpace = "linear"; - currentColorSpace = "sRGB"; - } - else - { - linearColorSpace = IECoreImage::OpenImageIOAlgo::colorSpace( "", imageBuf.spec() ); - currentColorSpace = IECoreImage::OpenImageIOAlgo::colorSpace( fileFormat, imageBuf.spec() ); - } + linearColorSpace = IECoreImage::OpenImageIOAlgo::colorSpace( "", imageBuf.spec() ); + currentColorSpace = IECoreImage::OpenImageIOAlgo::colorSpace( fileFormat, imageBuf.spec() ); if( !OIIO::ImageBufAlgo::colorconvert( imageBuf, imageBuf, currentColorSpace, linearColorSpace ) ) { diff --git a/src/IECoreImage/ImageReader.cpp b/src/IECoreImage/ImageReader.cpp index 49dad2f9f3..0fd51dbb51 100644 --- a/src/IECoreImage/ImageReader.cpp +++ b/src/IECoreImage/ImageReader.cpp @@ -479,24 +479,8 @@ class ImageReader::Implementation OIIO::TypeString, &fileFormat ); - if( strcmp( fileFormat, "png" ) == 0 ) - { - // The most common use for loading PNGs via Cortex is for icons in Gaffer. - // If we were to use the OCIO config to guess the colorspaces as below, we - // would get it spectacularly wrong. For instance, with an ACES config the - // resulting icons are so washed out as to be illegible. Instead, we hardcode - // the rudimentary colour spaces much more likely to be associated with a PNG. - // These are supported by OIIO regardless of what OCIO config is in use. - /// \todo Should this apply to other formats too? Can we somehow fix - /// `OpenImageIOAlgo::colorSpace` instead? - m_linearColorSpace = "linear"; - m_currentColorSpace = "sRGB"; - } - else - { - m_linearColorSpace = OpenImageIOAlgo::colorSpace( "", *spec ); - m_currentColorSpace = OpenImageIOAlgo::colorSpace( fileFormat, *spec ); - } + m_linearColorSpace = OpenImageIOAlgo::colorSpace( "", *spec ); + m_currentColorSpace = OpenImageIOAlgo::colorSpace( fileFormat, *spec ); return true; }