fix(png): alpha premultiplication adjustment and attribute

See discussion AcademySoftwareFoundation#4054
and PR AcademySoftwareFoundation#4315

In PR 4315, we fixed PNG read/write to do the required
premultiplication in linear space (that is, linearize, multiply by
alpha, then go back to the sRGB or gamma space). Which we really
believe is "correct."  Except that maybe there are a ton of
incorrectly made PNG files out there (maybe most of them?) where
partial alpha pixels had their premultiplication occur on the sRGB or
gamma values directly.

In this patch, we partly revert, allowing both potential behaviors,
controlled by an attribute "png:linear_premult", which instructs the
PNG driver to do any premultiplication in linear space if it's set to
nonzero. It can be set globally (via `OIIO::attribute()`), as well as
set/overridden on any individual file by setting the attribute in the
configuration hints for an ImageInput or in the spec for an
ImageOutput.

As presented in this patch, we're setting the default to 0, meaning
that by default we are reverting back to the old behavior of doing the
premultiply of partial alpha pixels on the direct values intead of in
a linear space. Applications or sites that are confident that any PNG
files they encounter are "correct" can set the attribute to do the
multiplication linearly.

I'm not 100% confident about the default, and so am very happy to
entertain arguments for keeping the default set to do the
multiplication in linear space.

Signed-off-by: Larry Gritz <[email protected]>

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:srgb_alpha_linear`` 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);
@@ -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;
@@ -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/run.py

@@ -22,7 +22,8 @@
 command += oiiotool ("--pattern fill:topleft=1,0,0,1:topright=0,1,0,1:bottomleft=0,0,1,1:bottomright=1,1,1,1 16x16 4 -d uint16 -o test16.png")
 
 # regression test for wrong gamma correction for partial alpha
-command += oiiotool ("src/alphagamma.png --printinfo:stats=1")
+command += oiiotool ("--oiioattrib png:linear_premult 1 " +
+                     "src/alphagamma.png --printinfo:stats=1")
 
 # Test high quality alpha deassociation using alpha value close to zero.
 # This example is inspired by Yafes on the Slack.