fix(png): alpha premultiplication adjustment and attribute

src/doc/builtinplugins.rst

@@ -1754,6 +1754,12 @@ attributes are supported:
      - ptr
      - Pointer to a ``Filesystem::IOProxy`` that will handle the I/O, for
        example by reading from memory rather than the file system.
+   * - ``png:linear_premult``
+     - int
+     - If nonzero, will convert  or gamma-encoded values to linear color
+       space for any premultiplication-by-alpha step done by the PNG reader.
+       If zero (the default), any needed premultiplication will happen directly
+       to the encoded values.
 
 **Configuration settings for PNG output**
 
@@ -1797,13 +1803,31 @@ control aspects of the writing itself:
        to have larger PNG files on disk, you may want to use that value for
        this attribute.
 
+   * - ``png:linear_premult``
+     - int
+     - If nonzero, will convert sRGB or gamma-encoded values to linear color
+       space for any unpremultiplication-by-alpha step done by the PNG writer.
+       If zero (the default), any needed unpremultiplication will happen
+       directly to the encoded sRGB or gamma-corrected values.
+
 **Custom I/O Overrides**
 
 PNG input and output both support the "custom I/O" feature via the special
 ``"oiio:ioproxy"`` attributes (see Sections :ref:`sec-imageoutput-ioproxy`
 and :ref:`sec-imageinput-ioproxy`) as well as the `set_ioproxy()` methods.
 
-
+**Note on premultiplication**
+
+PNG files encoded as sRGB or gamma-corrected values that also have alpha
+should (in theory) have any premultiplication performed in a linear space
+(that is, the color should first be linearized, then premultiplied by alpha,
+then converted back to the nonlinear form). However, many existing PNG files
+are apparently encoded with the assumption that any premultiplication will be
+performed directly on the encoded values, so that is the default behavior for
+OpenImageIO's PNG reader and writer will. If you want to force the reader or
+writer to linearize the values for premultiplication, you can set either the
+reader/writer configuration hint or the global OIIO attribute
+``png:linear_premult`` to 1.
 
 **Limitations**
 

src/include/OpenImageIO/imageio.h

@@ -2908,6 +2908,14 @@ OIIO_API std::string geterror(bool clear = true);
 ///    and only set ImageDescription if the parsing fails. Otherwise, always
 ///    set ImageDescription to the first comment block. Default is 1.
 ///
+/// - `int png:linear_premult` (0)
+///
+///    If nonzero, will convert perform any necessary premultiplication by
+///    alpha steps needed of the PNG reader/writer in a linear color space.
+///    If zero (the default), any needed premultiplication will happen
+///    directly on the values, even if they are sRGB or gamma-corrected.
+///    For more information, please see OpenImageIO's documentation on the
+///    built-in PNG format support.
 ///
 /// - `int limits:channels` (1024)
 ///

src/include/imageio_pvt.h

@@ -43,6 +43,7 @@ extern OIIO_UTIL_API int oiio_print_uncaught_errors;
 extern int oiio_log_times;
 extern int openexr_core;
 extern int jpeg_com_attributes;
+extern int png_linear_premult;
 extern int limit_channels;
 extern int limit_imagesize_MB;
 extern int imagebuf_print_uncaught_errors;

src/libOpenImageIO/imageio.cpp

@@ -49,6 +49,7 @@ atomic_int oiio_try_all_readers(1);
 // Should we use "Exr core C library"?
 int openexr_core(OIIO_OPENEXR_CORE_DEFAULT);
 int jpeg_com_attributes(1);
+int png_linear_premult(0);
 int tiff_half(0);
 int tiff_multithread(1);
 int dds_bc5normal(0);
@@ -72,7 +73,7 @@ using namespace pvt;
 
 namespace {
 // Hidden global OIIO data.
-static spin_mutex attrib_mutex;
+static std::recursive_mutex attrib_mutex;
 static const int maxthreads = 512;  // reasonable maximum for sanity check
 
 class TimingLog {
@@ -347,7 +348,7 @@ attribute(string_view name, TypeDesc type, const void* val)
     }
 
     // Things below here need to buarded by the attrib_mutex
-    spin_lock lock(attrib_mutex);
+    std::lock_guard lock(attrib_mutex);
     if (name == "read_chunk" && type == TypeInt) {
         oiio_read_chunk = *(const int*)val;
         return true;
@@ -372,6 +373,10 @@ attribute(string_view name, TypeDesc type, const void* val)
         jpeg_com_attributes = *(const int*)val;
         return true;
     }
+    if (name == "png:linear_premult" && type == TypeInt) {
+        png_linear_premult = *(const int*)val;
+        return true;
+    }
     if (name == "tiff:half" && type == TypeInt) {
         tiff_half = *(const int*)val;
         return true;
@@ -460,7 +465,7 @@ getattribute(string_view name, TypeDesc type, void* val)
     }
 
     // Things below here need to buarded by the attrib_mutex
-    spin_lock lock(attrib_mutex);
+    std::lock_guard lock(attrib_mutex);
     if (name == "read_chunk" && type == TypeInt) {
         *(int*)val = oiio_read_chunk;
         return true;
@@ -551,6 +556,10 @@ getattribute(string_view name, TypeDesc type, void* val)
         *(int*)val = jpeg_com_attributes;
         return true;
     }
+    if (name == "png:linear_premult" && type == TypeInt) {
+        *(int*)val = png_linear_premult;
+        return true;
+    }
     if (name == "tiff:half" && type == TypeInt) {
         *(int*)val = tiff_half;
         return true;

src/png.imageio/pnginput.cpp

@@ -45,6 +45,7 @@ class PNGInput final : public ImageInput {
     Imath::Color3f m_bg;               ///< Background color
     int m_next_scanline;
     bool m_keep_unassociated_alpha;  ///< Do not convert unassociated alpha
+    bool m_linear_premult;           ///< Do premult for sRGB images in linear
     bool m_srgb   = false;           ///< It's an sRGB image (not gamma)
     bool m_err    = false;
     float m_gamma = 1.0f;
@@ -60,9 +61,10 @@ class PNGInput final : public ImageInput {
         m_buf.clear();
         m_next_scanline           = 0;
         m_keep_unassociated_alpha = false;
-        m_srgb                    = false;
-        m_err                     = false;
-        m_gamma                   = 1.0;
+        m_linear_premult = OIIO::get_int_attribute("png:linear_premult");
+        m_srgb           = false;
+        m_err            = false;
+        m_gamma          = 1.0;
         m_config.reset();
         ioproxy_clear();
     }
@@ -88,8 +90,8 @@ class PNGInput final : public ImageInput {
     }
 
     template<class T>
-    static void associateAlpha(T* data, int size, int channels,
-                               int alpha_channel, bool srgb, float gamma);
+    void associateAlpha(T* data, int size, int channels, int alpha_channel,
+                        bool srgb, float gamma);
 };
 
 
@@ -189,6 +191,9 @@ PNGInput::open(const std::string& name, ImageSpec& newspec,
     // Check 'config' for any special requests
     if (config.get_int_attribute("oiio:UnassociatedAlpha", 0) == 1)
         m_keep_unassociated_alpha = true;
+    m_linear_premult = config.get_int_attribute("png:linear_premult",
+                                                OIIO::get_int_attribute(
+                                                    "png:linear_premult"));
     ioproxy_retrieve_from_config(config);
     m_config.reset(new ImageSpec(config));  // save config spec
     return open(name, newspec);
@@ -229,7 +234,8 @@ PNGInput::associateAlpha(T* data, int size, int channels, int alpha_channel,
 {
     // We need to transform to linear space, associate the alpha, and then
     // transform back.
-    if (srgb) {
+    if (srgb && m_linear_premult) {
+        // sRGB with request to do premult in linear space
         for (int x = 0; x < size; ++x, data += channels) {
             DataArrayProxy<T, float> val(data);
             float alpha = val[alpha_channel];
@@ -242,25 +248,29 @@ PNGInput::associateAlpha(T* data, int size, int channels, int alpha_channel,
                 }
             }
         }
-    } else if (gamma == 1.0f) {
+    } else if (gamma != 1.0f && m_linear_premult) {
+        // Gamma correction with request to do premult in linear space
+        float inv_gamma = 1.0f / gamma;
         for (int x = 0; x < size; ++x, data += channels) {
             DataArrayProxy<T, float> val(data);
             float alpha = val[alpha_channel];
             if (alpha != 1.0f) {
                 for (int c = 0; c < channels; c++)
                     if (c != alpha_channel)
-                        data[c] = data[c] * alpha;
+                        val[c] = powf((powf(val[c], gamma)) * alpha, inv_gamma);
             }
         }
-    } else {  // With gamma correction
-        float inv_gamma = 1.0f / gamma;
+    } else {
+        // Do the premult directly on the values. This is correct for the
+        // "gamma=1" case, and is also commonly what is needed for many sRGB
+        // images (even though it's technically wrong in that case).
         for (int x = 0; x < size; ++x, data += channels) {
             DataArrayProxy<T, float> val(data);
             float alpha = val[alpha_channel];
             if (alpha != 1.0f) {
                 for (int c = 0; c < channels; c++)
                     if (c != alpha_channel)
-                        val[c] = powf((powf(val[c], gamma)) * alpha, inv_gamma);
+                        val[c] = val[c] * alpha;
             }
         }
     }

src/png.imageio/pngoutput.cpp

@@ -47,6 +47,7 @@ class PNGOutput final : public ImageOutput {
     int m_color_type;       ///< PNG color model type
     bool m_convert_alpha;   ///< Do we deassociate alpha?
     bool m_need_swap;       ///< Do we need to swap bytes?
+    bool m_linear_premult;  ///< Do premult for sRGB images in linear
     bool m_srgb   = false;  ///< It's an sRGB image (not gamma)
     float m_gamma = 1.0f;   ///< Gamma to use for alpha conversion
     std::vector<unsigned char> m_scratch;
@@ -57,13 +58,14 @@ class PNGOutput final : public ImageOutput {
     // Initialize private members to pre-opened state
     void init(void)
     {
-        m_png           = NULL;
-        m_info          = NULL;
-        m_convert_alpha = true;
-        m_need_swap     = false;
-        m_srgb          = false;
-        m_err           = false;
-        m_gamma         = 1.0;
+        m_png            = NULL;
+        m_info           = NULL;
+        m_convert_alpha  = true;
+        m_need_swap      = false;
+        m_linear_premult = false;
+        m_srgb           = false;
+        m_err            = false;
+        m_gamma          = 1.0;
         m_pngtext.clear();
         ioproxy_clear();
     }
@@ -187,6 +189,10 @@ PNGOutput::open(const std::string& name, const ImageSpec& userspec,
 
     m_need_swap = (m_spec.format == TypeDesc::UINT16 && littleendian());
 
+    m_linear_premult = m_spec.get_int_attribute("png:linear_premult",
+                                                OIIO::get_int_attribute(
+                                                    "png:linear_premult"));
+
     png_set_filter(m_png, 0,
                    spec().get_int_attribute("png:filter", PNG_NO_FILTERS));
     // https://www.w3.org/TR/PNG-Encoders.html#E.Filter-selection
@@ -277,7 +283,8 @@ void
 PNGOutput::deassociateAlpha(T* data, size_t npixels, int channels,
                             int alpha_channel, bool srgb, float gamma)
 {
-    if (srgb) {
+    if (srgb && m_linear_premult) {
+        // sRGB with request to do unpremult in linear space
         for (size_t x = 0; x < npixels; ++x, data += channels) {
             DataArrayProxy<T, float> val(data);
             float alpha = val[alpha_channel];
@@ -290,27 +297,31 @@ PNGOutput::deassociateAlpha(T* data, size_t npixels, int channels,
                 }
             }
         }
-    } else if (gamma == 1) {
+    } else if (gamma != 1.0f && m_linear_premult) {
+        // Gamma correction with request to do unpremult in linear space
         for (size_t x = 0; x < npixels; ++x, data += channels) {
             DataArrayProxy<T, float> val(data);
             float alpha = val[alpha_channel];
             if (alpha != 0.0f && alpha != 1.0f) {
+                // See associateAlpha() for an explanation.
+                float alpha_deassociate = pow(1.0f / val[alpha_channel], gamma);
                 for (int c = 0; c < channels; c++) {
                     if (c != alpha_channel)
-                        val[c] = data[c] / alpha;
+                        val[c] = val[c] * alpha_deassociate;
                 }
             }
         }
     } else {
+        // Do the unpremult directly on the values. This is correct for the
+        // "gamma=1" case, and is also commonly what is needed for many sRGB
+        // images (even though it's technically wrong in that case).
         for (size_t x = 0; x < npixels; ++x, data += channels) {
             DataArrayProxy<T, float> val(data);
             float alpha = val[alpha_channel];
             if (alpha != 0.0f && alpha != 1.0f) {
-                // See associateAlpha() for an explanation.
-                float alpha_deassociate = pow(1.0f / val[alpha_channel], gamma);
                 for (int c = 0; c < channels; c++) {
                     if (c != alpha_channel)
-                        val[c] = val[c] * alpha_deassociate;
+                        val[c] = data[c] / alpha;
                 }
             }
         }

testsuite/png/ref/out-libpng15.txt

@@ -27,6 +27,7 @@ exif.png             :   64 x   64, 3 channel, uint8 png
     SHA-1: 7CB41FEA50720B48BE0C145E1473982B23E9AB77
     channel list: R, G, B
     oiio:ColorSpace: "sRGB"
+alphagamma:
    1 x    1, 4 channel, float png
     channel list: R, G, B, A
     ResolutionUnit: "inch"
@@ -46,6 +47,43 @@ exif.png             :   64 x   64, 3 channel, uint8 png
     Constant: Yes
     Constant Color: 186.00 186.00 186.00 127.00 (of 255)
     Monochrome: No
+gimp_gradient:
+ 256 x  256, 4 channel, float png
+    channel list: R, G, B, A
+    Comment: "Created with GIMP"
+    DateTime: "2025:01:05 04:44:59"
+    ICCProfile: 0, 0, 2, 160, 108, 99, 109, 115, 4, 64, 0, 0, 109, 110, 116, 114, ... [672 x uint8]
+    ResolutionUnit: "inch"
+    XResolution: 300
+    YResolution: 300
+    ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
+    ICCProfile:cmm_type: 1818455411
+    ICCProfile:color_space: "RGB"
+    ICCProfile:copyright: "Public Domain"
+    ICCProfile:creation_date: "2025:01:05 04:34:16"
+    ICCProfile:creator_signature: "6c636d73"
+    ICCProfile:device_class: "Display device profile"
+    ICCProfile:device_manufacturer_description: "GIMP"
+    ICCProfile:device_model_description: "sRGB"
+    ICCProfile:flags: "Not Embedded, Independent"
+    ICCProfile:manufacturer: "0"
+    ICCProfile:model: "0"
+    ICCProfile:platform_signature: "Apple Computer, Inc."
+    ICCProfile:profile_connection_space: "XYZ"
+    ICCProfile:profile_description: "GIMP built-in sRGB"
+    ICCProfile:profile_size: 672
+    ICCProfile:profile_version: "4.4.0"
+    ICCProfile:rendering_intent: "Perceptual"
+    oiio:ColorSpace: "sRGB"
+    Stats Min: 0 0 0 0 (of 255)
+    Stats Max: 255 255 0 255 (of 255)
+    Stats Avg: 142.37 105.72 0.00 154.72 (of 255)
+    Stats StdDev: 79.19 98.91 0.00 87.39 (of 255)
+    Stats NanCount: 0 0 0 0 
+    Stats InfCount: 0 0 0 0 
+    Stats FiniteCount: 65536 65536 65536 65536 
+    Constant: No
+    Monochrome: No
 smallalpha.png       :    1 x    1, 4 channel, uint8 png
     Pixel (0, 0): 240 108 119 1 (0.94117653 0.42352945 0.4666667 0.003921569)
 Comparing "test16.png" and "ref/test16.png"