Color transform unpremult logic changes. (#1864)

After some deep thinking and discussion with people who know more color
than me, I have become convinced that you *almost always* want to make
sure to un-premultiply colors (i.e. divide by alpha) before doing color
space transformations with OCIO, and then re-multiply by alpha (to get
back to "associated alpha") immediately after the color transformation.

To support this,

* For the ImageBufAlgo color transformation functions, change the default
  value of the `unpremult` parameter from `false` to `true`.

* `oiiotool --autocc` will do the unpremult/premult steps surrounding any
  automatic color transformations that it does upon input and output.

* `oiiotool --colorconvert` and the various `--ocio` commands now take
  an optional modifier `:unpremult=1` which will cause the conversion to
  be internally bracked by a unpremult/premult step (when you do this,
  you no longer need an explicit --unpremult and --premult on the
  command line).

* oiiotool now issues warnings for cases where it has a good reason to
  think you might be double-unpremulting (for example, if you do both
  --unpremult and follow it by a --colorconvert:unpremult=1).

Later, we'll stage a second part of this where we change the default
value for unpremult= to be 1, so that the modifier is not needed (at which
point you REALLY don't want the explicit --unpremult/--premult, or it will
be wrong).

On the whole, I think this makes the situation less error prone for
casual users. Most oiiotool command lines should just use --autocc and
then not need to worry about any explicit use of `--colorconvert` or
`--unpremult/--premult`. For the rare case when more control is needed,
the individual commands and modifiers can be used.

Note that for oiiotool, `--autocc` is not on by default. If you want
images auto-converted to linear space and ouputs auto-converted from
linear to the desired output space, you need to use `--autocc`.

Author: lgritz

CHANGES.md

@@ -43,6 +43,12 @@ Public API changes:
   only be used to pass into certain IBA functions). The color space names
   "rgb" and "default" are now understood to be synonyms for the default
   "linear" color space. #1788 (1.9.0)
+* `ImageBufAlgo::colorconvert` and various `ocio` transformations have
+  changed the default value of their `unpremult` parameter from `false` to
+  `true`, reflecting the fact that we believe this is almost always the more
+  correct choice. Also, if their input image is clearly marked as having
+  unasociated alpha already, they will not bracket the color conversion with
+  the requested unpremult/premult. #1864 (1.9.2)
 * Remove long-deprecated API calls:
     * ImageBuf::get_pixels/get_pixel_channels varieties deprecated since 1.6.
     * ImageBuf::set_deep_value_uint, deprecated since 1.7.
@@ -68,6 +74,16 @@ Fixes, minor enhancements, and performance improvements:
        look good converted to greyscale, and usable by people with color
        blindness. #1820 (1.9.2)
     * oiiotool no longer enables autotile by default. #1856 (1.9.2)
+    * `--colorconvert`, `--tocolorspace`, and all of the `--ocio` commands
+      now take an optional modifier `:unpremult=1` which causes the color
+      conversion to be internally bracketed by unpremult/premult steps (if
+      the image has alpha and is not already marked as having unassociated
+      alpha). You should therefore prefer `--colorconvert:unpremult=1 from to`
+      rather than the more complex `--unpremult --colorconvert from to -premult`.
+      #1864 (1.9.2)
+    * `--autocc` will also cause unpremult/premult to bracket any color
+      transformations it does automatically for read and write (if the image
+      has alpha and does not appear to already be unassociated). #1864 (1.9.2)
 * ImageBufAlgo:
     * `color_map()` new  maps "inferno", "magma", "plasma", "viridis".
        #1820 (1.9.2)

src/doc/oiiotool.tex

@@ -1019,7 +1019,7 @@ \subsection*{Reading images}
 \apiend
 
 \apiitem{\ce --autocc}
-Turns on automatic color conversion: Every input image file will be
+Turns on automatic color space conversion: Every input image file will be
 immediately converted to a scene-referred linear color space, and every file
 written will be first transformed to an appropriate output color space based
 on the filename or type.   Additionally, if the name of an output file
@@ -2903,6 +2903,11 @@ \section{\oiiotool commands for color management}
  & {\cf value=}\emph{str} & Adds a key/value pair to the ``context'' that
   OpenColorIO will used when applying the look. Multiple key/value pairs
   may be specified by making each one a comma-separated list. \\
+ & {\cf unpremult=}\emph{val} & If the numeric \emph{val} is nonzero, the
+     pixel values will be ``un-premultipled'' (divided by alpha) prior to
+     the actual color conversion, and then re-multipled by alpha afterwards.
+     The default is 0, meaning the color transformation not will be
+     automatically bracketed by divide-by-alpha / mult-by-alpha operations. \\
  & {\cf strict=}\emph{val} & When nonzero (the default), an
      inability to perform the color transform will be a hard error. If
      strict is 0, inability to find the transformation will just print a
@@ -2936,6 +2941,11 @@ \section{\oiiotool commands for color management}
  & {\cf value=}\emph{str} & Adds a key/value pair to the ``context'' that
   OpenColorIO will used when applying the look. Multiple key/value pairs
   may be specified by making each one a comma-separated list. \\
+ & {\cf unpremult=}\emph{val} & If the numeric \emph{val} is nonzero, the
+     pixel values will be ``un-premultipled'' (divided by alpha) prior to
+     the actual color conversion, and then re-multipled by alpha afterwards.
+     The default is 0, meaning the color transformation not will be
+     automatically bracketed by divide-by-alpha / mult-by-alpha operations. \\
 \end{tabular}
 
 This command is only meaningful if OIIO was compiled with OCIO support
@@ -2966,6 +2976,11 @@ \section{\oiiotool commands for color management}
  & {\cf value=}\emph{str} & Adds a key/value pair to the ``context'' that
   OpenColorIO will used when applying the display transform. Multiple key/value pairs
   may be specified by making each one a comma-separated list. \\
+ & {\cf unpremult=}\emph{val} & If the numeric \emph{val} is nonzero, the
+     pixel values will be ``un-premultipled'' (divided by alpha) prior to
+     the actual color conversion, and then re-multipled by alpha afterwards.
+     The default is 0, meaning the color transformation not will be
+     automatically bracketed by divide-by-alpha / mult-by-alpha operations. \\
 \end{tabular}
 
 This command is only meaningful if OIIO was compiled with OCIO support
@@ -2989,6 +3004,11 @@ \section{\oiiotool commands for color management}
 \begin{tabular}{p{10pt} p{1in} p{3.75in}}
  & {\cf inverse=}\emph{val} & If \emph{val} is nonzero, inverts the 
   color transformation. \\
+ & {\cf unpremult=}\emph{val} & If the numeric \emph{val} is nonzero, the
+     pixel values will be ``un-premultipled'' (divided by alpha) prior to
+     the actual color conversion, and then re-multipled by alpha afterwards.
+     The default is 0, meaning the color transformation not will be
+     automatically bracketed by divide-by-alpha / mult-by-alpha operations. \\
 \end{tabular}
 
 This command is only meaningful if OIIO was compiled with OCIO support

src/doc/openimageio.tex

@@ -94,8 +94,8 @@
  \bigskip \\
 }
 \date{{\large
-Date: 5 Dec 2017
-\\ (with corrections, 4 Feb 2018)
+Date: 9 Feb 2018
+%\\ (with corrections, 4 Feb 2018)
 }}
 
 

src/include/OpenImageIO/imagebufalgo.h

@@ -970,15 +970,17 @@ bool OIIO_API rangeexpand (ImageBuf &dst, const ImageBuf &src,
 /// size as specified by roi.  If roi is not defined it will be all
 /// of dst, if dst is defined, or all of src, if dst is not yet defined.
 ///
-/// If unpremult is true, unpremultiply before color conversion, then
-/// premultiply after the color conversion.  You may want to use this
-/// flag if your image contains an alpha channel.
+/// If unpremult is true, divide the RGB channels by alpha (if it exists and
+/// is nonzero) before color conversion, then re-multiply by alpha after the
+/// after the color conversion. Passing unpremult=false skips this step,
+/// which may be desirable if you know that the image is "unassociated alpha"
+/// (a.k.a. "not pre-multiplied colors").
 ///
 /// Return true on success, false on error (with an appropriate error
 /// message set in dst).
 bool OIIO_API colorconvert (ImageBuf &dst, const ImageBuf &src,
                             string_view from, string_view to,
-                            bool unpremult=false,
+                            bool unpremult=true,
                             string_view context_key="",
                             string_view context_value="",
                             ColorConfig *colorconfig=NULL,
@@ -991,9 +993,11 @@ bool OIIO_API colorconvert (ImageBuf &dst, const ImageBuf &src,
 /// size as specified by roi.  If roi is not defined it will be all
 /// of dst, if dst is defined, or all of src, if dst is not yet defined.
 ///
-/// If unpremult is true, unpremultiply before color conversion, then
-/// premultiply after the color conversion.  You may want to use this
-/// flag if your image contains an alpha channel.
+/// If unpremult is true, divide the RGB channels by alpha (if it exists and
+/// is nonzero) before color conversion, then re-multiply by alpha after the
+/// after the color conversion. Passing unpremult=false skips this step,
+/// which may be desirable if you know that the image is "unassociated alpha"
+/// (a.k.a. "not pre-multiplied colors").
 ///
 /// Return true on success, false on error (with an appropriate error
 /// message set in dst).
@@ -1029,7 +1033,7 @@ bool OIIO_API colorconvert (float *color, int nchannels,
 /// message set in dst).
 bool OIIO_API ociolook (ImageBuf &dst, const ImageBuf &src,
                         string_view looks, string_view from, string_view to,
-                        bool unpremult=false, bool inverse=false,
+                        bool unpremult=true, bool inverse=false,
                         string_view key="", string_view value="",
                         ColorConfig *colorconfig=NULL,
                         ROI roi=ROI::All(), int nthreads=0);
@@ -1038,23 +1042,25 @@ bool OIIO_API ociolook (ImageBuf &dst, const ImageBuf &src,
 /// "display" transform.  If from or looks are NULL, it will not
 /// override the look or source color space (subtly different than
 /// passing "", the empty string, which means to use no look or source
-/// space).
+/// space). If inverse is true, it will reverse the color transformation.
 ///
 /// If dst is not yet initialized, it will be allocated to the same
 /// size as specified by roi.  If roi is not defined it will be all
 /// of dst, if dst is defined, or all of src, if dst is not yet defined.
 /// In-place operations (dst == src) are supported.
 ///
-/// If unpremult is true, unpremultiply before color conversion, then
-/// premultiply after the color conversion.  You may want to use this
-/// flag if your image contains an alpha channel.
+/// If unpremult is true, divide the RGB channels by alpha (if it exists and
+/// is nonzero) before color conversion, then re-multiply by alpha after the
+/// after the color conversion. Passing unpremult=false skips this step,
+/// which may be desirable if you know that the image is "unassociated alpha"
+/// (a.k.a. "not pre-multiplied colors").
 ///
 /// Return true on success, false on error (with an appropriate error
 /// message set in dst).
 bool OIIO_API ociodisplay (ImageBuf &dst, const ImageBuf &src,
                         string_view display, string_view view,
                         string_view from="", string_view looks="",
-                        bool unpremult=false,
+                        bool unpremult=true,
                         string_view key="", string_view value="",
                         ColorConfig *colorconfig=NULL,
                         ROI roi=ROI::All(), int nthreads=0);
@@ -1067,15 +1073,17 @@ bool OIIO_API ociodisplay (ImageBuf &dst, const ImageBuf &src,
 /// size as specified by roi.  If roi is not defined it will be all
 /// of dst, if dst is defined, or all of src, if dst is not yet defined.
 ///
-/// If unpremult is true, unpremultiply before color conversion, then
-/// premultiply after the color conversion.  You may want to use this
-/// flag if your image contains an alpha channel. 
+/// If unpremult is true, divide the RGB channels by alpha (if it exists and
+/// is nonzero) before color conversion, then re-multiply by alpha after the
+/// after the color conversion. Passing unpremult=false skips this step,
+/// which may be desirable if you know that the image is "unassociated alpha"
+/// (a.k.a. "not pre-multiplied colors").
 ///
 /// Return true on success, false on error (with an appropriate error
 /// message set in dst).
 bool OIIO_API ociofiletransform (ImageBuf &dst, const ImageBuf &src,
                                  string_view name,
-                                 bool unpremult=false, bool inverse=false,
+                                 bool unpremult=true, bool inverse=false,
                                  ColorConfig *colorconfig=NULL,
                                  ROI roi=ROI::All(), int nthreads=0);
 

src/libOpenImageIO/color_ocio.cpp

@@ -1236,6 +1236,13 @@ ImageBufAlgo::colorconvert (ImageBuf &dst, const ImageBuf &src,
                                     roi.chbegin, src, roi, nthreads);
     }
 
+    if (unpremult && src.spec().alpha_channel >= 0 &&
+        src.spec().get_int_attribute("oiio:UnassociatedAlpha") != 0) {
+        // If we appear to be operating on an image that already has
+        // unassociated alpha, don't do a redundant unpremult step.
+        unpremult = false;
+    }
+
     bool ok = true;
     OIIO_DISPATCH_COMMON_TYPES2 (ok, "colorconvert", colorconvert_impl,
                                  dst.spec().format, src.spec().format,

src/libOpenImageIO/imagebufalgo_pixelmath.cpp

@@ -562,7 +562,11 @@ ImageBufAlgo::unpremult (ImageBuf &dst, const ImageBuf &src,
 {
     if (! IBAprep (roi, &dst, &src, IBAprep_CLAMP_MUTUAL_NCHANNELS))
         return false;
-    if (src.spec().alpha_channel < 0) {
+    if (src.spec().alpha_channel < 0 
+        // Wise?  || src.spec().get_int_attribute("oiio:UnassociatedAlpha") != 0
+        ) {
+        // If there is no alpha channel, just *copy* instead of dividing
+        // by alpha.
         if (&dst != &src)
             return paste (dst, src.spec().x, src.spec().y, src.spec().z,
                           roi.chbegin, src, roi, nthreads);
@@ -571,6 +575,8 @@ ImageBufAlgo::unpremult (ImageBuf &dst, const ImageBuf &src,
     bool ok;
     OIIO_DISPATCH_COMMON_TYPES2 (ok, "unpremult", unpremult_, dst.spec().format,
                           src.spec().format, dst, src, roi, nthreads);
+    // Mark the output as having unassociated alpha
+    dst.specmod().attribute ("oiio:UnassociatedAlpha", 1);
     return ok;
 }
 
@@ -624,6 +630,8 @@ ImageBufAlgo::premult (ImageBuf &dst, const ImageBuf &src,
     bool ok;
     OIIO_DISPATCH_COMMON_TYPES2 (ok, "premult", premult_, dst.spec().format,
                           src.spec().format, dst, src, roi, nthreads);
+    // Clear the output of any prior marking of associated alpha
+    dst.specmod().erase_attribute ("oiio:UnassociatedAlpha");
     return ok;
 }