diff --git a/src/OpenColorIO/fileformats/FileFormatICC.cpp b/src/OpenColorIO/fileformats/FileFormatICC.cpp index 476810112..69c0145d8 100755 --- a/src/OpenColorIO/fileformats/FileFormatICC.cpp +++ b/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::BASIC_FWD, + auto gamma = std::make_shared(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::BASIC_REV, + auto gamma = std::make_shared(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; } diff --git a/tests/cpu/fileformats/FileFormatICC_tests.cpp b/tests/cpu/fileformats/FileFormatICC_tests.cpp index e0c816093..d120e2487 100644 --- a/tests/cpu/fileformats/FileFormatICC_tests.cpp +++ b/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,26 +304,35 @@ 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("", ops[0]->getInfo()); + OCIO_CHECK_EQUAL("", ops[1]->getInfo()); // apply ops - float srcImage[] = { + const std::array 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 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 testImage = srcImage; for (const auto & op : ops) { - op->apply(srcImage, 3); + op->apply(testImage.data(), 3); } // Compare results @@ -328,7 +340,7 @@ OCIO_ADD_TEST(FileFormatICC, test_apply) 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("", opsInv[0]->getInfo()); + OCIO_CHECK_EQUAL("", 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); } }