Merge branch 'main' into cozdas/Issue2228_OutOfBoundReachInThrow

Author: doug-walker

share/ci/scripts/windows/install_doxygen.sh

@@ -10,7 +10,7 @@ DOXYGEN_LOCATION="$1"
 choco install jq
 
 # Get the URL of the latest zip package for Doxygen.
-url=$(curl -s 'https://api.github.com/repos/doxygen/doxygen/releases/latest' | jq -r '.assets[] | select(.name | test("doxygen-.*windows.x64.bin.zip")) | .browser_download_url')
+url=$(curl -s 'https://api.github.com/repos/doxygen/doxygen/releases/latest' | jq -r '.assets[] | select(.name | test("doxygen-.*\\.x64\\.bin\\.zip")) | .browser_download_url')
 
 # Download the zip.
 mkdir $DOXYGEN_LOCATION

src/OpenColorIO/fileformats/FileFormatICC.cpp

@@ -15,7 +15,6 @@
 #include "ops/gamma/GammaOp.h"
 #include "ops/lut1d/Lut1DOp.h"
 #include "ops/matrix/MatrixOp.h"
-#include "ops/range/RangeOp.h"
 #include "Platform.h"
 #include "transforms/FileTransform.h"
 
@@ -794,13 +793,23 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops,
         }
     }
 
-    // The matrix/TRC transform in the ICC profile converts display device code values to the
-    // CIE XYZ based version of the ICC profile connection space (PCS).
-    // However, in OCIO the most common use of an ICC monitor profile is as a display color space,
-    // and in that usage it is more natural for the XYZ to display code value transform to be called
-    // the forward direction.
+    // The matrix/TRC transform in the ICC profile converts display device code
+    // values to the CIE XYZ based version of the ICC profile connection space
+    // (PCS). However, in OCIO the most common use of an ICC monitor profile is
+    // as a display color space, and in that usage it is more natural for the
+    // XYZ to display code value transform to be called the forward direction.
 
-    // Curves / ParaCurves operates in the range 0.0 to 1.0 as per ICC specifications.
+    // The ICC spec states that the TRC tags should clamp to [0,1]. For curves
+    // that are implemented in the ICC profile as LUTs and most parametric
+    // curves (which become LUTs in OCIO), this is the case. However, as
+    // floating-point and HDR workflows become more common, the clamping has
+    // become a critical roadblock. For example, it is now common to have ICC
+    // profiles for linear color spaces that need to pass values outside [0,1].
+    // Therefore, OCIO now implements single entry 'curv' tags and type 0 'para'
+    // tags without clamping using an ExponentTransform which extends above 1
+    // and mirrors below 0. (Note that gamma values of 1 do not need to be
+    // tested for here since they will be omitted as no-ops later by the
+    // optimizer.)
 
     switch (newDir)
     {
@@ -817,18 +826,12 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops,
             const GammaOpData::Params greenParams = { cachedFile->mGammaRGB[1] };
             const GammaOpData::Params blueParams  = { cachedFile->mGammaRGB[2] };
             const GammaOpData::Params alphaParams = { cachedFile->mGammaRGB[3] };
-            auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_FWD, 
+            auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_MIRROR_FWD,
                                                        redParams,
                                                        greenParams,
                                                        blueParams,
                                                        alphaParams);
 
-            // GammaOp will clamp at 0 so we don't do it in the RangeOp.
-            CreateRangeOp(ops,
-                          RangeOpData::EmptyValue(), 1,
-                          RangeOpData::EmptyValue(), 1,
-                          TRANSFORM_DIR_FORWARD);
-
             CreateGammaOp(ops, gamma, TRANSFORM_DIR_FORWARD);
         }
 
@@ -859,18 +862,13 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops,
             const GammaOpData::Params greenParams = { cachedFile->mGammaRGB[1] };
             const GammaOpData::Params blueParams  = { cachedFile->mGammaRGB[2] };
             const GammaOpData::Params alphaParams = { cachedFile->mGammaRGB[3] };
-            auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_REV,
+            auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_MIRROR_REV,
                                                        redParams,
                                                        greenParams,
                                                        blueParams,
                                                        alphaParams);
 
             CreateGammaOp(ops, gamma, TRANSFORM_DIR_FORWARD);
-
-            CreateRangeOp(ops,
-                          RangeOpData::EmptyValue(), 1,
-                          RangeOpData::EmptyValue(), 1,
-                          TRANSFORM_DIR_FORWARD);
         }
         break;
     }

tests/cpu/fileformats/FileFormatICC_tests.cpp

@@ -244,6 +244,8 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
 {
     OCIO::ContextRcPtr context = OCIO::Context::Create();
     {
+        // This test uses a profile where the TRC is a 1024 element LUT.
+
         static const std::string iccFileName("icc-test-3.icm");
         OCIO::OpRcPtrVec ops;
         OCIO_CHECK_NO_THROW(BuildOpsTest(ops, iccFileName, context, OCIO::TRANSFORM_DIR_INVERSE));
@@ -287,7 +289,8 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
             op->apply(srcImage, 3);
         }
 
-        // Values outside [0.0, 1.0] are clamped and won't round-trip.
+        // Currently the LUT-based TRC's clamp the values outside 
+        // [0.0, 1.0], thus those values won't round-trip.
         static constexpr float bckImage[] = {
             0.0f, 0.0f, 0.3f, 0.0f,
             0.4f, 0.5f, 0.6f, 0.5f,
@@ -301,34 +304,43 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
     }
 
     {
+        // This test uses a profile where the TRC is 
+        // a parametric curve of type 0 (basic gamma) with
+        // gamma values {2.174, 2.174, 2.174, 1.0}.
+
         static const std::string iccFileName("icc-test-2.pf");
         OCIO::OpRcPtrVec ops;
         OCIO_CHECK_NO_THROW(BuildOpsTest(ops, iccFileName, context, OCIO::TRANSFORM_DIR_INVERSE));
         OCIO_CHECK_NO_THROW(ops.finalize());
         OCIO_CHECK_NO_THROW(ops.optimize(OCIO::OPTIMIZATION_LOSSLESS));
+        OCIO_REQUIRE_EQUAL(2, ops.size());
+        OCIO_CHECK_EQUAL("<GammaOp>",        ops[0]->getInfo());
+        OCIO_CHECK_EQUAL("<MatrixOffsetOp>", ops[1]->getInfo());
 
         // apply ops
-        float srcImage[] = {
+        const std::array<float, 12> srcImage{
             -0.1f, 0.0f, 0.3f, 0.0f,
              0.4f, 0.5f, 0.6f, 0.5f,
              0.7f, 1.0f, 1.9f, 1.0f };
 
-        const float dstImage[] = {
-            0.012437f, 0.004702f, 0.070333f, 0.0f,
+        const std::array<float, 12> dstImage{
+            0.009241f, 0.003003f, 0.070198f, 0.0f,
             0.188392f, 0.206965f, 0.343595f, 0.5f,
-            0.693246f, 0.863199f, 1.07867f , 1.0f };
+            1.210462f, 1.058761f, 4.003706f, 1.0f };
+
+        std::array<float, 12> testImage = srcImage;
 
         for (const auto & op : ops)
         {
-            op->apply(srcImage, 3);
+            op->apply(testImage.data(), 3);
         }
 
         // Compare results
         const float error = 2e-5f;
 
         for (unsigned int i = 0; i<12; ++i)
         {
-            OCIO_CHECK_CLOSE(srcImage[i], dstImage[i], error);
+            OCIO_CHECK_CLOSE(testImage[i], dstImage[i], error);
         }
 
         // Invert the processing.
@@ -337,24 +349,22 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
         OCIO_CHECK_NO_THROW(BuildOpsTest(opsInv, iccFileName, context, OCIO::TRANSFORM_DIR_FORWARD));
         OCIO_CHECK_NO_THROW(opsInv.finalize());
         OCIO_CHECK_NO_THROW(opsInv.optimize(OCIO::OPTIMIZATION_LOSSLESS));
+        OCIO_REQUIRE_EQUAL(2, opsInv.size());
+        OCIO_CHECK_EQUAL("<MatrixOffsetOp>", opsInv[0]->getInfo());
+        OCIO_CHECK_EQUAL("<GammaOp>",        opsInv[1]->getInfo());
 
         for (const auto & op : opsInv)
         {
-            op->apply(srcImage, 3);
+            op->apply(testImage.data(), 3);
         }
 
-        // Values outside [0.0, 1.0] are clamped and won't round-trip.
-        const float bckImage[] = {
-            0.0f, 0.0f, 0.3f, 0.0f,
-            0.4f, 0.5f, 0.6f, 0.5f,
-            0.7f, 1.0f, 1.0f, 1.0f };
-
-        // Compare results
+        // For pure-gamma TRCs, values outside [0.0, 1.0] are NOT clamped
+        // thus those values should round-trip correctly.
         const float error2 = 2e-4f;
 
         for (unsigned int i = 0; i<12; ++i)
         {
-            OCIO_CHECK_CLOSE(srcImage[i], bckImage[i], error2);
+            OCIO_CHECK_CLOSE(testImage[i], srcImage[i], error2);
         }
     }