From e15186547120ad263c003f19f019d226589e824a Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Fri, 12 Sep 2025 13:46:06 -0400 Subject: [PATCH 01/15] Attempt to fix CI fail involving halfs (#2188) Signed-off-by: Doug Walker --- tests/cpu/fileformats/FileFormatCTF_tests.cpp | 2 +- tests/cpu/ops/lut1d/Lut1DOpCPU_tests.cpp | 58 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/tests/cpu/fileformats/FileFormatCTF_tests.cpp b/tests/cpu/fileformats/FileFormatCTF_tests.cpp index e4906fd40b..b9b2662165 100644 --- a/tests/cpu/fileformats/FileFormatCTF_tests.cpp +++ b/tests/cpu/fileformats/FileFormatCTF_tests.cpp @@ -5124,7 +5124,7 @@ OCIO_ADD_TEST(CTFTransform, save_lut1d_halfdomain) } else { - OCIO_CHECK_EQUAL(loaded, expected); + OCIO_CHECK_EQUAL(loaded.bits(), expected.bits()); OCIO_CHECK_EQUAL(loadedVal, lut->getArray()[3 * i + 1]); OCIO_CHECK_EQUAL(loadedVal, lut->getArray()[3 * i + 2]); } diff --git a/tests/cpu/ops/lut1d/Lut1DOpCPU_tests.cpp b/tests/cpu/ops/lut1d/Lut1DOpCPU_tests.cpp index 94b45fb4c8..f4fa4aa9c9 100644 --- a/tests/cpu/ops/lut1d/Lut1DOpCPU_tests.cpp +++ b/tests/cpu/ops/lut1d/Lut1DOpCPU_tests.cpp @@ -643,25 +643,25 @@ OCIO_ADD_TEST(Lut1DRenderer, bit_depth_support) cpuOp->apply(&uint8_inImg[0], &outImg[0], NB_PIXELS); - OCIO_CHECK_EQUAL(outImg[ 0], 0.0f); - OCIO_CHECK_EQUAL(outImg[ 1], 0.0f); - OCIO_CHECK_EQUAL(outImg[ 2], 0.0f); - OCIO_CHECK_EQUAL(outImg[ 3], 0.0f); - - OCIO_CHECK_CLOSE(outImg[ 4], 0.066650390625f, 1e-6f); - OCIO_CHECK_CLOSE(outImg[ 5], 0.070617675781f, 1e-6f); - OCIO_CHECK_CLOSE(outImg[ 6], 0.070617675781f, 1e-6f); - OCIO_CHECK_EQUAL(outImg[ 7], 1.0f); - - OCIO_CHECK_CLOSE(outImg[ 8], 0.7138671875f, 1e-6f); - OCIO_CHECK_CLOSE(outImg[ 9], 0.7255859375f, 1e-6f); - OCIO_CHECK_CLOSE(outImg[10], 0.7373046875f, 1e-6f); - OCIO_CHECK_EQUAL(outImg[11], 0.0f); - - OCIO_CHECK_EQUAL(outImg[12], 1.0f); - OCIO_CHECK_EQUAL(outImg[13], 1.0f); - OCIO_CHECK_EQUAL(outImg[14], 1.0f); - OCIO_CHECK_EQUAL(outImg[15], 1.0f); + OCIO_CHECK_EQUAL((float)outImg[ 0], 0.0f); + OCIO_CHECK_EQUAL((float)outImg[ 1], 0.0f); + OCIO_CHECK_EQUAL((float)outImg[ 2], 0.0f); + OCIO_CHECK_EQUAL((float)outImg[ 3], 0.0f); + + OCIO_CHECK_CLOSE((float)outImg[ 4], 0.066650390625f, 1e-6f); + OCIO_CHECK_CLOSE((float)outImg[ 5], 0.070617675781f, 1e-6f); + OCIO_CHECK_CLOSE((float)outImg[ 6], 0.070617675781f, 1e-6f); + OCIO_CHECK_EQUAL((float)outImg[ 7], 1.0f); + + OCIO_CHECK_CLOSE((float)outImg[ 8], 0.7138671875f, 1e-6f); + OCIO_CHECK_CLOSE((float)outImg[ 9], 0.7255859375f, 1e-6f); + OCIO_CHECK_CLOSE((float)outImg[10], 0.7373046875f, 1e-6f); + OCIO_CHECK_EQUAL((float)outImg[11], 0.0f); + + OCIO_CHECK_EQUAL((float)outImg[12], 1.0f); + OCIO_CHECK_EQUAL((float)outImg[13], 1.0f); + OCIO_CHECK_EQUAL((float)outImg[14], 1.0f); + OCIO_CHECK_EQUAL((float)outImg[15], 1.0f); } // Processing from UINT8 to F32. @@ -878,15 +878,15 @@ OCIO_ADD_TEST(Lut1DRenderer, half) std::vector outImg(2 * 4, -1.f); cpuOp->apply(&inImg[0], &outImg[0], 2); - OCIO_CHECK_EQUAL(outImg[0], inImg[0]); - OCIO_CHECK_EQUAL(outImg[1], inImg[1]); - OCIO_CHECK_EQUAL(outImg[2], inImg[2]); - OCIO_CHECK_EQUAL(outImg[3], inImg[3]); + OCIO_CHECK_EQUAL(outImg[0], (float)inImg[0]); + OCIO_CHECK_EQUAL(outImg[1], (float)inImg[1]); + OCIO_CHECK_EQUAL(outImg[2], (float)inImg[2]); + OCIO_CHECK_EQUAL(outImg[3], (float)inImg[3]); - OCIO_CHECK_EQUAL(outImg[4], inImg[4]); - OCIO_CHECK_EQUAL(outImg[5], inImg[5]); + OCIO_CHECK_EQUAL(outImg[4], (float)inImg[4]); + OCIO_CHECK_EQUAL(outImg[5], (float)inImg[5]); OCIO_CHECK_CLOSE(outImg[6], arbitraryVal, 1e-5f); - OCIO_CHECK_EQUAL(outImg[7], inImg[7]); + OCIO_CHECK_EQUAL(outImg[7], (float)inImg[7]); } OCIO_ADD_TEST(Lut1DRenderer, nan) @@ -1594,9 +1594,9 @@ OCIO_ADD_TEST(Lut1DRenderer, lut_1d_identity_half) } else { - OCIO_CHECK_EQUAL(outImg[imgCntr + 0], hVal); - OCIO_CHECK_EQUAL(outImg[imgCntr + 1], hVal); - OCIO_CHECK_EQUAL(outImg[imgCntr + 2], hVal); + OCIO_CHECK_EQUAL(outImg[imgCntr + 0].bits(), hVal.bits()); + OCIO_CHECK_EQUAL(outImg[imgCntr + 1].bits(), hVal.bits()); + OCIO_CHECK_EQUAL(outImg[imgCntr + 2].bits(), hVal.bits()); OCIO_CHECK_EQUAL((float)outImg[imgCntr + 3], 1.f); } } From b3252c7cd1a81f01ffea1421ec1528b90bfcbf0d Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Fri, 12 Sep 2025 14:16:45 -0400 Subject: [PATCH 02/15] Add mirrored displays (#2183) Signed-off-by: Doug Walker --- .../transforms/builtins/Displays.cpp | 119 ++++++++++++++---- .../cpu/transforms/BuiltinTransform_tests.cpp | 35 +++++- 2 files changed, 126 insertions(+), 28 deletions(-) diff --git a/src/OpenColorIO/transforms/builtins/Displays.cpp b/src/OpenColorIO/transforms/builtins/Displays.cpp index bd6f0c5e7c..a3e7689b13 100644 --- a/src/OpenColorIO/transforms/builtins/Displays.cpp +++ b/src/OpenColorIO/transforms/builtins/Displays.cpp @@ -176,27 +176,44 @@ void GenerateLinearToHLGOps(OpRcPtrVec& ops) void RegisterAll(BuiltinTransformRegistryImpl & registry) noexcept { + using ConversionFunctor = std::function; + { - auto CIE_XYZ_D65_to_REC1886_REC709_Functor = [](OpRcPtrVec & ops) + auto CIE_XYZ_D65_to_REC1886_REC709 = [](OpRcPtrVec & ops, GammaOpData::Style gammaStyle) { - MatrixOpData::MatrixArrayPtr matrix + MatrixOpData::MatrixArrayPtr matrix = build_conversion_matrix_from_XYZ_D65(REC709::primaries, ADAPTATION_NONE); CreateMatrixOp(ops, matrix, TRANSFORM_DIR_FORWARD); const GammaOpData::Params rgbParams = { 2.4 }; const GammaOpData::Params alphaParams = { 1.0 }; - auto gammaData = std::make_shared(GammaOpData::BASIC_REV, - rgbParams, rgbParams, rgbParams, alphaParams); + + auto gammaData = std::make_shared(gammaStyle, + rgbParams, rgbParams, rgbParams, alphaParams); CreateGammaOp(ops, gammaData, TRANSFORM_DIR_FORWARD); }; + ConversionFunctor CIE_XYZ_D65_to_REC1886_REC709_BASIC = + [CIE_XYZ_D65_to_REC1886_REC709](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_REC1886_REC709(ops, GammaOpData::BASIC_REV); + }; registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.709", - "Convert CIE XYZ (D65 white) to Rec.1886/Rec.709 (HD video)", - CIE_XYZ_D65_to_REC1886_REC709_Functor); + "Convert CIE XYZ (D65 white) to Rec.1886/Rec.709, clamp neg. values", + CIE_XYZ_D65_to_REC1886_REC709_BASIC); + + ConversionFunctor CIE_XYZ_D65_to_REC1886_REC709_MIRROR = + [CIE_XYZ_D65_to_REC1886_REC709](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_REC1886_REC709(ops, GammaOpData::BASIC_MIRROR_REV); + }; + registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.709 - MIRROR NEGS", + "Convert CIE XYZ (D65 white) to Rec.1886/Rec.709, mirror neg. values", + CIE_XYZ_D65_to_REC1886_REC709_MIRROR); } { - auto CIE_XYZ_D65_to_REC1886_REC2020_Functor = [](OpRcPtrVec & ops) + auto CIE_XYZ_D65_to_REC1886_REC2020 = [](OpRcPtrVec & ops, GammaOpData::Style gammaStyle) { MatrixOpData::MatrixArrayPtr matrix = build_conversion_matrix_from_XYZ_D65(REC2020::primaries, ADAPTATION_NONE); @@ -204,18 +221,32 @@ void RegisterAll(BuiltinTransformRegistryImpl & registry) noexcept const GammaOpData::Params rgbParams = { 2.4 }; const GammaOpData::Params alphaParams = { 1.0 }; - auto gammaData = std::make_shared(GammaOpData::BASIC_REV, + auto gammaData = std::make_shared(gammaStyle, rgbParams, rgbParams, rgbParams, alphaParams); CreateGammaOp(ops, gammaData, TRANSFORM_DIR_FORWARD); }; + ConversionFunctor CIE_XYZ_D65_to_REC1886_REC2020_BASIC = + [CIE_XYZ_D65_to_REC1886_REC2020](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_REC1886_REC2020(ops, GammaOpData::BASIC_REV); + }; registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.2020", - "Convert CIE XYZ (D65 white) to Rec.1886/Rec.2020 (UHD video)", - CIE_XYZ_D65_to_REC1886_REC2020_Functor); + "Convert CIE XYZ (D65 white) to Rec.1886/Rec.2020, clamp neg. values", + CIE_XYZ_D65_to_REC1886_REC2020_BASIC); + + ConversionFunctor CIE_XYZ_D65_to_REC1886_REC2020_MIRROR = + [CIE_XYZ_D65_to_REC1886_REC2020](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_REC1886_REC2020(ops, GammaOpData::BASIC_MIRROR_REV); + }; + registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.2020 - MIRROR NEGS", + "Convert CIE XYZ (D65 white) to Rec.1886/Rec.2020, mirror neg. values", + CIE_XYZ_D65_to_REC1886_REC2020_MIRROR); } { - auto CIE_XYZ_D65_to_G22_REC709_Functor = [](OpRcPtrVec & ops) + auto CIE_XYZ_D65_to_G22_REC709 = [](OpRcPtrVec & ops, GammaOpData::Style gammaStyle) { MatrixOpData::MatrixArrayPtr matrix = build_conversion_matrix_from_XYZ_D65(REC709::primaries, ADAPTATION_NONE); @@ -223,18 +254,32 @@ void RegisterAll(BuiltinTransformRegistryImpl & registry) noexcept const GammaOpData::Params rgbParams = { 2.2 }; const GammaOpData::Params alphaParams = { 1.0 }; - auto gammaData = std::make_shared(GammaOpData::BASIC_REV, + auto gammaData = std::make_shared(gammaStyle, rgbParams, rgbParams, rgbParams, alphaParams); CreateGammaOp(ops, gammaData, TRANSFORM_DIR_FORWARD); }; + ConversionFunctor CIE_XYZ_D65_to_G22_REC709_BASIC = + [CIE_XYZ_D65_to_G22_REC709](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_G22_REC709(ops, GammaOpData::BASIC_REV); + }; registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_G2.2-REC.709", - "Convert CIE XYZ (D65 white) to Gamma2.2, Rec.709", - CIE_XYZ_D65_to_G22_REC709_Functor); + "Convert CIE XYZ (D65 white) to Gamma2.2, Rec.709, clamp neg. values", + CIE_XYZ_D65_to_G22_REC709_BASIC); + + ConversionFunctor CIE_XYZ_D65_to_G22_REC709_MIRROR = + [CIE_XYZ_D65_to_G22_REC709](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_G22_REC709(ops, GammaOpData::BASIC_MIRROR_REV); + }; + registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_G2.2-REC.709 - MIRROR NEGS", + "Convert CIE XYZ (D65 white) to Gamma2.2, Rec.709, mirror neg. values", + CIE_XYZ_D65_to_G22_REC709_MIRROR); } { - auto CIE_XYZ_D65_to_SRGB_Functor = [](OpRcPtrVec & ops) + auto CIE_XYZ_D65_to_SRGB = [](OpRcPtrVec & ops, GammaOpData::Style gammaStyle) { MatrixOpData::MatrixArrayPtr matrix = build_conversion_matrix_from_XYZ_D65(REC709::primaries, ADAPTATION_NONE); @@ -242,14 +287,28 @@ void RegisterAll(BuiltinTransformRegistryImpl & registry) noexcept const GammaOpData::Params rgbParams = { 2.4, 0.055 }; const GammaOpData::Params alphaParams = { 1.0, 0.0 }; - auto gammaData = std::make_shared(GammaOpData::MONCURVE_REV, + auto gammaData = std::make_shared(gammaStyle, rgbParams, rgbParams, rgbParams, alphaParams); CreateGammaOp(ops, gammaData, TRANSFORM_DIR_FORWARD); }; + ConversionFunctor CIE_XYZ_D65_to_SRGB_MONCURVE = + [CIE_XYZ_D65_to_SRGB](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_SRGB(ops, GammaOpData::MONCURVE_REV); + }; registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_sRGB", "Convert CIE XYZ (D65 white) to sRGB (piecewise EOTF)", - CIE_XYZ_D65_to_SRGB_Functor); + CIE_XYZ_D65_to_SRGB_MONCURVE); + + ConversionFunctor CIE_XYZ_D65_to_SRGB_MIRROR = + [CIE_XYZ_D65_to_SRGB](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_SRGB(ops, GammaOpData::MONCURVE_MIRROR_REV); + }; + registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_sRGB - MIRROR NEGS", + "Convert CIE XYZ (D65 white) to sRGB (piecewise EOTF), mirror neg. values", + CIE_XYZ_D65_to_SRGB_MIRROR); } { @@ -272,7 +331,7 @@ void RegisterAll(BuiltinTransformRegistryImpl & registry) noexcept } { - auto CIE_XYZ_D65_to_P3_D65_Functor = [](OpRcPtrVec & ops) + auto CIE_XYZ_D65_to_P3_D65 = [](OpRcPtrVec & ops, GammaOpData::Style gammaStyle) { MatrixOpData::MatrixArrayPtr matrix = build_conversion_matrix_from_XYZ_D65(P3_D65::primaries, ADAPTATION_NONE); @@ -280,14 +339,28 @@ void RegisterAll(BuiltinTransformRegistryImpl & registry) noexcept const GammaOpData::Params rgbParams = { 2.6 }; const GammaOpData::Params alphaParams = { 1.0 }; - auto gammaData = std::make_shared(GammaOpData::BASIC_REV, + auto gammaData = std::make_shared(gammaStyle, rgbParams, rgbParams, rgbParams, alphaParams); CreateGammaOp(ops, gammaData, TRANSFORM_DIR_FORWARD); }; + ConversionFunctor CIE_XYZ_D65_to_P3_D65_BASIC = + [CIE_XYZ_D65_to_P3_D65](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_P3_D65(ops, GammaOpData::BASIC_REV); + }; registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_G2.6-P3-D65", - "Convert CIE XYZ (D65 white) to Gamma 2.6, P3-D65", - CIE_XYZ_D65_to_P3_D65_Functor); + "Convert CIE XYZ (D65 white) to Gamma 2.6, P3-D65, clamp neg. values", + CIE_XYZ_D65_to_P3_D65_BASIC); + + ConversionFunctor CIE_XYZ_D65_to_P3_D65_MIRROR = + [CIE_XYZ_D65_to_P3_D65](OpRcPtrVec& ops) + { + CIE_XYZ_D65_to_P3_D65(ops, GammaOpData::BASIC_MIRROR_REV); + }; + registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_G2.6-P3-D65 - MIRROR NEGS", + "Convert CIE XYZ (D65 white) to Gamma 2.6, P3-D65, mirror neg. values", + CIE_XYZ_D65_to_P3_D65_MIRROR); } { @@ -356,13 +429,13 @@ void RegisterAll(BuiltinTransformRegistryImpl & registry) noexcept }; registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_DisplayP3", - "Convert CIE XYZ (D65 white) to Apple Display P3", + "Convert CIE XYZ (D65 white) to Apple Display P3, mirror neg. values", CIE_XYZ_D65_to_DisplayP3_Functor); // NOTE: This builtin is defined to be able to partition SDR and HDR view transforms under two separate // displays rather than a single one. registry.addBuiltin("DISPLAY - CIE-XYZ-D65_to_DisplayP3-HDR", - "Convert CIE XYZ (D65 white) to Apple Display P3 (HDR)", + "Convert CIE XYZ (D65 white) to Apple Display P3 (HDR), mirror neg. values", CIE_XYZ_D65_to_DisplayP3_Functor); } diff --git a/tests/cpu/transforms/BuiltinTransform_tests.cpp b/tests/cpu/transforms/BuiltinTransform_tests.cpp index abb7ae8bb5..cc57edf412 100644 --- a/tests/cpu/transforms/BuiltinTransform_tests.cpp +++ b/tests/cpu/transforms/BuiltinTransform_tests.cpp @@ -636,22 +636,47 @@ AllValues UnitTestValues { "DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.709", { 1.0e-6f, - { 0.5f, 0.4f, 0.3f }, { 0.937245093108f, 0.586817090358f, 0.573498106368f } } }, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.937245093108f, 0.586817090358f, 0.573498106368f, 0.f, 0.505174310421f, 1.118456082347f } } }, + { "DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.709 - MIRROR NEGS", + { 1.0e-6f, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.937245093108f, 0.586817090358f, 0.573498106368f, -0.940082660458f, 0.505174310421f, 1.118456082347f } } }, { "DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.2020", { 1.0e-6f, - { 0.5f, 0.4f, 0.3f }, { 0.830338272693f, 0.620393283803f, 0.583385370254f } } }, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.830338272693f, 0.620393283803f, 0.583385370254f, 0.f, 0.432629991358f, 1.069355537167f } } }, + { "DISPLAY - CIE-XYZ-D65_to_REC.1886-REC.2020 - MIRROR NEGS", + { 1.0e-6f, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.830338272693f, 0.620393283803f, 0.583385370254f, -0.696883299726f, 0.432629991358f, 1.069355537167f } } }, { "DISPLAY - CIE-XYZ-D65_to_G2.2-REC.709", { 1.0e-6f, - { 0.5f, 0.4f, 0.3f }, { 0.931739212204f, 0.559058879141f, 0.545230761999f } } }, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.931739212204f, 0.559058879141f, 0.545230761999f, 0.f, 0.474767926071f, 1.129896956592f } } }, + { "DISPLAY - CIE-XYZ-D65_to_G2.2-REC.709 - MIRROR NEGS", + { 1.0e-6f, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.931739212204f, 0.559058879141f, 0.545230761999f, -0.934816978533f, 0.474767926071f, 1.129896956592f } } }, { "DISPLAY - CIE-XYZ-D65_to_sRGB", { 1.0e-6f, - { 0.5f, 0.4f, 0.3f }, { 0.933793573229f, 0.564092030327f, 0.550040502218f } } }, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.933793573229f, 0.564092030327f, 0.550040502218f, -11.142147651136028f, 0.477958897494f, 1.124971166876f } } }, + { "DISPLAY - CIE-XYZ-D65_to_sRGB - MIRROR NEGS", + { 1.0e-6f, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.933793573229f, 0.564092030327f, 0.550040502218f, -0.936787206783f, 0.477958897494f, 1.124971166876f } } }, { "DISPLAY - CIE-XYZ-D65_to_G2.6-P3-DCI-BFD", { 1.0e-6f, { 0.5f, 0.4f, 0.3f }, { 0.908856342287f, 0.627840575107f, 0.608053675805f } } }, { "DISPLAY - CIE-XYZ-D65_to_G2.6-P3-D65", { 1.0e-6f, - { 0.5f, 0.4f, 0.3f }, { 0.896805202281f, 0.627254277624f, 0.608228132100f } } }, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.896805202281f, 0.627254277624f, 0.608228132100f, 0.f, 0.493163009212f, 1.069368427937f } } }, + { "DISPLAY - CIE-XYZ-D65_to_G2.6-P3-D65 - MIRROR NEGS", + { 1.0e-6f, + { 0.5f, 0.4f, 0.3f, -0.05f, 0.05f, 1.25f }, + { 0.896805202281f, 0.627254277624f, 0.608228132100f, -0.859521292874f, 0.493163009212f, 1.069368427937f } } }, { "DISPLAY - CIE-XYZ-D65_to_G2.6-P3-D60-BFD", { 1.0e-6f, { 0.5f, 0.4f, 0.3f }, { 0.892433142142f, 0.627011653770f, 0.608093643982f } } }, From 0215dc3d6a41b9aaee4528fa8d0b514da6840f1e Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Sat, 7 Jun 2025 22:37:06 -0700 Subject: [PATCH 03/15] Adding 3 new attributes to the color space: interop_id, amf_transform_ids and icc_profile_name - Addressing the issues #1975, #2152 and #2153 - Bumped the config version to 2.5 - newly added attributes require v2.5 config both for serialization and de-serialization and will throw if they appear in older configs. - Hardened multiple existing functions against null parameters. Previously those functions were crashing if null is passed, as assigning null to std::string is undefined behavior. - Expanded tests to some affected functions' unit tests to include null passing. Setters will take null as empty string. compare functions will throw. Signed-off-by: cuneyt.ozdas --- include/OpenColorIO/OpenColorIO.h | 31 +++ src/OpenColorIO/Baker.cpp | 12 +- src/OpenColorIO/ColorSpace.cpp | 64 ++++- src/OpenColorIO/Config.cpp | 46 +++- src/OpenColorIO/Context.cpp | 2 +- src/OpenColorIO/Look.cpp | 6 +- src/OpenColorIO/OCIOYaml.cpp | 47 +++- src/OpenColorIO/Platform.cpp | 10 + src/OpenColorIO/ViewTransform.cpp | 6 +- src/bindings/python/PyColorSpace.cpp | 33 ++- tests/cpu/ColorSpace_tests.cpp | 365 ++++++++++++++++++++++++++- tests/cpu/Config_tests.cpp | 4 +- tests/python/ColorSpaceTest.py | 142 ++++++++++- 13 files changed, 728 insertions(+), 40 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index d97a376365..c379b451ba 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -1979,6 +1979,37 @@ class OCIOEXPORT ColorSpace const char * getDescription() const noexcept; void setDescription(const char * description); + /** + * Get/Set the interop ID for the color space. + * + * The interop ID is a standardized identifier to uniquely identify commonly + * used color spaces. These IDs are defined by the Academy Software + * Foundation's Color Interop Forum project. If you create your own ID, you + * must prefix it with unique characters that will ensure it won't conflict + * with future Color Interop Forum IDs. + */ + const char * getInteropID() const noexcept; + void setInteropID(const char * interopID); + + /** + * Get/Set the AMF transform IDs for the color space. + * + * The AMF transform IDs are used to identify specific transforms in the ACES Metadata File. + * Multiple transform IDs can be specified in a newline-separated string. + */ + const char * getAMFTransformIDs() const noexcept; + void setAMFTransformIDs(const char * amfTransformIDs); + + /** + * Get/Set the ICC profile name for the color space. + * + * The ICC profile name identifies the ICC color profile associated with this color space. + * This can be used to link OCIO color spaces with corresponding ICC profiles for + * applications that need to work with both color management systems. + */ + const char * getICCProfileName() const noexcept; + void setICCProfileName(const char * iccProfileName); + BitDepth getBitDepth() const noexcept; void setBitDepth(BitDepth bitDepth); diff --git a/src/OpenColorIO/Baker.cpp b/src/OpenColorIO/Baker.cpp index b8a78a958c..e92a4f3d31 100755 --- a/src/OpenColorIO/Baker.cpp +++ b/src/OpenColorIO/Baker.cpp @@ -126,7 +126,7 @@ void Baker::setFormat(const char * formatName) { if (formatInfoVec[i].capabilities & FORMAT_CAPABILITY_BAKE) { - getImpl()->m_formatName = formatName; + getImpl()->m_formatName = formatName ? formatName : ""; return; } } @@ -155,7 +155,7 @@ FormatMetadata & Baker::getFormatMetadata() void Baker::setInputSpace(const char * inputSpace) { - getImpl()->m_inputSpace = inputSpace; + getImpl()->m_inputSpace = inputSpace ? inputSpace : ""; } const char * Baker::getInputSpace() const @@ -165,7 +165,7 @@ const char * Baker::getInputSpace() const void Baker::setShaperSpace(const char * shaperSpace) { - getImpl()->m_shaperSpace = shaperSpace; + getImpl()->m_shaperSpace = shaperSpace ? shaperSpace : ""; } const char * Baker::getShaperSpace() const @@ -175,7 +175,7 @@ const char * Baker::getShaperSpace() const void Baker::setLooks(const char * looks) { - getImpl()->m_looks = looks; + getImpl()->m_looks = looks ? looks : ""; } const char * Baker::getLooks() const @@ -185,7 +185,7 @@ const char * Baker::getLooks() const void Baker::setTargetSpace(const char * targetSpace) { - getImpl()->m_targetSpace = targetSpace; + getImpl()->m_targetSpace = targetSpace ? targetSpace : ""; } const char * Baker::getTargetSpace() const @@ -205,7 +205,7 @@ const char * Baker::getView() const void Baker::setDisplayView(const char * display, const char * view) { - if (!display ^ !view) + if (!display || !view) { throw Exception("Both display and view must be set."); } diff --git a/src/OpenColorIO/ColorSpace.cpp b/src/OpenColorIO/ColorSpace.cpp index 4b7ecb25d0..7a57c0779f 100644 --- a/src/OpenColorIO/ColorSpace.cpp +++ b/src/OpenColorIO/ColorSpace.cpp @@ -24,6 +24,9 @@ class ColorSpace::Impl std::string m_equalityGroup; std::string m_description; std::string m_encoding; + std::string m_interopID; + std::string m_AMFTransformIDs; + std::string m_ICCProfileName; StringUtils::StringVec m_aliases; BitDepth m_bitDepth{ BIT_DEPTH_UNKNOWN }; @@ -62,6 +65,9 @@ class ColorSpace::Impl m_equalityGroup = rhs.m_equalityGroup; m_description = rhs.m_description; m_encoding = rhs.m_encoding; + m_interopID = rhs.m_interopID; + m_AMFTransformIDs = rhs.m_AMFTransformIDs; + m_ICCProfileName = rhs.m_ICCProfileName; m_bitDepth = rhs.m_bitDepth; m_isData = rhs.m_isData; m_referenceSpaceType = rhs.m_referenceSpaceType; @@ -195,7 +201,7 @@ const char * ColorSpace::getFamily() const noexcept void ColorSpace::setFamily(const char * family) { - getImpl()->m_family = family; + getImpl()->m_family = family ? family : ""; } const char * ColorSpace::getEqualityGroup() const noexcept @@ -205,7 +211,7 @@ const char * ColorSpace::getEqualityGroup() const noexcept void ColorSpace::setEqualityGroup(const char * equalityGroup) { - getImpl()->m_equalityGroup = equalityGroup; + getImpl()->m_equalityGroup = equalityGroup ? equalityGroup : ""; } const char * ColorSpace::getDescription() const noexcept @@ -215,7 +221,37 @@ const char * ColorSpace::getDescription() const noexcept void ColorSpace::setDescription(const char * description) { - getImpl()->m_description = description; + getImpl()->m_description = description ? description : ""; +} + +const char * ColorSpace::getInteropID() const noexcept +{ + return getImpl()->m_interopID.c_str(); +} + +void ColorSpace::setInteropID(const char * interopID) +{ + getImpl()->m_interopID = interopID ? interopID : ""; +} + +const char * ColorSpace::getAMFTransformIDs() const noexcept +{ + return getImpl()->m_AMFTransformIDs.c_str(); +} + +void ColorSpace::setAMFTransformIDs(const char * amfTransformIDs) +{ + getImpl()->m_AMFTransformIDs = amfTransformIDs ? amfTransformIDs : ""; +} + +const char * ColorSpace::getICCProfileName() const noexcept +{ + return getImpl()->m_ICCProfileName.c_str(); +} + +void ColorSpace::setICCProfileName(const char * iccProfileName) +{ + getImpl()->m_ICCProfileName = iccProfileName ? iccProfileName : ""; } BitDepth ColorSpace::getBitDepth() const noexcept @@ -265,7 +301,7 @@ const char * ColorSpace::getEncoding() const noexcept void ColorSpace::setEncoding(const char * encoding) { - getImpl()->m_encoding = encoding; + getImpl()->m_encoding = encoding ? encoding : ""; } bool ColorSpace::isData() const noexcept @@ -371,7 +407,6 @@ std::ostream & operator<< (std::ostream & os, const ColorSpace & cs) break; } os << "name=" << cs.getName() << ", "; - std::string str{ cs.getFamily() }; const auto numAliases = cs.getNumAliases(); if (numAliases == 1) { @@ -386,6 +421,15 @@ std::ostream & operator<< (std::ostream & os, const ColorSpace & cs) } os << "], "; } + + std::string str; + + str = cs.getInteropID(); + if (!str.empty()) + { + os << "interop_id=" << str << ", "; + } + str = cs.getFamily(); if (!str.empty()) { os << "family=" << str << ", "; @@ -429,6 +473,16 @@ std::ostream & operator<< (std::ostream & os, const ColorSpace & cs) { os << ", description=" << str; } + str = cs.getAMFTransformIDs(); + if (!str.empty()) + { + os << ", amf_transform_ids=" << str; + } + str = cs.getICCProfileName(); + if (!str.empty()) + { + os << ", icc_profile_name=" << str; + } if(cs.getTransform(COLORSPACE_DIR_TO_REFERENCE)) { os << ",\n " << cs.getName() << " --> Reference"; diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 727d434e9f..d2f9f1f040 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -247,7 +247,7 @@ static constexpr unsigned LastSupportedMajorVersion = OCIO_VERSION_MAJOR; // For each major version keep the most recent minor. static const unsigned int LastSupportedMinorVersion[] = {0, // Version 1 - 4 // Version 2 + 5 // Version 2 }; } // namespace @@ -1185,7 +1185,7 @@ ConstConfigRcPtr Config::CreateFromFile(const char * filename) // Check if it is an OCIOZ archive. if (magicNumber[0] == 'P' && magicNumber[1] == 'K') { - // Closing ifstream even though it should be close by ifstream deconstructor (RAII). + // Closing ifstream even though it should be closed by ifstream destructor (RAII). ifstream.close(); // The file should be an OCIOZ archive file. @@ -1499,7 +1499,8 @@ void Config::validate() const // Check for interchange roles requirements - scene-referred and display-referred. - if (getMajorVersion() >= 2 && getMinorVersion() >= 2) + unsigned int versionHex = (getMajorVersion() << 24) | (getMinorVersion() << 16); + if (versionHex >= 0x02020000u) // v2.2 or higher { bool hasRoleSceneLinear = false; bool hasRoleCompositingLog = false; @@ -5589,6 +5590,8 @@ void Config::Impl::checkVersionConsistency(ConstTransformRcPtr & transform) cons void Config::Impl::checkVersionConsistency() const { + unsigned int hexVersion = (m_majorVersion << 24) | (m_minorVersion << 16); + // Check for the Transforms. ConstTransformVec transforms; @@ -5658,18 +5661,43 @@ void Config::Impl::checkVersionConsistency() const } } - // Check for the DisplayColorSpaces. + // Check for the ColorSpaces. - if (m_majorVersion < 2) + const int nbCS = m_allColorSpaces->getNumColorSpaces(); + for (int i = 0; i < nbCS; ++i) { - const int nbCS = m_allColorSpaces->getNumColorSpaces(); - for (int i = 0; i < nbCS; ++i) + const auto & cs = m_allColorSpaces->getColorSpaceByIndex(i); + if (m_majorVersion < 2) { - const auto & cs = m_allColorSpaces->getColorSpaceByIndex(i); - if (MatchReferenceType(SEARCH_REFERENCE_SPACE_DISPLAY, cs->getReferenceSpaceType())) + if (MatchReferenceType(SEARCH_REFERENCE_SPACE_DISPLAY, cs->getReferenceSpaceType())) { throw Exception("Only version 2 (or higher) can have DisplayColorSpaces."); } + } + + if (hexVersion < 0x02050000) // Version 2.5 + { + if (*cs->getInteropID()) + { + std::ostringstream os; + os << "Config failed validation. The color space '" << cs->getName() << "' "; + os << "has non-empty InteropID and config version is less than 2.5."; + throw Exception(os.str().c_str()); + } + if (*cs->getAMFTransformIDs()) + { + std::ostringstream os; + os << "Config failed validation. The color space '" << cs->getName() << "' "; + os << "has non-empty AMFTransformIDs and config version is less than 2.5."; + throw Exception(os.str().c_str()); + } + if (*cs->getICCProfileName()) + { + std::ostringstream os; + os << "Config failed validation. The color space '" << cs->getName() << "' "; + os << "has non-empty ICCProfileName and config version is less than 2.5."; + throw Exception(os.str().c_str()); + } } } diff --git a/src/OpenColorIO/Context.cpp b/src/OpenColorIO/Context.cpp index 3a1294be58..b7e71ae6e7 100644 --- a/src/OpenColorIO/Context.cpp +++ b/src/OpenColorIO/Context.cpp @@ -253,7 +253,7 @@ void Context::setWorkingDir(const char * dirname) { AutoMutex lock(getImpl()->m_resultsCacheMutex); - getImpl()->m_workingDir = dirname; + getImpl()->m_workingDir = dirname ? dirname : ""; getImpl()->clearCaches(); } diff --git a/src/OpenColorIO/Look.cpp b/src/OpenColorIO/Look.cpp index c0e7cbef13..9830d0a460 100644 --- a/src/OpenColorIO/Look.cpp +++ b/src/OpenColorIO/Look.cpp @@ -86,7 +86,7 @@ const char * Look::getName() const void Look::setName(const char * name) { - getImpl()->m_name = name; + getImpl()->m_name = name ? name : ""; } const char * Look::getProcessSpace() const @@ -96,7 +96,7 @@ const char * Look::getProcessSpace() const void Look::setProcessSpace(const char * processSpace) { - getImpl()->m_processSpace = processSpace; + getImpl()->m_processSpace = processSpace ? processSpace : ""; } ConstTransformRcPtr Look::getTransform() const @@ -126,7 +126,7 @@ const char * Look::getDescription() const void Look::setDescription(const char * description) { - getImpl()->m_description = description; + getImpl()->m_description = description ? description : ""; } bool CollectContextVariables(const Config & config, diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index b1cee18982..bdf45d7109 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -3202,11 +3202,26 @@ inline void load(const YAML::Node& node, ColorSpaceRcPtr& cs, unsigned int major cs->addAlias(alias.c_str()); } } + else if (key == "interop_id") + { + load(iter->second, stringval); + cs->setInteropID(stringval.c_str()); + } else if(key == "description") { loadDescription(iter->second, stringval); cs->setDescription(stringval.c_str()); } + else if (key == "amf_transform_ids") + { + load(iter->second, stringval); + cs->setAMFTransformIDs(stringval.c_str()); + } + else if (key == "icc_profile_name") + { + load(iter->second, stringval); + cs->setICCProfileName(stringval.c_str()); + } else if(key == "family") { load(iter->second, stringval); @@ -3323,11 +3338,37 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major } out << YAML::Flow << YAML::Value << aliases; } + + const std::string interopID{ cs->getInteropID() }; + if (!interopID.empty()) + { + out << YAML::Key << "interop_id"; + out << YAML::Value << interopID; + } + out << YAML::Key << "family" << YAML::Value << cs->getFamily(); + out << YAML::Key << "equalitygroup" << YAML::Value << cs->getEqualityGroup(); + out << YAML::Key << "bitdepth" << YAML::Value; save(out, cs->getBitDepth()); + saveDescription(out, cs->getDescription()); + + const std::string amfTransformIDs{cs->getAMFTransformIDs()}; + if (!amfTransformIDs.empty()) + { + out << YAML::Key << "amf_transform_ids"; + out << YAML::Value << amfTransformIDs; + } + + const std::string iccProfileName{cs->getICCProfileName()}; + if (!iccProfileName.empty()) + { + out << YAML::Key << "icc_profile_name"; + out << YAML::Value << iccProfileName; + } + out << YAML::Key << "isdata" << YAML::Value << cs->isData(); if(cs->getNumCategories() > 0) @@ -4754,10 +4795,12 @@ inline void save(YAML::Emitter & out, const Config & config) { std::stringstream ss; const unsigned configMajorVersion = config.getMajorVersion(); + const unsigned configMinorVersion = config.getMinorVersion(); + ss << configMajorVersion; - if(config.getMinorVersion()!=0) + if(configMinorVersion != 0) { - ss << "." << config.getMinorVersion(); + ss << "." << configMinorVersion; } out << YAML::Block; diff --git a/src/OpenColorIO/Platform.cpp b/src/OpenColorIO/Platform.cpp index 1dbd846f35..0e7fc68b2d 100644 --- a/src/OpenColorIO/Platform.cpp +++ b/src/OpenColorIO/Platform.cpp @@ -154,6 +154,11 @@ bool isEnvPresent(const char * name) int Strcasecmp(const char * str1, const char * str2) { + if (!str1 || !str2) + { + throw Exception("String pointer for comparison must not be null."); + } + #ifdef _WIN32 return ::_stricmp(str1, str2); #else @@ -163,6 +168,11 @@ int Strcasecmp(const char * str1, const char * str2) int Strncasecmp(const char * str1, const char * str2, size_t n) { + if (!str1 || !str2) + { + throw Exception("String pointer for comparison must not be null."); + } + #ifdef _WIN32 return ::_strnicmp(str1, str2, n); #else diff --git a/src/OpenColorIO/ViewTransform.cpp b/src/OpenColorIO/ViewTransform.cpp index 0a7ca21091..7fd8914a34 100644 --- a/src/OpenColorIO/ViewTransform.cpp +++ b/src/OpenColorIO/ViewTransform.cpp @@ -91,7 +91,7 @@ const char * ViewTransform::getName() const noexcept void ViewTransform::setName(const char * name) noexcept { - getImpl()->m_name = name; + getImpl()->m_name = name ? name : ""; } const char * ViewTransform::getFamily() const noexcept @@ -101,7 +101,7 @@ const char * ViewTransform::getFamily() const noexcept void ViewTransform::setFamily(const char * family) { - getImpl()->m_family = family; + getImpl()->m_family = family ? family : ""; } const char * ViewTransform::getDescription() const noexcept @@ -111,7 +111,7 @@ const char * ViewTransform::getDescription() const noexcept void ViewTransform::setDescription(const char * description) { - getImpl()->m_description = description; + getImpl()->m_description = description ? description : ""; } bool ViewTransform::hasCategory(const char * category) const diff --git a/src/bindings/python/PyColorSpace.cpp b/src/bindings/python/PyColorSpace.cpp index 6827943bc0..b74c98c196 100644 --- a/src/bindings/python/PyColorSpace.cpp +++ b/src/bindings/python/PyColorSpace.cpp @@ -90,7 +90,10 @@ void bindPyColorSpace(py::module & m) const std::vector & allocationVars, const TransformRcPtr & toReference, const TransformRcPtr & fromReference, - const std::vector & categories) + const std::vector & categories, + const std::string & interopID, + const std::string& AMFTransformID, + const std::string& ICCProfileName) { ColorSpaceRcPtr p = ColorSpace::Create(referenceSpace); if (!aliases.empty()) @@ -102,11 +105,14 @@ void bindPyColorSpace(py::module & m) } } // Setting the name will remove alias named the same, so set name after. - if (!name.empty()) { p->setName(name.c_str()); } - if (!family.empty()) { p->setFamily(family.c_str()); } - if (!encoding.empty()) { p->setEncoding(encoding.c_str()); } - if (!equalityGroup.empty()) { p->setEqualityGroup(equalityGroup.c_str()); } - if (!description.empty()) { p->setDescription(description.c_str()); } + if (!name.empty()) { p->setName(name.c_str()); } + if (!family.empty()) { p->setFamily(family.c_str()); } + if (!encoding.empty()) { p->setEncoding(encoding.c_str()); } + if (!equalityGroup.empty()) { p->setEqualityGroup(equalityGroup.c_str()); } + if (!description.empty()) { p->setDescription(description.c_str()); } + if (!interopID.empty()) { p->setInteropID(interopID.c_str()); } + if (!AMFTransformID.empty()) { p->setAMFTransformIDs(AMFTransformID.c_str()); } + if (!ICCProfileName.empty()) { p->setICCProfileName(ICCProfileName.c_str()); } p->setBitDepth(bitDepth); p->setIsData(isData); p->setAllocation(allocation); @@ -150,6 +156,9 @@ void bindPyColorSpace(py::module & m) "toReference"_a = DEFAULT->getTransform(COLORSPACE_DIR_TO_REFERENCE), "fromReference"_a = DEFAULT->getTransform(COLORSPACE_DIR_FROM_REFERENCE), "categories"_a = getCategoriesStdVec(DEFAULT), + "interopID"_a = DEFAULT->getInteropID(), + "amfTransformIDs"_a = DEFAULT->getAMFTransformIDs(), + "iccProfileName"_a = DEFAULT->getICCProfileName(), DOC(ColorSpace, Create, 2)) .def("__deepcopy__", [](const ConstColorSpaceRcPtr & self, py::dict) @@ -193,6 +202,18 @@ void bindPyColorSpace(py::module & m) DOC(ColorSpace, getDescription)) .def("setDescription", &ColorSpace::setDescription, "description"_a, DOC(ColorSpace, setDescription)) + .def("getInteropID", &ColorSpace::getInteropID, + DOC(ColorSpace, getInteropID)) + .def("setInteropID", &ColorSpace::setInteropID, "interopID"_a, + DOC(ColorSpace, setInteropID)) + .def("getAMFTransformIDs", &ColorSpace::getAMFTransformIDs, + DOC(ColorSpace, getAMFTransformIDs)) + .def("setAMFTransformIDs", &ColorSpace::setAMFTransformIDs, "amfTransformIDs"_a, + DOC(ColorSpace, setAMFTransformIDs)) + .def("getICCProfileName", &ColorSpace::getICCProfileName, + DOC(ColorSpace, getICCProfileName)) + .def("setICCProfileName", &ColorSpace::setICCProfileName, "iccProfileName"_a, + DOC(ColorSpace, setICCProfileName)) .def("getBitDepth", &ColorSpace::getBitDepth, DOC(ColorSpace, getBitDepth)) .def("setBitDepth", &ColorSpace::setBitDepth, "bitDepth"_a, diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index 54521be171..4e85e2cf32 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -39,7 +39,39 @@ OCIO_ADD_TEST(ColorSpace, basic) OCIO_CHECK_ASSERT(!cs->isData()); OCIO_CHECK_EQUAL(OCIO::ALLOCATION_UNIFORM, cs->getAllocation()); OCIO_CHECK_EQUAL(0, cs->getAllocationNumVars()); - + + // Check the nullptr assignment hardening. + // First set the fields to non-empty values. + cs->setName("NAME"); + cs->setDescription("DESC"); + cs->setFamily("FAMILY"); + cs->setEqualityGroup("EQGRP"); + cs->setEncoding("ENC"); + cs->setAMFTransformIDs("AMF"); + cs->setInteropID("INTEROP"); + cs->setICCProfileName("ICC"); + + // Set to nullptr, this should erase the old values. + OCIO_CHECK_NO_THROW(cs->setName(nullptr)); + OCIO_CHECK_NO_THROW(cs->setDescription(nullptr)); + OCIO_CHECK_NO_THROW(cs->setFamily(nullptr)); + OCIO_CHECK_NO_THROW(cs->setEqualityGroup(nullptr)); + OCIO_CHECK_NO_THROW(cs->setEncoding(nullptr)); + OCIO_CHECK_NO_THROW(cs->setAMFTransformIDs(nullptr)); + OCIO_CHECK_NO_THROW(cs->setInteropID(nullptr)); + OCIO_CHECK_NO_THROW(cs->setICCProfileName(nullptr)); + + // Check that the values are empty now. + OCIO_CHECK_ASSERT(!*cs->getName()); + OCIO_CHECK_ASSERT(!*cs->getDescription()); + OCIO_CHECK_ASSERT(!*cs->getFamily()); + OCIO_CHECK_ASSERT(!*cs->getEqualityGroup()); + OCIO_CHECK_ASSERT(!*cs->getEncoding()); + OCIO_CHECK_ASSERT(!*cs->getAMFTransformIDs()); + OCIO_CHECK_ASSERT(!*cs->getInteropID()); + OCIO_CHECK_ASSERT(!*cs->getICCProfileName()); + + // Test set/get roundtrip. cs->setName("name"); OCIO_CHECK_EQUAL(std::string("name"), cs->getName()); cs->setFamily("family"); @@ -63,10 +95,17 @@ OCIO_ADD_TEST(ColorSpace, basic) cs->getAllocationVars(readVars); OCIO_CHECK_EQUAL(1.f, readVars[0]); OCIO_CHECK_EQUAL(2.f, readVars[1]); + cs->setInteropID("interop_id"); + OCIO_CHECK_EQUAL(std::string("interop_id"), cs->getInteropID()); + cs->setAMFTransformIDs("amf_transform_id1\namf_tranform_id2"); + OCIO_CHECK_EQUAL(std::string("amf_transform_id1\namf_tranform_id2"), + cs->getAMFTransformIDs()); + cs->setICCProfileName("icc_profile_name"); + OCIO_CHECK_EQUAL(std::string("icc_profile_name"), cs->getICCProfileName()); std::ostringstream oss; oss << *cs; - OCIO_CHECK_EQUAL(oss.str().size(), 193); + OCIO_CHECK_EQUAL(oss.str().size(), 305); } OCIO_ADD_TEST(ColorSpace, alias) @@ -619,6 +658,81 @@ active_views: [] OCIO_CHECK_EQUAL(cfgRes, os.str()); } + + // Test that the interop_id can't be used in v2.0 config. + { + constexpr char End[]{R"(colorspaces: + - ! + name: raw + interop_id: data + family: raw + equalitygroup: "" + bitdepth: 32f + description: Some text. + isdata: true + allocation: uniform +)"}; + std::string cfgString{Start}; + cfgString += End; + + std::istringstream is; + is.str(cfgString); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_THROW_WHAT( + config = OCIO::Config::CreateFromStream(is), + OCIO::Exception, + "Config failed validation. The color space 'raw' has non-empty InteropID and config version is less than 2.5."); + } + + // Test that the amf_transform_ids can't be used in v2.0 config. + { + constexpr char End[]{R"(colorspaces: + - ! + name: raw + amf_transform_ids: this:shouldnt:be:here + family: raw + equalitygroup: "" + bitdepth: 32f + description: Some text. + isdata: true + allocation: uniform +)"}; + std::string cfgString{Start}; + cfgString += End; + + std::istringstream is; + is.str(cfgString); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_THROW_WHAT( + config = OCIO::Config::CreateFromStream(is), OCIO::Exception, + "Config failed validation. The color space 'raw' has non-empty " + "AMFTransformIDs and config version is less than 2.5."); + } + + // Test that the icc_profile_name can't be used in v2.0 config. + { + constexpr char End[]{R"(colorspaces: + - ! + name: raw + icc_profile_name: not valid in v2.0 config + family: raw + equalitygroup: "" + bitdepth: 32f + description: Some text. + isdata: true + allocation: uniform +)"}; + std::string cfgString{Start}; + cfgString += End; + + std::istringstream is; + is.str(cfgString); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_THROW_WHAT( + config = OCIO::Config::CreateFromStream(is), OCIO::Exception, + "Config failed validation. The color space 'raw' has non-empty " + "ICCProfileName and config version is less than 2.5."); + } } OCIO_ADD_TEST(Config, use_alias) @@ -1702,3 +1816,250 @@ inactive_colorspaces: [scene-linear Rec.709-sRGB, ACES2065-1] ); } } + +OCIO_ADD_TEST(ColorSpace, interop_id) +{ + OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); + + // Test default value. + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); + + // Test setting and getting single profile name. + const char * interop_id = "srgb_p3d65_scene"; + cs->setInteropID(interop_id); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), interop_id); + + // Test setting empty string. + cs->setInteropID(""); + OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + + // Test setting and getting another value. + const char * anotherID= "lin_rec2020_scene"; + cs->setInteropID(anotherID); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), anotherID); + + // Test setting null pointer (should be safe). + OCIO_CHECK_NO_THROW(cs->setInteropID(nullptr)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); + + // Test copy constructor preserves ICC profile name. + cs->setInteropID(interop_id); + OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); + OCIO_CHECK_EQUAL(std::string(copy->getInteropID()), interop_id); +} + +OCIO_ADD_TEST(ColorSpace, interop_id_serialization) +{ + // Test YAML serialization and deserialization of InteropID. + auto cfg = OCIO::Config::Create(); + auto cs = OCIO::ColorSpace::Create(); + cs->setName("test_colorspace"); + + const std::string interop_id = "lin_rec709_scene"; + + cs->setInteropID(interop_id.c_str()); + cfg->addColorSpace(cs); + + // Serialize the Config. + std::stringstream ss; + cfg->serialize(ss); + std::string yamlStr = ss.str(); + + // Verify interop_id appears in YAML. + OCIO_CHECK_NE(yamlStr.find("interop_id"), std::string::npos); + OCIO_CHECK_NE(yamlStr.find(interop_id), std::string::npos); + + // Deserialize and verify. + std::istringstream iss(yamlStr); + OCIO::ConstConfigRcPtr deserializedCfg; + OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); + + // Verify interop_id is preserved. + OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); + OCIO_CHECK_EQUAL(std::string(deserializedCs->getInteropID()), interop_id); + + // verify that that earlier versions reject interop_id. + OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); + cfgCopy->setVersion(2, 4); + OCIO_CHECK_THROW_WHAT( + cfgCopy->serialize(ss), + OCIO::Exception, + "Config failed validation. The color space 'test_colorspace' has non-empty " + "InteropID and config version is less than 2.5."); + + // Test with empty interop_id (should not appear in YAML). + cs->setInteropID(nullptr); + cfg->addColorSpace(cs); // Replace the existing CS. + ss.str(""); + cfg->serialize(ss); + std::string yamlStr2 = ss.str(); + + // Verify empty imterop_id does not appear in YAML. + OCIO_CHECK_EQUAL(yamlStr2.find("interop_id"), std::string::npos); +} + +OCIO_ADD_TEST(ColorSpace, amf_transform_ids) +{ + OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); + + // Test default value. + OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), ""); + + // Test setting and getting single ID. + const char * singleID = "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3"; + cs->setAMFTransformIDs(singleID); + OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), singleID); + + // Test setting to empty string. + cs->setAMFTransformIDs(""); + OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), ""); + + // Test setting and getting multiple IDs. + const char * multipleIDs = + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; + cs->setAMFTransformIDs(multipleIDs); + OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), multipleIDs); + + // Test setting to nullptr. + cs->setDescription(nullptr); + cs->setAMFTransformIDs(nullptr); + OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), ""); + + // Test copy constructor preserves AMF transform IDs. + cs->setAMFTransformIDs(singleID); + OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); + OCIO_CHECK_EQUAL(std::string(copy->getAMFTransformIDs()), singleID); +} + +OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) +{ + // Test YAML serialization and deserialization of AmfTransformIDs. + auto cfg = OCIO::Config::Create(); + auto cs = OCIO::ColorSpace::Create(); + cs->setName("test_colorspace"); + + const std::string amfIDs = + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; + + cs->setAMFTransformIDs(amfIDs.c_str()); + cfg->addColorSpace(cs); + + // Serialize the Config. + std::stringstream ss; + cfg->serialize(ss); + std::string yamlStr = ss.str(); + + // Verify AmfTransformIDs appears in YAML. + OCIO_CHECK_NE(yamlStr.find("amf_transform_ids"), std::string::npos); + OCIO_CHECK_NE(yamlStr.find("ACEScsc.Academy.ACEScc_to_ACES"), std::string::npos); + + // Deserialize and verify. + std::istringstream iss(yamlStr); + OCIO::ConstConfigRcPtr deserializedCfg; + OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); + + // Verify AmfTransformIDs is preserved. + OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); + OCIO_CHECK_EQUAL(std::string(deserializedCs->getAMFTransformIDs()), amfIDs); + + // Verify that that earlier versions reject amf_transform_ids. + OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); + cfgCopy->setVersion(2,4); + OCIO_CHECK_THROW_WHAT(cfgCopy->serialize(ss), + OCIO::Exception, + "Config failed validation. The color space 'test_colorspace' has non-empty " + "AMFTransformIDs and config version is less than 2.5."); + + // Test with empty AmfTransformIDs (should not appear in YAML). + cs->setAMFTransformIDs(nullptr); + cfg->addColorSpace(cs); // Replace the existing CS. + ss.str(""); + cfg->serialize(ss); + std::string yamlStr2 = ss.str(); + + // Verify empty AmfTransformIDs does not appear in YAML. + OCIO_CHECK_EQUAL(yamlStr2.find("amf_transform_ids"), std::string::npos); +} + +OCIO_ADD_TEST(ColorSpace, icc_profile_name) +{ + OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); + + // Test default value. + OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + + // Test setting and getting single profile name. + const char * profileName = "sRGB IEC61966-2.1"; + cs->setICCProfileName(profileName); + OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), profileName); + + // Test setting and getting another profile name. + const char * anotherProfile = "Adobe RGB (1998)"; + cs->setICCProfileName(anotherProfile); + OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), anotherProfile); + + // Test setting empty string. + cs->setICCProfileName(""); + OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + + // Test setting null pointer (should be safe). + OCIO_CHECK_NO_THROW(cs->setICCProfileName(nullptr)); + OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + + // Test copy constructor preserves ICC profile name. + cs->setICCProfileName(profileName); + OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); + OCIO_CHECK_EQUAL(std::string(copy->getICCProfileName()), profileName); +} + +OCIO_ADD_TEST(ColorSpace, icc_profile_name_serialization) +{ + // Test YAML serialization and deserialization of IccProfileName. + auto cfg = OCIO::Config::Create(); + auto cs = OCIO::ColorSpace::Create(); + cs->setName("test_colorspace"); + + const std::string profileName = "sRGB IEC61966-2.1"; + + cs->setICCProfileName(profileName.c_str()); + cfg->addColorSpace(cs); + + // Serialize the Config. + std::stringstream ss; + cfg->serialize(ss); + std::string yamlStr = ss.str(); + + // Verify IccProfileName appears in YAML. + OCIO_CHECK_NE(yamlStr.find("icc_profile_name"), std::string::npos); + OCIO_CHECK_NE(yamlStr.find(profileName), std::string::npos); + + // Deserialize and verify. + std::istringstream iss(yamlStr); + OCIO::ConstConfigRcPtr deserializedCfg; + OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); + + // Verify IccProfileName is preserved. + OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); + OCIO_CHECK_EQUAL(std::string(deserializedCs->getICCProfileName()), profileName); + + // verify that that earlier versions reject amf_transform_ids. + OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); + cfgCopy->setVersion(2, 4); + OCIO_CHECK_THROW_WHAT( + cfgCopy->serialize(ss), + OCIO::Exception, + "Config failed validation. The color space 'test_colorspace' has non-empty " + "ICCProfileName and config version is less than 2.5."); + + // Test with empty IccProfileName (should not appear in YAML). + cs->setICCProfileName(nullptr); + cfg->addColorSpace(cs); // replace the existing CS + ss.str(""); + cfg->serialize(ss); + std::string yamlStr2 = ss.str(); + + // Verify empty IccProfileName does not appear in YAML. + OCIO_CHECK_EQUAL(yamlStr2.find("icc_profile_name"), std::string::npos); +} \ No newline at end of file diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp index 6a310aae04..144d1a5816 100644 --- a/tests/cpu/Config_tests.cpp +++ b/tests/cpu/Config_tests.cpp @@ -2093,12 +2093,12 @@ OCIO_ADD_TEST(Config, version) { OCIO_CHECK_THROW_WHAT(config->setVersion(2, 9), OCIO::Exception, "The minor version 9 is not supported for major version 2. " - "Maximum minor version is 4"); + "Maximum minor version is 5"); OCIO_CHECK_NO_THROW(config->setMajorVersion(2)); OCIO_CHECK_THROW_WHAT(config->setMinorVersion(9), OCIO::Exception, "The minor version 9 is not supported for major version 2. " - "Maximum minor version is 4"); + "Maximum minor version is 5"); } { diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index 0860e7a3f0..f8f141194c 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -42,6 +42,9 @@ def test_copy(self): self.colorspace.setTransform(direction=OCIO.COLORSPACE_DIR_FROM_REFERENCE, transform=mat) self.colorspace.addAlias('alias') self.colorspace.addCategory('cat') + self.colorspace.setInteropID('ACEScg') + self.colorspace.setAMFTransformIDs('urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3') + self.colorspace.setICCProfileName('sRGB IEC61966-2.1') other = copy.deepcopy(self.colorspace) self.assertFalse(other is self.colorspace) @@ -59,6 +62,9 @@ def test_copy(self): self.assertTrue(other.getTransform(OCIO.COLORSPACE_DIR_FROM_REFERENCE).equals(self.colorspace.getTransform(OCIO.COLORSPACE_DIR_FROM_REFERENCE))) self.assertEqual(list(other.getAliases()), list(self.colorspace.getAliases())) self.assertEqual(list(other.getCategories()), list(self.colorspace.getCategories())) + self.assertEqual(other.getInteropID(), self.colorspace.getInteropID()) + self.assertEqual(other.getAMFTransformIDs(), self.colorspace.getAMFTransformIDs()) + self.assertEqual(other.getICCProfileName(), self.colorspace.getICCProfileName()) def test_allocation(self): """ @@ -279,6 +285,9 @@ def test_constructor_without_parameter(self): self.assertFalse(cs.isData()) self.assertEqual(cs.getAllocation(), OCIO.ALLOCATION_UNIFORM) self.assertEqual(cs.getAllocationVars(), []) + self.assertEqual(cs.getInteropID(), '') + self.assertEqual(cs.getAMFTransformIDs(), '') + self.assertEqual(cs.getICCProfileName(), '') def test_data(self): """ @@ -632,9 +641,138 @@ def test_display_referred(self, cfg, cs_name, expected_value): test_display_referred(self, cfg, "scene_linear-trans-alias", False) test_display_referred(self, cfg, "scene_ref", False) + def test_interop_id(self): + """ + Test the setInteropID() and getInteropID() methods. + """ + + # Test default value (should be empty). + self.assertEqual(self.colorspace.getInteropID(), '') + + # Test setting and getting a simple interop ID. + test_id = 'lin_ap0_scene' + self.colorspace.setInteropID(test_id) + self.assertEqual(self.colorspace.getInteropID(), test_id) + + # Test setting and getting a different interop ID. + test_id2 = 'srgb_ap1_scene' + self.colorspace.setInteropID(test_id2) + self.assertEqual(self.colorspace.getInteropID(), test_id2) + + # Test setting empty string. + self.colorspace.setInteropID('') + self.assertEqual(self.colorspace.getInteropID(), '') + + # Test setting None (should convert to empty string). + self.colorspace.setInteropID('something') + self.colorspace.setInteropID(None) + self.assertEqual(self.colorspace.getInteropID(), '') + + # Test wrong type (should raise TypeError). + with self.assertRaises(TypeError): + self.colorspace.setInteropID(123) + + with self.assertRaises(TypeError): + self.colorspace.setInteropID(['list']) + + def test_amf_transform_ids(self): + """ + Test the setAMFTransformIDs() and getAMFTransformIDs() methods. + """ + + # Test default value (should be empty). + self.assertEqual(self.colorspace.getAMFTransformIDs(), '') + + # Test setting and getting a single transform ID. + single_id = 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3' + self.colorspace.setAMFTransformIDs(single_id) + self.assertEqual(self.colorspace.getAMFTransformIDs(), single_id) + + # Test setting and getting multiple transform IDs (newline-separated). + multiple_ids = ('urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3\n' + 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_CG.a1.0.3\n' + 'urn:ampas:aces:transformId:v1.5:RRT.a1.0.3') + self.colorspace.setAMFTransformIDs(multiple_ids) + self.assertEqual(self.colorspace.getAMFTransformIDs(), multiple_ids) + + # Test setting empty string. + self.colorspace.setAMFTransformIDs('') + self.assertEqual(self.colorspace.getAMFTransformIDs(), '') + + # Test setting None (should convert to empty string). + self.colorspace.setAMFTransformIDs('something') + self.colorspace.setAMFTransformIDs(None) + self.assertEqual(self.colorspace.getAMFTransformIDs(), '') + + # Test with different line endings. + mixed_endings = 'id1\nid2\rid3\r\nid4' + self.colorspace.setAMFTransformIDs(mixed_endings) + self.assertEqual(self.colorspace.getAMFTransformIDs(), mixed_endings) + + # Test with leading/trailing whitespace. + whitespace_ids = ' \n id1 \n id2 \n ' + self.colorspace.setAMFTransformIDs(whitespace_ids) + self.assertEqual(self.colorspace.getAMFTransformIDs(), whitespace_ids) + + # Test wrong type (should raise TypeError). + with self.assertRaises(TypeError): + self.colorspace.setAMFTransformIDs(123) + + with self.assertRaises(TypeError): + self.colorspace.setAMFTransformIDs(['list', 'of', 'ids']) + + def test_icc_profile_name(self): + """ + Test the setICCProfileName() and getICCProfileName() methods. + """ + + # Test default value (should be empty). + self.assertEqual(self.colorspace.getICCProfileName(), '') + + # Test setting and getting a simple profile name. + profile_name = 'sRGB IEC61966-2.1' + self.colorspace.setICCProfileName(profile_name) + self.assertEqual(self.colorspace.getICCProfileName(), profile_name) + + # Test setting and getting a different profile name. + profile_name2 = 'Adobe RGB (1998)' + self.colorspace.setICCProfileName(profile_name2) + self.assertEqual(self.colorspace.getICCProfileName(), profile_name2) + + # Test with a more complex profile name. + complex_name = 'Display P3 - Apple Cinema Display (Calibrated 2023-01-15)' + self.colorspace.setICCProfileName(complex_name) + self.assertEqual(self.colorspace.getICCProfileName(), complex_name) + + # Test setting empty string. + self.colorspace.setICCProfileName('') + self.assertEqual(self.colorspace.getICCProfileName(), '') + + # Test setting None (should convert to empty string). + self.colorspace.setICCProfileName('something') + self.colorspace.setICCProfileName(None) + self.assertEqual(self.colorspace.getICCProfileName(), '') + + # Test with special characters and numbers. + special_name = 'ProPhoto RGB v2.0 (γ=1.8) [Custom Profile #123]' + self.colorspace.setICCProfileName(special_name) + self.assertEqual(self.colorspace.getICCProfileName(), special_name) + + # Test with Unicode characters. + unicode_name = 'Профиль RGB γ=2.2' + self.colorspace.setICCProfileName(unicode_name) + self.assertEqual(self.colorspace.getICCProfileName(), unicode_name) + + # Test wrong type (should raise TypeError). + with self.assertRaises(TypeError): + self.colorspace.setICCProfileName(123) + + with self.assertRaises(TypeError): + self.colorspace.setICCProfileName(['profile', 'name']) + def test_processor_to_known_colorspace(self): - CONFIG = """ocio_profile_version: 2 + CONFIG = """ocio_profile_version: 2.5 roles: default: raw @@ -690,12 +828,14 @@ def test_processor_to_known_colorspace(self): - ! name: ACES cg description: An ACEScg space with an unusual spelling. + interop_id: lin_ap1_scene isdata: false to_scene_reference: ! {style: ACEScg_to_ACES2065-1} - ! name: Linear ITU-R BT.709 description: A linear Rec.709 space with an unusual spelling. + interop_id: lin_rec709_scene isdata: false from_scene_reference: ! name: AP0 to Linear Rec.709 (sRGB) From a0ba230a0efeb5d83d13f7dda02921f8ef592e1e Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Mon, 9 Jun 2025 20:12:09 -0700 Subject: [PATCH 04/15] As discussed in the TSC meeting, adding a namespace field separator logic to the interop_id field. - only zero or one ":" is allowed in the interop_id name - ":" can not be the last character - added cpp and python unit tests. Signed-off-by: cuneyt.ozdas --- src/OpenColorIO/ColorSpace.cpp | 38 +++++++++++++++++- tests/cpu/ColorSpace_tests.cpp | 71 ++++++++++++++++++++++++++++++++-- tests/python/ColorSpaceTest.py | 67 +++++++++++++++++++++++++++++++- 3 files changed, 170 insertions(+), 6 deletions(-) diff --git a/src/OpenColorIO/ColorSpace.cpp b/src/OpenColorIO/ColorSpace.cpp index 7a57c0779f..b6115dea47 100644 --- a/src/OpenColorIO/ColorSpace.cpp +++ b/src/OpenColorIO/ColorSpace.cpp @@ -231,7 +231,43 @@ const char * ColorSpace::getInteropID() const noexcept void ColorSpace::setInteropID(const char * interopID) { - getImpl()->m_interopID = interopID ? interopID : ""; + std::string id = interopID ? interopID : ""; + + if (!id.empty()) + { + // Count the number of ':' characters in the string + // We need to handle UTF-8 properly, so we iterate byte by byte + // but only count ASCII ':' characters (0x3A) + size_t colonCount = 0; + size_t lastColonPos = std::string::npos; + + for (size_t i = 0; i < id.length(); ++i) + { + if (id[i] == ':') + { + colonCount++; + lastColonPos = i; + } + } + + // Validate: only zero or one ':' character allowed + if (colonCount > 1) + { + std::ostringstream oss; + oss << "InteropID '" << id << "' is invalid: only zero or one ':' character is allowed."; + throw Exception(oss.str().c_str()); + } + + // Validate: ':' cannot be the last character + if (colonCount == 1 && lastColonPos == id.length() - 1) + { + std::ostringstream oss; + oss << "InteropID '" << id << "' is invalid: ':' character cannot be the last character."; + throw Exception(oss.str().c_str()); + } + } + + getImpl()->m_interopID = id; } const char * ColorSpace::getAMFTransformIDs() const noexcept diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index 4e85e2cf32..125319364d 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -1831,7 +1831,7 @@ OCIO_ADD_TEST(ColorSpace, interop_id) // Test setting empty string. cs->setInteropID(""); - OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); // Test setting and getting another value. const char * anotherID= "lin_rec2020_scene"; @@ -1839,13 +1839,75 @@ OCIO_ADD_TEST(ColorSpace, interop_id) OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), anotherID); // Test setting null pointer (should be safe). + cs->setInteropID("something"); OCIO_CHECK_NO_THROW(cs->setInteropID(nullptr)); OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); - // Test copy constructor preserves ICC profile name. + // Test copy constructor preserves InteropID. cs->setInteropID(interop_id); OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); OCIO_CHECK_EQUAL(std::string(copy->getInteropID()), interop_id); + + // Test valid InteropID with colon in the middle. + const char * validColonMiddle = "namespace:colorspace_name"; + OCIO_CHECK_NO_THROW(cs->setInteropID(validColonMiddle)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), validColonMiddle); + + // Test invalid InteropID with multiple colons. + const char * invalidMultipleColons = "name:space:cs_name"; + OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidMultipleColons), + OCIO::Exception, + "InteropID 'name:space:cs_name' is invalid: only zero or one ':' character is allowed."); + + // Test invalid InteropID with colon at the end. + const char * invalidColonAtEnd = "namespace:"; + OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidColonAtEnd), + OCIO::Exception, + "InteropID 'namespace:' is invalid: ':' character cannot be the last character."); + + // Test invalid InteropID with three colons. + const char * invalidThreeColons = "a:b:c:d"; + OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidThreeColons), + OCIO::Exception, + "InteropID 'a:b:c:d' is invalid: only zero or one ':' character is allowed."); + + // Test UTF-8 strings with valid single colon. + const char * utf8ValidColon = "標準:萬國碼"; + OCIO_CHECK_NO_THROW(cs->setInteropID(utf8ValidColon)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), utf8ValidColon); + + // Test UTF-8 strings with invalid multiple colons. + const char * utf8InvalidMultipleColons = "標準:萬國:碼"; + OCIO_CHECK_THROW_WHAT(cs->setInteropID(utf8InvalidMultipleColons), + OCIO::Exception, + "only zero or one ':' character is allowed."); + + // Test UTF-8 strings with invalid colon at end. + const char * utf8InvalidColonAtEnd = "標準萬國碼:"; + OCIO_CHECK_THROW_WHAT(cs->setInteropID(utf8InvalidColonAtEnd), + OCIO::Exception, + "':' character cannot be the last character."); + + // Test UTF-8 strings without colon (should be valid). + const char * utf8NoColon = "標準萬國碼"; + OCIO_CHECK_NO_THROW(cs->setInteropID(utf8NoColon)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), utf8NoColon); + + // Test edge case: single colon character. + const char * singleColon = ":"; + OCIO_CHECK_THROW_WHAT(cs->setInteropID(singleColon), + OCIO::Exception, + "':' character cannot be the last character."); + + // Test edge case: colon at beginning (should be valid). + const char * colonAtBeginning = ":valid_name"; + OCIO_CHECK_NO_THROW(cs->setInteropID(colonAtBeginning)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), colonAtBeginning); + + // Test valid InteropID with one colon (not at the end). + const char * validWithColon = "namespace:cs_name"; + OCIO_CHECK_NO_THROW(cs->setInteropID(validWithColon)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), validWithColon); } OCIO_ADD_TEST(ColorSpace, interop_id_serialization) @@ -1921,8 +1983,8 @@ OCIO_ADD_TEST(ColorSpace, amf_transform_ids) cs->setAMFTransformIDs(multipleIDs); OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), multipleIDs); - // Test setting to nullptr. - cs->setDescription(nullptr); + // Test setting to null pointer (should be safe). + cs->setAMFTransformIDs("something"); cs->setAMFTransformIDs(nullptr); OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), ""); @@ -2005,6 +2067,7 @@ OCIO_ADD_TEST(ColorSpace, icc_profile_name) OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); // Test setting null pointer (should be safe). + cs->setICCProfileName("something"); OCIO_CHECK_NO_THROW(cs->setICCProfileName(nullptr)); OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index f8f141194c..e79b4d63b5 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -675,6 +675,71 @@ def test_interop_id(self): with self.assertRaises(TypeError): self.colorspace.setInteropID(['list']) + # Test valid InteropID with one colon (not at the end). + valid_with_colon = 'namespace:cs_name' + self.colorspace.setInteropID(valid_with_colon) + self.assertEqual(self.colorspace.getInteropID(), valid_with_colon) + + # Test valid InteropID with colon in the middle. + valid_colon_middle = 'namespace:colorspace_name' + self.colorspace.setInteropID(valid_colon_middle) + self.assertEqual(self.colorspace.getInteropID(), valid_colon_middle) + + # Test invalid InteropID with multiple colons. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('name:space:cs_name') + self.assertIn("only zero or one ':' character is allowed", str(context.exception)) + + # Test invalid InteropID with colon at the end. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('lin_ap1_scene:') + self.assertIn("':' character cannot be the last character", str(context.exception)) + + # Test invalid InteropID with three colons. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('a:b:c:d') + self.assertIn("only zero or one ':' character is allowed", str(context.exception)) + + # Test UTF-8 strings with valid single colon. + utf8_valid_colon = '標準:萬國碼' + self.colorspace.setInteropID(utf8_valid_colon) + self.assertEqual(self.colorspace.getInteropID(), utf8_valid_colon) + + # Test UTF-8 strings with invalid multiple colons. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('標準:萬國:碼') + self.assertIn("only zero or one ':' character is allowed", str(context.exception)) + + # Test UTF-8 strings with invalid colon at end. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('標準萬國碼:') + self.assertIn("':' character cannot be the last character", str(context.exception)) + + # Test UTF-8 strings without colon (should be valid). + utf8_no_colon = '標準萬國碼' + self.colorspace.setInteropID(utf8_no_colon) + self.assertEqual(self.colorspace.getInteropID(), utf8_no_colon) + + # Test edge case: single colon character. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID(':') + self.assertIn("':' character cannot be the last character", str(context.exception)) + + # Test edge case: colon at beginning (should be valid). + colon_at_beginning = ':valid_name' + self.colorspace.setInteropID(colon_at_beginning) + self.assertEqual(self.colorspace.getInteropID(), colon_at_beginning) + + # Test with Unicode characters that might look like colons but aren't ASCII colons. + unicode_similar = 'test：name' # This uses a full-width colon (U+FF1A), not ASCII colon + self.colorspace.setInteropID(unicode_similar) + self.assertEqual(self.colorspace.getInteropID(), unicode_similar) + + # Test mixed ASCII and Unicode with valid single ASCII colon. + mixed_valid = 'ñamespace:色空間' + self.colorspace.setInteropID(mixed_valid) + self.assertEqual(self.colorspace.getInteropID(), mixed_valid) + def test_amf_transform_ids(self): """ Test the setAMFTransformIDs() and getAMFTransformIDs() methods. @@ -835,7 +900,7 @@ def test_processor_to_known_colorspace(self): - ! name: Linear ITU-R BT.709 description: A linear Rec.709 space with an unusual spelling. - interop_id: lin_rec709_scene + interop_id: "mycompany:lin_rec709_scene" isdata: false from_scene_reference: ! name: AP0 to Linear Rec.709 (sRGB) From 6800e86f7d9b61837fb412be4b1954d82e699631 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Mon, 9 Jun 2025 21:14:21 -0700 Subject: [PATCH 05/15] Adding interop_id verification in ociocheck. Non-namespaced IDs are checked against the known CIF values and warning is issued if not found. Signed-off-by: cuneyt.ozdas --- src/apps/ociocheck/main.cpp | 42 +++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/apps/ociocheck/main.cpp b/src/apps/ociocheck/main.cpp index b3c8c33854..f3e9ff1430 100644 --- a/src/apps/ociocheck/main.cpp +++ b/src/apps/ociocheck/main.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include namespace OCIO = OCIO_NAMESPACE; @@ -287,6 +288,26 @@ int main(int argc, const char **argv) std::cout << std::endl; std::cout << "** ColorSpaces **" << std::endl; + // Valid Color Interop Forum IDs + std::set validInteropIDs = { + "lin_ap1_scene", + "lin_ap0_scene", + "lin_rec709_scene", + "lin_p3d65_scene", + "lin_rec2020_scene", + "lin_adobergb_scene", + "lin_ciexyzd65_scene", + "srgb_rec709_scene", + "g22_rec709_scene", + "g18_rec709_scene", + "srgb_ap1_scene", + "g22_ap1_scene", + "srgb_p3d65_scene", + "g22_adobergb_scene", + "data", + "unknown" + }; + const int numCS = config->getNumColorSpaces( OCIO::SEARCH_REFERENCE_SPACE_ALL, // Iterate over scene & display color spaces. OCIO::COLORSPACE_ALL); // Iterate over active & inactive color spaces. @@ -298,6 +319,27 @@ int main(int argc, const char **argv) OCIO::COLORSPACE_ALL, i)); + // Validate InteropID if present + std::string interopID = cs->getInteropID(); + if (!interopID.empty()) + { + // Check if the interopID contains a colon + if (interopID.find(':') == std::string::npos) + { + // No colon found, check if it's a valid Color Interop Forum ID (case insensitive) + std::string lowerInteropID = interopID; + std::transform(lowerInteropID.begin(), lowerInteropID.end(), lowerInteropID.begin(), ::tolower); + + if (validInteropIDs.find(lowerInteropID) == validInteropIDs.end()) + { + std::cout << "WARNING: Color space '" << cs->getName() + << "' has unknown interop_id '" << interopID + << "'. Expected one of the Color Interop Forum standard IDs or a namespaced ID with ':'." << std::endl; + } + } + // If it contains a colon, it's assumed to be a namespaced ID and is valid + } + // Try to load the transform for the to_ref direction -- this will load any LUTs. bool toRefOK = true; std::string toRefErrorText; From 2e9d541a94976f1bf10d281984fec2d0ba036148 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Mon, 9 Jun 2025 21:23:44 -0700 Subject: [PATCH 06/15] minor touches. Signed-off-by: cuneyt.ozdas --- src/apps/ociocheck/main.cpp | 4 ++++ tests/cpu/ColorSpace_tests.cpp | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/apps/ociocheck/main.cpp b/src/apps/ociocheck/main.cpp index f3e9ff1430..6db3b0cf54 100644 --- a/src/apps/ociocheck/main.cpp +++ b/src/apps/ociocheck/main.cpp @@ -320,6 +320,10 @@ int main(int argc, const char **argv) i)); // Validate InteropID if present + // TODO: When the CIF list for display color spaces is ready consider + // splitting this validation into two parts so that display color spaces + // are tested against the display CIF list. + std::string interopID = cs->getInteropID(); if (!interopID.empty()) { diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index 125319364d..f5bf00b8da 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -97,8 +97,8 @@ OCIO_ADD_TEST(ColorSpace, basic) OCIO_CHECK_EQUAL(2.f, readVars[1]); cs->setInteropID("interop_id"); OCIO_CHECK_EQUAL(std::string("interop_id"), cs->getInteropID()); - cs->setAMFTransformIDs("amf_transform_id1\namf_tranform_id2"); - OCIO_CHECK_EQUAL(std::string("amf_transform_id1\namf_tranform_id2"), + cs->setAMFTransformIDs("amf_transform_id1\namf_transform_id2"); + OCIO_CHECK_EQUAL(std::string("amf_transform_id1\namf_transform_id2"), cs->getAMFTransformIDs()); cs->setICCProfileName("icc_profile_name"); OCIO_CHECK_EQUAL(std::string("icc_profile_name"), cs->getICCProfileName()); From 27a54870857006d8b879adbb6c146efbd172a130 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Mon, 9 Jun 2025 21:26:29 -0700 Subject: [PATCH 07/15] fix the failing test. Signed-off-by: cuneyt.ozdas --- src/OpenColorIO/ColorSpace.cpp | 5 +++-- tests/cpu/ColorSpace_tests.cpp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/OpenColorIO/ColorSpace.cpp b/src/OpenColorIO/ColorSpace.cpp index b6115dea47..a65b719d3f 100644 --- a/src/OpenColorIO/ColorSpace.cpp +++ b/src/OpenColorIO/ColorSpace.cpp @@ -236,8 +236,7 @@ void ColorSpace::setInteropID(const char * interopID) if (!id.empty()) { // Count the number of ':' characters in the string - // We need to handle UTF-8 properly, so we iterate byte by byte - // but only count ASCII ':' characters (0x3A) + // This is UTF-8 safe. size_t colonCount = 0; size_t lastColonPos = std::string::npos; @@ -265,6 +264,8 @@ void ColorSpace::setInteropID(const char * interopID) oss << "InteropID '" << id << "' is invalid: ':' character cannot be the last character."; throw Exception(oss.str().c_str()); } + + // TODO: If no namespace is given, verify the interopID against CIF list and warn if not found. } getImpl()->m_interopID = id; diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index f5bf00b8da..c9fb9522db 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -105,7 +105,7 @@ OCIO_ADD_TEST(ColorSpace, basic) std::ostringstream oss; oss << *cs; - OCIO_CHECK_EQUAL(oss.str().size(), 305); + OCIO_CHECK_EQUAL(oss.str().size(), 306); } OCIO_ADD_TEST(ColorSpace, alias) From 07237efce13beb1cbe748b04d18ba4275d058c24 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Mon, 9 Jun 2025 22:53:43 -0700 Subject: [PATCH 08/15] Disallow non-ASCII characters (outside of [0..127] range) in the interopID field. Signed-off-by: cuneyt.ozdas --- src/OpenColorIO/ColorSpace.cpp | 10 ++++-- tests/cpu/ColorSpace_tests.cpp | 63 +++++++--------------------------- tests/python/ColorSpaceTest.py | 55 ++++------------------------- 3 files changed, 28 insertions(+), 100 deletions(-) diff --git a/src/OpenColorIO/ColorSpace.cpp b/src/OpenColorIO/ColorSpace.cpp index a65b719d3f..229fafd6bf 100644 --- a/src/OpenColorIO/ColorSpace.cpp +++ b/src/OpenColorIO/ColorSpace.cpp @@ -236,7 +236,7 @@ void ColorSpace::setInteropID(const char * interopID) if (!id.empty()) { // Count the number of ':' characters in the string - // This is UTF-8 safe. + // and check for non-ASCII characters size_t colonCount = 0; size_t lastColonPos = std::string::npos; @@ -247,6 +247,12 @@ void ColorSpace::setInteropID(const char * interopID) colonCount++; lastColonPos = i; } + else if (static_cast(id[i]) >= 0x80) + { + std::ostringstream oss; + oss << "InteropID '" << id << "' is invalid: only ASCII characters [0x00..0x7F] are allowed."; + throw Exception(oss.str().c_str()); + } } // Validate: only zero or one ':' character allowed @@ -265,7 +271,7 @@ void ColorSpace::setInteropID(const char * interopID) throw Exception(oss.str().c_str()); } - // TODO: If no namespace is given, verify the interopID against CIF list and warn if not found. + // TODO: Do we want to verify the interopID against the CIF list here? } getImpl()->m_interopID = id; diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index c9fb9522db..58ebe87d9a 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -1855,59 +1855,22 @@ OCIO_ADD_TEST(ColorSpace, interop_id) // Test invalid InteropID with multiple colons. const char * invalidMultipleColons = "name:space:cs_name"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidMultipleColons), - OCIO::Exception, - "InteropID 'name:space:cs_name' is invalid: only zero or one ':' character is allowed."); + OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidMultipleColons), OCIO::Exception, + "only zero or one ':' character is allowed"); // Test invalid InteropID with colon at the end. const char * invalidColonAtEnd = "namespace:"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidColonAtEnd), - OCIO::Exception, - "InteropID 'namespace:' is invalid: ':' character cannot be the last character."); - - // Test invalid InteropID with three colons. - const char * invalidThreeColons = "a:b:c:d"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidThreeColons), - OCIO::Exception, - "InteropID 'a:b:c:d' is invalid: only zero or one ':' character is allowed."); - - // Test UTF-8 strings with valid single colon. - const char * utf8ValidColon = "標準:萬國碼"; - OCIO_CHECK_NO_THROW(cs->setInteropID(utf8ValidColon)); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), utf8ValidColon); - - // Test UTF-8 strings with invalid multiple colons. - const char * utf8InvalidMultipleColons = "標準:萬國:碼"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(utf8InvalidMultipleColons), - OCIO::Exception, - "only zero or one ':' character is allowed."); - - // Test UTF-8 strings with invalid colon at end. - const char * utf8InvalidColonAtEnd = "標準萬國碼:"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(utf8InvalidColonAtEnd), - OCIO::Exception, - "':' character cannot be the last character."); - - // Test UTF-8 strings without colon (should be valid). - const char * utf8NoColon = "標準萬國碼"; - OCIO_CHECK_NO_THROW(cs->setInteropID(utf8NoColon)); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), utf8NoColon); - - // Test edge case: single colon character. - const char * singleColon = ":"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(singleColon), - OCIO::Exception, - "':' character cannot be the last character."); - - // Test edge case: colon at beginning (should be valid). - const char * colonAtBeginning = ":valid_name"; - OCIO_CHECK_NO_THROW(cs->setInteropID(colonAtBeginning)); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), colonAtBeginning); - - // Test valid InteropID with one colon (not at the end). - const char * validWithColon = "namespace:cs_name"; - OCIO_CHECK_NO_THROW(cs->setInteropID(validWithColon)); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), validWithColon); + OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidColonAtEnd), OCIO::Exception, + "':' character cannot be the last character"); + + // Test invalid InteropID with non-ASCII characters. + const char * invalidNonASCII1 = "café_scene"; // Contains é (0xC3 0xA9) + OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidNonASCII1), OCIO::Exception, + "contains non-ASCII characters"); + + const char * invalidNonASCII2 = "space±_name"; // Contains ± (ANSI 0xB1) + OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidNonASCII2), OCIO::Exception, + "contains non-ASCII characters"); } OCIO_ADD_TEST(ColorSpace, interop_id_serialization) diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index e79b4d63b5..999de8bceb 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -680,11 +680,6 @@ def test_interop_id(self): self.colorspace.setInteropID(valid_with_colon) self.assertEqual(self.colorspace.getInteropID(), valid_with_colon) - # Test valid InteropID with colon in the middle. - valid_colon_middle = 'namespace:colorspace_name' - self.colorspace.setInteropID(valid_colon_middle) - self.assertEqual(self.colorspace.getInteropID(), valid_colon_middle) - # Test invalid InteropID with multiple colons. with self.assertRaises(Exception) as context: self.colorspace.setInteropID('name:space:cs_name') @@ -692,53 +687,17 @@ def test_interop_id(self): # Test invalid InteropID with colon at the end. with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('lin_ap1_scene:') + self.colorspace.setInteropID('namespace:') self.assertIn("':' character cannot be the last character", str(context.exception)) - # Test invalid InteropID with three colons. - with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('a:b:c:d') - self.assertIn("only zero or one ':' character is allowed", str(context.exception)) - - # Test UTF-8 strings with valid single colon. - utf8_valid_colon = '標準:萬國碼' - self.colorspace.setInteropID(utf8_valid_colon) - self.assertEqual(self.colorspace.getInteropID(), utf8_valid_colon) - - # Test UTF-8 strings with invalid multiple colons. - with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('標準:萬國:碼') - self.assertIn("only zero or one ':' character is allowed", str(context.exception)) - - # Test UTF-8 strings with invalid colon at end. + # Test invalid InteropID with non-ASCII characters. with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('標準萬國碼:') - self.assertIn("':' character cannot be the last character", str(context.exception)) + self.colorspace.setInteropID('café_scene') # Contains é (UTF-8) + self.assertIn("contains non-ASCII characters", str(context.exception)) - # Test UTF-8 strings without colon (should be valid). - utf8_no_colon = '標準萬國碼' - self.colorspace.setInteropID(utf8_no_colon) - self.assertEqual(self.colorspace.getInteropID(), utf8_no_colon) - - # Test edge case: single colon character. with self.assertRaises(Exception) as context: - self.colorspace.setInteropID(':') - self.assertIn("':' character cannot be the last character", str(context.exception)) - - # Test edge case: colon at beginning (should be valid). - colon_at_beginning = ':valid_name' - self.colorspace.setInteropID(colon_at_beginning) - self.assertEqual(self.colorspace.getInteropID(), colon_at_beginning) - - # Test with Unicode characters that might look like colons but aren't ASCII colons. - unicode_similar = 'test：name' # This uses a full-width colon (U+FF1A), not ASCII colon - self.colorspace.setInteropID(unicode_similar) - self.assertEqual(self.colorspace.getInteropID(), unicode_similar) - - # Test mixed ASCII and Unicode with valid single ASCII colon. - mixed_valid = 'ñamespace:色空間' - self.colorspace.setInteropID(mixed_valid) - self.assertEqual(self.colorspace.getInteropID(), mixed_valid) + self.colorspace.setInteropID('space±_name') # Contains ± (ANSI 0xB1) + self.assertIn("contains non-ASCII characters", str(context.exception)) def test_amf_transform_ids(self): """ @@ -900,7 +859,7 @@ def test_processor_to_known_colorspace(self): - ! name: Linear ITU-R BT.709 description: A linear Rec.709 space with an unusual spelling. - interop_id: "mycompany:lin_rec709_scene" + interop_id: "mycompany:led_wall_1" isdata: false from_scene_reference: ! name: AP0 to Linear Rec.709 (sRGB) From 316846337137156523e442bb43c513cc50a57d93 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Thu, 12 Jun 2025 16:02:26 -0700 Subject: [PATCH 09/15] Fixing the tests which were failing due to exception wording change. Signed-off-by: cuneyt.ozdas --- tests/cpu/ColorSpace_tests.cpp | 4 ++-- tests/python/ColorSpaceTest.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index 58ebe87d9a..d0eae45ae9 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -1866,11 +1866,11 @@ OCIO_ADD_TEST(ColorSpace, interop_id) // Test invalid InteropID with non-ASCII characters. const char * invalidNonASCII1 = "café_scene"; // Contains é (0xC3 0xA9) OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidNonASCII1), OCIO::Exception, - "contains non-ASCII characters"); + "is invalid: only ASCII characters [0x00..0x7F] are allowed."); const char * invalidNonASCII2 = "space±_name"; // Contains ± (ANSI 0xB1) OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidNonASCII2), OCIO::Exception, - "contains non-ASCII characters"); + "is invalid: only ASCII characters [0x00..0x7F] are allowed."); } OCIO_ADD_TEST(ColorSpace, interop_id_serialization) diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index 999de8bceb..72767db7b8 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -693,11 +693,11 @@ def test_interop_id(self): # Test invalid InteropID with non-ASCII characters. with self.assertRaises(Exception) as context: self.colorspace.setInteropID('café_scene') # Contains é (UTF-8) - self.assertIn("contains non-ASCII characters", str(context.exception)) + self.assertIn("is invalid: only ASCII characters [0x00..0x7F] are allowed.", str(context.exception)) with self.assertRaises(Exception) as context: self.colorspace.setInteropID('space±_name') # Contains ± (ANSI 0xB1) - self.assertIn("contains non-ASCII characters", str(context.exception)) + self.assertIn("is invalid: only ASCII characters [0x00..0x7F] are allowed.", str(context.exception)) def test_amf_transform_ids(self): """ From cc15e517320491f987e3bffcc21c491a762aaafe Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Wed, 10 Sep 2025 18:45:24 -0700 Subject: [PATCH 10/15] - replacing getAMFTransformIDs and getICCProfileName with more generic getInterchangeAttribute function. Same with the setters - getInterchangeAttribute() and setInterchangeAttribute() functions currently knows and accepts "amf_transform_ids" and "icc_profile_name" keys. other keys will throw. - ColorSpace serializer will list all of the key/value pairs. - InteropID and interchangeAttributes are now allowed in earlier config versions down to 2.0 too. Config::checkVersionConsistency() will throw only for version smaller than 2.0. - YAML loader and saver now supports the "interchange" section. Any unknown keys under that section will be ignored upon load and will generate a warning but won't throw. - Generic str key/value loader/saver is extended for re usability (and fixed incorrect throw message). - since the amf keywords are separated by newline characters, the newline sanitizer that was used for the description is also generalized and applied to all of the current and possible future interchange fields. - ColorSpace_test.cpp file updated to test the current state and behavior of the API. - ComputeValues function in the CPUProcessor_tests.cpp was taking the linenumber as as template parameter. This was a very bad idea as that means each invocation of the function would create a separate copy of the function which was causing all sorts of issues in the debugger and failing to compile when the line numbers are not constants (happens in JIT debug sessions). Fixed by making the line number a normal parameter. - Python ColorSpace object no longer takes the amf of icc parameters in the constructor. User needs to set them explicitely later on. Signed-off-by: cuneyt.ozdas --- include/OpenColorIO/OpenColorIO.h | 35 +- src/OpenColorIO/ColorSpace.cpp | 80 +++-- src/OpenColorIO/Config.cpp | 15 +- src/OpenColorIO/OCIOYaml.cpp | 187 ++++++---- src/OpenColorIO/ViewingRules.cpp | 2 - src/bindings/python/PyColorSpace.cpp | 21 +- tests/cpu/CPUProcessor_tests.cpp | 520 +++++++++++++-------------- tests/cpu/ColorSpace_tests.cpp | 137 ++++--- tests/python/ColorSpaceTest.py | 110 +++--- 9 files changed, 587 insertions(+), 520 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index c379b451ba..b7cb7523c5 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -13,6 +13,7 @@ #include #include #include +#include #include "OpenColorABI.h" #include "OpenColorTypes.h" @@ -1992,23 +1993,25 @@ class OCIOEXPORT ColorSpace void setInteropID(const char * interopID); /** - * Get/Set the AMF transform IDs for the color space. - * - * The AMF transform IDs are used to identify specific transforms in the ACES Metadata File. - * Multiple transform IDs can be specified in a newline-separated string. - */ - const char * getAMFTransformIDs() const noexcept; - void setAMFTransformIDs(const char * amfTransformIDs); - - /** - * Get/Set the ICC profile name for the color space. - * - * The ICC profile name identifies the ICC color profile associated with this color space. - * This can be used to link OCIO color spaces with corresponding ICC profiles for - * applications that need to work with both color management systems. + * Get/Set the interchange attributes. + * + * Currently supported attribute names are "amf_transform_ids" and + * "icc_profile_name". Using any other name will throw. If the attribute is + * not defined, it'll return empty string. Similarly setting it to empty + * string will effectively delete the attribute. + * + * The AMF transform IDs are used to identify specific transforms in the + * ACES Metadata File. Multiple transform IDs can be specified in a + * newline-separated string. + * + * The ICC profile name identifies the ICC color profile associated with + * this color space. This can be used to link OCIO color spaces with + * corresponding ICC profiles for applications that need to work with both + * color management systems. */ - const char * getICCProfileName() const noexcept; - void setICCProfileName(const char * iccProfileName); + const char *getInterchangeAttribute(const char *attrName) const; + void setInterchangeAttribute(const char* attrName, const char *value); + std::map getInterchangeAttributes() const noexcept; BitDepth getBitDepth() const noexcept; void setBitDepth(BitDepth bitDepth); diff --git a/src/OpenColorIO/ColorSpace.cpp b/src/OpenColorIO/ColorSpace.cpp index 229fafd6bf..e1ca36926e 100644 --- a/src/OpenColorIO/ColorSpace.cpp +++ b/src/OpenColorIO/ColorSpace.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include @@ -13,6 +14,13 @@ #include "utils/StringUtils.h" +namespace +{ +const std::array knownInterchangeNames = { + "amf_transform_ids", + "icc_profile_name" }; +} + namespace OCIO_NAMESPACE { @@ -25,9 +33,8 @@ class ColorSpace::Impl std::string m_description; std::string m_encoding; std::string m_interopID; - std::string m_AMFTransformIDs; - std::string m_ICCProfileName; StringUtils::StringVec m_aliases; + std::map m_interchangeAttribs; BitDepth m_bitDepth{ BIT_DEPTH_UNKNOWN }; bool m_isData{ false }; @@ -66,8 +73,7 @@ class ColorSpace::Impl m_description = rhs.m_description; m_encoding = rhs.m_encoding; m_interopID = rhs.m_interopID; - m_AMFTransformIDs = rhs.m_AMFTransformIDs; - m_ICCProfileName = rhs.m_ICCProfileName; + m_interchangeAttribs= rhs.m_interchangeAttribs; m_bitDepth = rhs.m_bitDepth; m_isData = rhs.m_isData; m_referenceSpaceType = rhs.m_referenceSpaceType; @@ -277,27 +283,60 @@ void ColorSpace::setInteropID(const char * interopID) getImpl()->m_interopID = id; } -const char * ColorSpace::getAMFTransformIDs() const noexcept +const char * ColorSpace::getInterchangeAttribute(const char* attrName) const { - return getImpl()->m_AMFTransformIDs.c_str(); -} + std::string nameTrimmed = StringUtils::Trim(attrName); + for (auto& key : knownInterchangeNames) + { + // do case-insensitive comparison. + if (StringUtils::Compare(key, nameTrimmed)) + { + auto it = m_impl->m_interchangeAttribs.find(key); + if (it != m_impl->m_interchangeAttribs.end()) + { + return it->second.c_str(); + } + return ""; + } + } -void ColorSpace::setAMFTransformIDs(const char * amfTransformIDs) -{ - getImpl()->m_AMFTransformIDs = amfTransformIDs ? amfTransformIDs : ""; + std::ostringstream oss; + oss << "Unknown attribute name '" << attrName << "'."; + throw Exception(oss.str().c_str()); } -const char * ColorSpace::getICCProfileName() const noexcept +void ColorSpace::setInterchangeAttribute(const char* attrName, const char* value) { - return getImpl()->m_ICCProfileName.c_str(); + std::string nameTrimmed = StringUtils::Trim(attrName); + for (auto& key : knownInterchangeNames) + { + // Do case-insensitive comparison. + if (StringUtils::Compare(key, nameTrimmed)) + { + // use key instead of nameTrim for storing in correct capitalization. + if (!value || !*value) + { + m_impl->m_interchangeAttribs.erase(key); + } + else + { + m_impl->m_interchangeAttribs[key] = value; + } + return; + } + } + + std::ostringstream oss; + oss << "Unknown attribute name '" << attrName << "'."; + throw Exception(oss.str().c_str()); } -void ColorSpace::setICCProfileName(const char * iccProfileName) +std::map ColorSpace::getInterchangeAttributes() const noexcept { - getImpl()->m_ICCProfileName = iccProfileName ? iccProfileName : ""; + return m_impl->m_interchangeAttribs; } -BitDepth ColorSpace::getBitDepth() const noexcept +BitDepth ColorSpace::ColorSpace::getBitDepth() const noexcept { return getImpl()->m_bitDepth; } @@ -516,15 +555,10 @@ std::ostream & operator<< (std::ostream & os, const ColorSpace & cs) { os << ", description=" << str; } - str = cs.getAMFTransformIDs(); - if (!str.empty()) - { - os << ", amf_transform_ids=" << str; - } - str = cs.getICCProfileName(); - if (!str.empty()) + // TODO: Do we need to output the section name too? + for (const auto& attr : cs.getInterchangeAttributes()) { - os << ", icc_profile_name=" << str; + os << ", " << attr.first << "=" << attr.second; } if(cs.getTransform(COLORSPACE_DIR_TO_REFERENCE)) { diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index d2f9f1f040..5e894a2f7c 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -5675,27 +5675,20 @@ void Config::Impl::checkVersionConsistency() const } } - if (hexVersion < 0x02050000) // Version 2.5 + if (m_majorVersion < 2) { if (*cs->getInteropID()) { std::ostringstream os; os << "Config failed validation. The color space '" << cs->getName() << "' "; - os << "has non-empty InteropID and config version is less than 2.5."; - throw Exception(os.str().c_str()); - } - if (*cs->getAMFTransformIDs()) - { - std::ostringstream os; - os << "Config failed validation. The color space '" << cs->getName() << "' "; - os << "has non-empty AMFTransformIDs and config version is less than 2.5."; + os << "has non-empty InteropID and config version is less than 2.0."; throw Exception(os.str().c_str()); } - if (*cs->getICCProfileName()) + if (cs->getInterchangeAttributes().size()>0) { std::ostringstream os; os << "Config failed validation. The color space '" << cs->getName() << "' "; - os << "has non-empty ICCProfileName and config version is less than 2.5."; + os << "has non-empty interchange attributes and config version is less than 2.0."; throw Exception(os.str().c_str()); } } diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index bdf45d7109..29ab436bbe 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -33,6 +33,29 @@ namespace { typedef YAML::const_iterator Iterator; +void SanitizeLoadedNewlines(std::string &str) +{ + if (!str.empty()) + { + // YAML is changing the trailing newlines when reading them: + // - Written as YAML::Literal (starts with a "|"), descriptions will be read back with a + // single newline. One is added if there was none, only one is kept if there were + // several. + // - Written as YAML::Value string (does not start with "|"), all trailing newlines ('\n') + // are preserved. + // Trailing newlines are inconsistently preserved, lets remove them in all cases. + auto last = str.back(); + while (last == '\n' && str.length()) + { + str.pop_back(); + last = str.back(); + } + } + // Also, note that a \n is only interpreted as a newline if it is used in a string that is + // within double quotes. E.g., "A string \n with embedded \n newlines." Indeed, without the + // double quotes the backslash is generally not interpreted as an escape character in YAML. +} + // Basic types inline void load(const YAML::Node& node, bool& x) @@ -185,25 +208,7 @@ inline void save(YAML::Emitter& out, Interpolation interp) inline void loadDescription(const YAML::Node& node, std::string& x) { load(node, x); - if (!x.empty()) - { - // YAML is changing the trailing newlines when reading them: - // - Written as YAML::Literal (starts with a "|"), descriptions will be read back with a - // single newline. One is added if there was none, only one is kept if there were - // several. - // - Written as YAML::Value string (does not start with "|"), all trailing newlines ('\n') - // are preserved. - // Trailing newlines are inconsistently preserved, lets remove them in all cases. - auto last = x.back(); - while (last == '\n' && x.length()) - { - x.pop_back(); - last = x.back(); - } - } - // Also, note that a \n is only interpreted as a newline if it is used in a string that is - // within double quotes. E.g., "A string \n with embedded \n newlines." Indeed, without the - // double quotes the backslash is generally not interpreted as an escape character in YAML. + SanitizeLoadedNewlines(x); } inline void saveDescription(YAML::Emitter & out, const char * desc) @@ -319,6 +324,31 @@ inline void CheckDuplicates(const YAML::Node & node) } } +// Custom Key Loader + +struct CustomKeysLoader +{ + using Type = std::vector>; + Type m_keyVals; +}; + +inline void loadCustomKeys(const YAML::Node& node, CustomKeysLoader & ck, const char* sectionName) +{ + if (node.Type() == YAML::NodeType::Map) + { + for (Iterator iter = node.begin(); iter != node.end(); ++iter) + { + ck.m_keyVals.push_back(std::make_pair(iter->first, iter->second)); + } + } + else + { + std::ostringstream ss; + ss << "The '" << sectionName << "' section need to be a YAML map."; + throwError(node, ss.str().c_str()); + } +} + // View inline void load(const YAML::Node& node, View& v) @@ -3212,15 +3242,28 @@ inline void load(const YAML::Node& node, ColorSpaceRcPtr& cs, unsigned int major loadDescription(iter->second, stringval); cs->setDescription(stringval.c_str()); } - else if (key == "amf_transform_ids") + else if (key == "interchange") { - load(iter->second, stringval); - cs->setAMFTransformIDs(stringval.c_str()); - } - else if (key == "icc_profile_name") - { - load(iter->second, stringval); - cs->setICCProfileName(stringval.c_str()); + CustomKeysLoader kv; + loadCustomKeys(iter->second, kv, "ColorSpace interchange"); + + for (const auto& keyval : kv.m_keyVals) + { + // OCIO exception means the key is not recognized. Convert that to a warning. + try + { + std::string val = keyval.second.as(); + SanitizeLoadedNewlines(val); + + cs->setInterchangeAttribute( + keyval.first.as().c_str(), + val.c_str()); + } + catch (Exception*) + { + LogUnknownKeyWarning(iter->second, keyval.first); + } + } } else if(key == "family") { @@ -3355,18 +3398,38 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major saveDescription(out, cs->getDescription()); - const std::string amfTransformIDs{cs->getAMFTransformIDs()}; - if (!amfTransformIDs.empty()) + auto interchangemap = cs->getInterchangeAttributes(); + if (interchangemap.size()) { - out << YAML::Key << "amf_transform_ids"; - out << YAML::Value << amfTransformIDs; - } + out << YAML::Key << "interchange"; + out << YAML::Value; + out << YAML::BeginMap; + for (const auto& keyval : interchangemap) + { + //out << YAML::Key << keyval.first; + //<< YAML::Value << keyval.second; - const std::string iccProfileName{cs->getICCProfileName()}; - if (!iccProfileName.empty()) - { - out << YAML::Key << "icc_profile_name"; - out << YAML::Value << iccProfileName; + // Remove trailing newlines so that only one is saved because they won't + // be read back. + std::string valStr{keyval.second}; + { + auto last = valStr.back(); + while (last == '\n' && valStr.length()) + { + valStr.pop_back(); + last = valStr.back(); + } + } + + out << YAML::Key << keyval.first << YAML::Value; + if (valStr.find_first_of('\n') != std::string::npos) + { + out << YAML::Literal; + } + out << valStr; + } + + out << YAML::EndMap; } out << YAML::Key << "isdata" << YAML::Value << cs->isData(); @@ -3819,30 +3882,6 @@ inline void save(YAML::Emitter & out, ConstNamedTransformRcPtr & nt, unsigned in // File rules -struct CustomKeysLoader -{ - StringUtils::StringVec m_keyVals; -}; - -inline void loadCustomKeys(const YAML::Node& node, CustomKeysLoader & ck) -{ - if (node.Type() == YAML::NodeType::Map) - { - for (Iterator iter = node.begin(); iter != node.end(); ++iter) - { - const std::string & key = iter->first.as(); - const std::string & val = iter->second.as(); - - ck.m_keyVals.push_back(key); - ck.m_keyVals.push_back(val); - } - } - else - { - throwError(node, "The 'file_rules' custom attributes need to be a YAML map."); - } -} - inline void load(const YAML::Node & node, FileRulesRcPtr & fr, bool & defaultRuleFound) { if (node.Tag() != "Rule") @@ -3852,7 +3891,7 @@ inline void load(const YAML::Node & node, FileRulesRcPtr & fr, bool & defaultRul std::string stringval; std::string name, colorspace, pattern, extension, regex; - StringUtils::StringVec keyVals; + CustomKeysLoader::Type keyVals; for (Iterator iter = node.begin(); iter != node.end(); ++iter) { @@ -3888,7 +3927,7 @@ inline void load(const YAML::Node & node, FileRulesRcPtr & fr, bool & defaultRul else if (key == FileRuleUtils::CustomKey) { CustomKeysLoader kv; - loadCustomKeys(iter->second, kv); + loadCustomKeys(iter->second, kv, "file_rules custom attribute"); keyVals = kv.m_keyVals; } else @@ -3959,11 +3998,15 @@ inline void load(const YAML::Node & node, FileRulesRcPtr & fr, bool & defaultRul fr->insertRule(pos, name.c_str(), colorspace.c_str(), regex.c_str()); } } - const auto numKeyVal = keyVals.size() / 2; - for (size_t i = 0; i < numKeyVal; ++i) + + for (const auto& keyval : keyVals) { - fr->setCustomKey(pos, keyVals[i * 2].c_str(), keyVals[i * 2 + 1].c_str()); + fr->setCustomKey( + pos, + keyval.first.as().c_str(), + keyval.second.as().c_str()); } + } catch (Exception & ex) { @@ -4026,7 +4069,7 @@ inline void load(const YAML::Node & node, ViewingRulesRcPtr & vr) std::string stringval; std::string name; StringUtils::StringVec colorspaces, encodings; - StringUtils::StringVec keyVals; + CustomKeysLoader::Type keyVals; for (Iterator iter = node.begin(); iter != node.end(); ++iter) { @@ -4068,7 +4111,7 @@ inline void load(const YAML::Node & node, ViewingRulesRcPtr & vr) else if (key == ViewingRuleUtils::CustomKey) { CustomKeysLoader kv; - loadCustomKeys(iter->second, kv); + loadCustomKeys(iter->second, kv, "viewing_rules custom attribute"); keyVals = kv.m_keyVals; } else @@ -4091,10 +4134,12 @@ inline void load(const YAML::Node & node, ViewingRulesRcPtr & vr) vr->addEncoding(pos, is.c_str()); } - const auto numKeyVal = keyVals.size() / 2; - for (size_t i = 0; i < numKeyVal; ++i) + for (const auto& keyval : keyVals) { - vr->setCustomKey(pos, keyVals[i * 2].c_str(), keyVals[i * 2 + 1].c_str()); + vr->setCustomKey( + pos, + keyval.first.as().c_str(), + keyval.second.as().c_str()); } } catch (Exception & ex) diff --git a/src/OpenColorIO/ViewingRules.cpp b/src/OpenColorIO/ViewingRules.cpp index be780c94e1..bbe27172c1 100644 --- a/src/OpenColorIO/ViewingRules.cpp +++ b/src/OpenColorIO/ViewingRules.cpp @@ -38,8 +38,6 @@ class ViewingRule { public: - using CustomKeys = std::map; - ViewingRule() = delete; ViewingRule(const ViewingRule &) = delete; ViewingRule & operator=(const ViewingRule &) = delete; diff --git a/src/bindings/python/PyColorSpace.cpp b/src/bindings/python/PyColorSpace.cpp index b74c98c196..a115b20bd9 100644 --- a/src/bindings/python/PyColorSpace.cpp +++ b/src/bindings/python/PyColorSpace.cpp @@ -91,9 +91,8 @@ void bindPyColorSpace(py::module & m) const TransformRcPtr & toReference, const TransformRcPtr & fromReference, const std::vector & categories, - const std::string & interopID, - const std::string& AMFTransformID, - const std::string& ICCProfileName) + const std::string & interopID + ) { ColorSpaceRcPtr p = ColorSpace::Create(referenceSpace); if (!aliases.empty()) @@ -111,8 +110,6 @@ void bindPyColorSpace(py::module & m) if (!equalityGroup.empty()) { p->setEqualityGroup(equalityGroup.c_str()); } if (!description.empty()) { p->setDescription(description.c_str()); } if (!interopID.empty()) { p->setInteropID(interopID.c_str()); } - if (!AMFTransformID.empty()) { p->setAMFTransformIDs(AMFTransformID.c_str()); } - if (!ICCProfileName.empty()) { p->setICCProfileName(ICCProfileName.c_str()); } p->setBitDepth(bitDepth); p->setIsData(isData); p->setAllocation(allocation); @@ -157,8 +154,6 @@ void bindPyColorSpace(py::module & m) "fromReference"_a = DEFAULT->getTransform(COLORSPACE_DIR_FROM_REFERENCE), "categories"_a = getCategoriesStdVec(DEFAULT), "interopID"_a = DEFAULT->getInteropID(), - "amfTransformIDs"_a = DEFAULT->getAMFTransformIDs(), - "iccProfileName"_a = DEFAULT->getICCProfileName(), DOC(ColorSpace, Create, 2)) .def("__deepcopy__", [](const ConstColorSpaceRcPtr & self, py::dict) @@ -206,14 +201,10 @@ void bindPyColorSpace(py::module & m) DOC(ColorSpace, getInteropID)) .def("setInteropID", &ColorSpace::setInteropID, "interopID"_a, DOC(ColorSpace, setInteropID)) - .def("getAMFTransformIDs", &ColorSpace::getAMFTransformIDs, - DOC(ColorSpace, getAMFTransformIDs)) - .def("setAMFTransformIDs", &ColorSpace::setAMFTransformIDs, "amfTransformIDs"_a, - DOC(ColorSpace, setAMFTransformIDs)) - .def("getICCProfileName", &ColorSpace::getICCProfileName, - DOC(ColorSpace, getICCProfileName)) - .def("setICCProfileName", &ColorSpace::setICCProfileName, "iccProfileName"_a, - DOC(ColorSpace, setICCProfileName)) + .def("getInterchangeAttribute", &ColorSpace::getInterchangeAttribute, "attrName"_a, + DOC(ColorSpace, getInterchangeAttribute)) + .def("setInterchangeAttribute", &ColorSpace::setInterchangeAttribute, "attrName"_a, "attrValue"_a, + DOC(ColorSpace, setInterchangeAttribute)) .def("getBitDepth", &ColorSpace::getBitDepth, DOC(ColorSpace, getBitDepth)) .def("setBitDepth", &ColorSpace::setBitDepth, "bitDepth"_a, diff --git a/tests/cpu/CPUProcessor_tests.cpp b/tests/cpu/CPUProcessor_tests.cpp index c2fed39022..925a75397c 100644 --- a/tests/cpu/CPUProcessor_tests.cpp +++ b/tests/cpu/CPUProcessor_tests.cpp @@ -57,8 +57,9 @@ OCIO_ADD_TEST(CPUProcessor, flag_composition) // a missing/partial optimization. -template -OCIO::ConstCPUProcessorRcPtr ComputeValues(OCIO::ConstProcessorRcPtr processor, +template +OCIO::ConstCPUProcessorRcPtr ComputeValues(unsigned line, + OCIO::ConstProcessorRcPtr processor, const void * inImg, OCIO::ChannelOrdering inChans, const void * resImg, @@ -147,12 +148,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.5025f, 0.9050f, 1.5896f, 1.5000f, 2.4002f, 1.6505f, 2.0707f, 0.5000f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, + 1e-5f); } { @@ -161,12 +162,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 0.182999f, 0.9050f, 2.9091f, 1.5000f, 1.0807f, 1.6505f, 3.3902f, 0.5000f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS, + 1e-5f); } { @@ -175,12 +176,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 0.602300f, 0.585199f, 1.909399f, 2.400200f, 1.500000f, 1.330700f, 2.390500f, 1.400200f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_ABGR, - &resImg[0], OCIO::CHANNEL_ORDERING_ABGR, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_ABGR, + &resImg[0], OCIO::CHANNEL_ORDERING_ABGR, + NB_PIXELS, + 1e-5f); } { @@ -189,12 +190,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.5896f, 0.9050f, 1.5025f, 1.5000f, 2.0707f, 1.6505f, 2.4002f, 0.5000f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS, + 1e-5f); } { @@ -203,12 +204,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.5000f, 1.5896f, 0.9050f, 1.5025f, 0.5000f, 2.0707f, 1.6505f, 2.4002f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_ABGR, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_ABGR, + NB_PIXELS, + 1e-5f); } { @@ -222,12 +223,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.5025f, 0.9050f, 1.5896f, 2.4002f, 1.6505f, 2.0707f }; - ComputeValues(processor, - &inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &resImg[0], OCIO::CHANNEL_ORDERING_RGB, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &resImg[0], OCIO::CHANNEL_ORDERING_RGB, + NB_PIXELS, + 1e-5f); } { @@ -241,12 +242,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 0.182999f, 0.905000f, 2.909100f, 1.080700f, 1.650500f, 3.390200f }; - ComputeValues(processor, - &inImg[0], OCIO::CHANNEL_ORDERING_BGR, - &resImg[0], OCIO::CHANNEL_ORDERING_BGR, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &inImg[0], OCIO::CHANNEL_ORDERING_BGR, + &resImg[0], OCIO::CHANNEL_ORDERING_BGR, + NB_PIXELS, + 1e-5f); } { @@ -260,12 +261,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.58960f, 0.9050f, 1.5025f, 2.070699f, 1.6505f, 2.4002f }; - ComputeValues(processor, - &inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &resImg[0], OCIO::CHANNEL_ORDERING_BGR, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &resImg[0], OCIO::CHANNEL_ORDERING_BGR, + NB_PIXELS, + 1e-5f); } { @@ -279,12 +280,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.58960f, 0.9050f, 1.5025f, 0.5f, 2.070699f, 1.6505f, 2.4002f, 0.5f }; - ComputeValues(processor, - &inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS, + 1e-5f); } { @@ -298,12 +299,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.58960f, 0.9050f, 1.5025f, 2.070699f, 1.6505f, 2.4002f }; - ComputeValues(processor, - &inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGR, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGR, + NB_PIXELS, + 1e-5f); } const std::vector ui16_inImg = @@ -317,12 +318,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1.40117657f, 0.40245315f, 0.08460631f, 0.5f, 1.47832620f, 0.70781672f, 1.08070004f, 0.5f }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, - 1e-5f); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, + 1e-5f); } { @@ -331,11 +332,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 65535, 26375, 5545, 32768, 65535, 46387, 65535, 32768 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -344,11 +345,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 5545, 26375, 65535, 32768, 65535, 46387, 65535, 32768 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } { @@ -357,11 +358,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 5545, 26375, 65535, 65535, 46387, 65535 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGR, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGR, + NB_PIXELS); } { @@ -370,11 +371,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 255, 103, 22, 128, 255, 180, 255, 128 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -383,11 +384,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 22, 103, 255, 255, 180, 255 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGR, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGR, + NB_PIXELS); } { @@ -396,11 +397,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 128, 22, 103, 255, 128, 255, 180, 255 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_ABGR, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_ABGR, + NB_PIXELS); } // Test OCIO::BIT_DEPTH_UINT10. @@ -411,11 +412,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 1023, 412, 87, 512, 1023, 724, 1023, 512 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &ui10_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &ui10_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); const std::vector ui10_inImg = { 0, 8, 12, 256, @@ -427,11 +428,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 65535, 27272, 9389, 65535, 65535, 28297, 11439, 65535 }; - ComputeValues(processor, - &ui10_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui10_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } // Test OCIO::BIT_DEPTH_UINT12. @@ -442,11 +443,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 4095, 1648, 346, 2048, 4095, 2899, 4095, 2048 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &ui12_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &ui12_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); const std::vector ui12_inImg = { 0, 8, 12, 1024, @@ -458,11 +459,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_matrix) 65535, 26503, 6313, 65535, 65535, 26759, 6825, 65535 }; - ComputeValues(processor, - &ui12_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui12_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } } @@ -499,12 +500,12 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 0.29089212f, 0.50935059f, 1.91091322f, 1, 64, 64, 64, 0 }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, - 1e-7f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, + 1e-7f); } { @@ -514,11 +515,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 19064, 33380, 65535, 65535, 65535, 65535, 65535, 0 }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } const std::vector ui16_inImg = @@ -534,11 +535,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 0.00601041f, 0.00912247f, 0.01456576f, 0.00097657f, 0.03030112f, 0.13105739f, 64, 0.00781261f }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, 1e-7f); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, 1e-7f); } { @@ -548,11 +549,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 394, 598, 955, 64, 1986, 8589, 65535, 512 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -562,11 +563,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 955, 598, 394, 64, 65535, 8589, 1986, 512 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -576,11 +577,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 955, 598, 394, 64, 65535, 8589, 1986, 512 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } { @@ -590,11 +591,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 955, 598, 394, 65535, 8589, 1986 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGR, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGR, + NB_PIXELS); } { @@ -604,11 +605,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 394, 598, 955, 64, 1986, 8589, 65535, 512 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } { @@ -624,11 +625,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 955, 598, 394, 0, 65535, 8589, 1986, 0 }; - ComputeValues(processor, - &my_i_inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &my_i_inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } // Test OCIO::BIT_DEPTH_UINT10. @@ -640,11 +641,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 6, 9, 15, 1, 31, 134, 1023, 8 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &ui10_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &ui10_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -660,11 +661,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 36, 106, 252, 0, 48, 1023, 384, 0 }; - ComputeValues(processor, - &ui10_inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &ui10_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui10_inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &ui10_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); const std::vector ui16_resImg = { 0, 394, 955, 0, @@ -672,11 +673,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 2301, 6794, 16162, 0, 3092, 65535, 24593, 0 }; - ComputeValues(processor, - &ui10_inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui10_inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } // Test OCIO::BIT_DEPTH_UINT12. @@ -688,11 +689,11 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 25, 37, 60, 4, 124, 537, 4095, 32 }; - ComputeValues(processor, - &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &ui12_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui16_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &ui12_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -708,22 +709,22 @@ OCIO_ADD_TEST(CPUProcessor, with_one_1d_lut) 49, 103, 193, 0, 424, 1009, 4095, 0 }; - ComputeValues(processor, - &ui12_inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &ui12_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui12_inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &ui12_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); const std::vector ui16_resImg = { 0, 178, 394, 0, 598, 955, 1655, 0, 779, 1655, 3089, 0, 6789, 16143, 65535, 0 }; - ComputeValues(processor, - &ui12_inImg[0], OCIO::CHANNEL_ORDERING_RGB, - &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &ui12_inImg[0], OCIO::CHANNEL_ORDERING_RGB, + &ui16_resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } } @@ -794,11 +795,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 0.15488569f, 1.69210147f, 1.90666747f, 1.0f, 0.81575858f, 64.0f, 64.0f, 0.0f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, 1e-7f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, 1e-7f); } { @@ -808,11 +809,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 10150, 65535, 65535, 65535, 53461, 65535, 65535, 0 }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } const std::vector i_inImg = @@ -828,12 +829,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 0.0f, 0.08474064f, 0.01450117f, 0.0f, 0.0f, 0.24826171f, 56.39490891f, 1.0f }; - auto cpuProcessor = ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, 1e-7f); + auto cpuProcessor = ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, 1e-7f); // SSE2/AVX/AVX2 generate a slightly different LUT1D // floating error below the absErrorThreshold, but cacheID hash will be different @@ -874,11 +874,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 0, 5553, 950, 0, 0, 16270, 65535, 65535 }; - ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -888,11 +888,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 0, 5553, 388, 0, 53461, 16270, 1982, 65535 }; - ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } { @@ -902,11 +902,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 950, 5553, 0, 0, 65535, 16270, 0, 65535 }; - ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } { @@ -916,11 +916,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 388, 5553, 0, 0, 1982, 16270, 53461, 65535 }; - ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } } @@ -960,11 +960,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 0.10089212f, 0.69935059f, 1.91072320f, 1.0f, 63.81000137f, 64.19000244f, 63.99980927f, 0.0f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, 1e-7f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, 1e-7f); } { @@ -974,11 +974,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 6612, 45832, 65535, 65535, 65535, 65535, 65535, 0 }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS); } const std::vector i_inImg = @@ -993,11 +993,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) -0.18398958f, 0.19912247f, 0.01437576f, 0.0f, -0.15969887f, 0.32105737f, 63.99980927f, 0.0f }; - ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, 1e-7f); + ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, 1e-7f); } { @@ -1007,11 +1007,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 381, 13049, 0, 0, 1973, 21040, 65535, 0 }; - ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } } @@ -1053,11 +1053,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) -0.23451784f, 0.92250210f, 3.26448941f, 1.0f, 3.43709063f, 3.43709063f, 3.43709063f, 0.0f }; - ComputeValues(processor, - &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, - &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, - NB_PIXELS, 1e-7f); + ComputeValues( + __LINE__, processor, + &f_inImg[0], OCIO::CHANNEL_ORDERING_RGBA, + &resImg[0], OCIO::CHANNEL_ORDERING_RGBA, + NB_PIXELS, 1e-7f); } const std::vector i_inImg = @@ -1073,11 +1073,11 @@ OCIO_ADD_TEST(CPUProcessor, with_several_ops) 0, 0, 0, 0, 0, 12526, 65535, 0 }; - ComputeValues(processor, - &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, - &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, - NB_PIXELS); + ComputeValues( + __LINE__, processor, + &i_inImg[0], OCIO::CHANNEL_ORDERING_BGRA, + &resImg[0], OCIO::CHANNEL_ORDERING_BGRA, + NB_PIXELS); } } } diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index d0eae45ae9..3722a5c169 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -47,9 +47,9 @@ OCIO_ADD_TEST(ColorSpace, basic) cs->setFamily("FAMILY"); cs->setEqualityGroup("EQGRP"); cs->setEncoding("ENC"); - cs->setAMFTransformIDs("AMF"); cs->setInteropID("INTEROP"); - cs->setICCProfileName("ICC"); + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("amf_transform_ids", "AMF")); + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("icc_profile_name", "ICC")); // Set to nullptr, this should erase the old values. OCIO_CHECK_NO_THROW(cs->setName(nullptr)); @@ -57,9 +57,9 @@ OCIO_ADD_TEST(ColorSpace, basic) OCIO_CHECK_NO_THROW(cs->setFamily(nullptr)); OCIO_CHECK_NO_THROW(cs->setEqualityGroup(nullptr)); OCIO_CHECK_NO_THROW(cs->setEncoding(nullptr)); - OCIO_CHECK_NO_THROW(cs->setAMFTransformIDs(nullptr)); OCIO_CHECK_NO_THROW(cs->setInteropID(nullptr)); - OCIO_CHECK_NO_THROW(cs->setICCProfileName(nullptr)); + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("amf_transform_ids", nullptr)); + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("icc_profile_name", nullptr)); // Check that the values are empty now. OCIO_CHECK_ASSERT(!*cs->getName()); @@ -67,9 +67,9 @@ OCIO_ADD_TEST(ColorSpace, basic) OCIO_CHECK_ASSERT(!*cs->getFamily()); OCIO_CHECK_ASSERT(!*cs->getEqualityGroup()); OCIO_CHECK_ASSERT(!*cs->getEncoding()); - OCIO_CHECK_ASSERT(!*cs->getAMFTransformIDs()); OCIO_CHECK_ASSERT(!*cs->getInteropID()); - OCIO_CHECK_ASSERT(!*cs->getICCProfileName()); + OCIO_CHECK_ASSERT(!*cs->getInterchangeAttribute("amf_transform_ids")); + OCIO_CHECK_ASSERT(!*cs->getInterchangeAttribute("icc_profile_name")); // Test set/get roundtrip. cs->setName("name"); @@ -97,11 +97,11 @@ OCIO_ADD_TEST(ColorSpace, basic) OCIO_CHECK_EQUAL(2.f, readVars[1]); cs->setInteropID("interop_id"); OCIO_CHECK_EQUAL(std::string("interop_id"), cs->getInteropID()); - cs->setAMFTransformIDs("amf_transform_id1\namf_transform_id2"); + cs->setInterchangeAttribute("amf_transform_ids", "amf_transform_id1\namf_transform_id2"); OCIO_CHECK_EQUAL(std::string("amf_transform_id1\namf_transform_id2"), - cs->getAMFTransformIDs()); - cs->setICCProfileName("icc_profile_name"); - OCIO_CHECK_EQUAL(std::string("icc_profile_name"), cs->getICCProfileName()); + cs->getInterchangeAttribute("amf_transform_ids")); + cs->setInterchangeAttribute("icc_profile_name","icc_profile_name"); + OCIO_CHECK_EQUAL(std::string("icc_profile_name"), cs->getInterchangeAttribute("icc_profile_name")); std::ostringstream oss; oss << *cs; @@ -659,7 +659,7 @@ active_views: [] OCIO_CHECK_EQUAL(cfgRes, os.str()); } - // Test that the interop_id can't be used in v2.0 config. + // Test that the interop_id is valid in v2.0 config too. { constexpr char End[]{R"(colorspaces: - ! @@ -678,18 +678,19 @@ active_views: [] std::istringstream is; is.str(cfgString); OCIO::ConstConfigRcPtr config; - OCIO_CHECK_THROW_WHAT( - config = OCIO::Config::CreateFromStream(is), - OCIO::Exception, - "Config failed validation. The color space 'raw' has non-empty InteropID and config version is less than 2.5."); + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + auto cs = config->getColorSpace("raw"); + OCIO_CHECK_ASSERT(cs); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), "data"); } - // Test that the amf_transform_ids can't be used in v2.0 config. + // Test that the amf_transform_ids is valid in v2.0 config too. { constexpr char End[]{R"(colorspaces: - ! name: raw - amf_transform_ids: this:shouldnt:be:here + interchange: + amf_transform_ids: should be valid in 2.0 config too family: raw equalitygroup: "" bitdepth: 32f @@ -703,18 +704,16 @@ active_views: [] std::istringstream is; is.str(cfgString); OCIO::ConstConfigRcPtr config; - OCIO_CHECK_THROW_WHAT( - config = OCIO::Config::CreateFromStream(is), OCIO::Exception, - "Config failed validation. The color space 'raw' has non-empty " - "AMFTransformIDs and config version is less than 2.5."); + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); } - // Test that the icc_profile_name can't be used in v2.0 config. + // Test that the icc_profile_name is valid in v2.0 config too. { constexpr char End[]{R"(colorspaces: - ! name: raw - icc_profile_name: not valid in v2.0 config + interchange: + icc_profile_name: should be valid in 2.0 config too family: raw equalitygroup: "" bitdepth: 32f @@ -728,10 +727,7 @@ active_views: [] std::istringstream is; is.str(cfgString); OCIO::ConstConfigRcPtr config; - OCIO_CHECK_THROW_WHAT( - config = OCIO::Config::CreateFromStream(is), OCIO::Exception, - "Config failed validation. The color space 'raw' has non-empty " - "ICCProfileName and config version is less than 2.5."); + OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); } } @@ -1903,14 +1899,17 @@ OCIO_ADD_TEST(ColorSpace, interop_id_serialization) OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); OCIO_CHECK_EQUAL(std::string(deserializedCs->getInteropID()), interop_id); - // verify that that earlier versions reject interop_id. + // verify that that versions earlier than 2.0 reject interop_id. OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); - cfgCopy->setVersion(2, 4); + cfgCopy->setVersion(2, 0); + OCIO_CHECK_NO_THROW(cfgCopy->serialize(ss)); + + cfgCopy->setVersion(1, 0); OCIO_CHECK_THROW_WHAT( - cfgCopy->serialize(ss), + cfgCopy->serialize(ss), OCIO::Exception, "Config failed validation. The color space 'test_colorspace' has non-empty " - "InteropID and config version is less than 2.5."); + "InteropID and config version is less than 2.0."); // Test with empty interop_id (should not appear in YAML). cs->setInteropID(nullptr); @@ -1928,33 +1927,33 @@ OCIO_ADD_TEST(ColorSpace, amf_transform_ids) OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); // Test default value. - OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), ""); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); // Test setting and getting single ID. const char * singleID = "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3"; - cs->setAMFTransformIDs(singleID); - OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), singleID); + cs->setInterchangeAttribute("amf_transform_ids", singleID); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), singleID); // Test setting to empty string. - cs->setAMFTransformIDs(""); - OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), ""); + cs->setInterchangeAttribute("amf_transform_ids", ""); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); // Test setting and getting multiple IDs. const char * multipleIDs = "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; - cs->setAMFTransformIDs(multipleIDs); - OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), multipleIDs); + cs->setInterchangeAttribute("amf_transform_ids", multipleIDs); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), multipleIDs); // Test setting to null pointer (should be safe). - cs->setAMFTransformIDs("something"); - cs->setAMFTransformIDs(nullptr); - OCIO_CHECK_EQUAL(std::string(cs->getAMFTransformIDs()), ""); + cs->setInterchangeAttribute("amf_transform_ids", "something"); + cs->setInterchangeAttribute("amf_transform_ids", nullptr); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); // Test copy constructor preserves AMF transform IDs. - cs->setAMFTransformIDs(singleID); + cs->setInterchangeAttribute("amf_transform_ids", singleID); OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); - OCIO_CHECK_EQUAL(std::string(copy->getAMFTransformIDs()), singleID); + OCIO_CHECK_EQUAL(std::string(copy->getInterchangeAttribute("amf_transform_ids")), singleID); } OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) @@ -1968,7 +1967,7 @@ OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; - cs->setAMFTransformIDs(amfIDs.c_str()); + cs->setInterchangeAttribute("amf_transform_ids", amfIDs.c_str()); cfg->addColorSpace(cs); // Serialize the Config. @@ -1987,18 +1986,16 @@ OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) // Verify AmfTransformIDs is preserved. OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); - OCIO_CHECK_EQUAL(std::string(deserializedCs->getAMFTransformIDs()), amfIDs); + auto deserializedIDs = deserializedCs->getInterchangeAttribute("amf_transform_ids"); + OCIO_CHECK_EQUAL(deserializedIDs, amfIDs); - // Verify that that earlier versions reject amf_transform_ids. + // Verify that that earlier versions retain amf_transform_ids. OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); cfgCopy->setVersion(2,4); - OCIO_CHECK_THROW_WHAT(cfgCopy->serialize(ss), - OCIO::Exception, - "Config failed validation. The color space 'test_colorspace' has non-empty " - "AMFTransformIDs and config version is less than 2.5."); + OCIO_CHECK_NO_THROW(cfgCopy->serialize(ss)); // Test with empty AmfTransformIDs (should not appear in YAML). - cs->setAMFTransformIDs(nullptr); + cs->setInterchangeAttribute("amf_transform_ids", nullptr); cfg->addColorSpace(cs); // Replace the existing CS. ss.str(""); cfg->serialize(ss); @@ -2013,31 +2010,31 @@ OCIO_ADD_TEST(ColorSpace, icc_profile_name) OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); // Test default value. - OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); // Test setting and getting single profile name. const char * profileName = "sRGB IEC61966-2.1"; - cs->setICCProfileName(profileName); - OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), profileName); + cs->setInterchangeAttribute("icc_profile_name",profileName); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), profileName); // Test setting and getting another profile name. const char * anotherProfile = "Adobe RGB (1998)"; - cs->setICCProfileName(anotherProfile); - OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), anotherProfile); + cs->setInterchangeAttribute("icc_profile_name", anotherProfile); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), anotherProfile); // Test setting empty string. - cs->setICCProfileName(""); - OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + cs->setInterchangeAttribute("icc_profile_name", ""); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); // Test setting null pointer (should be safe). - cs->setICCProfileName("something"); - OCIO_CHECK_NO_THROW(cs->setICCProfileName(nullptr)); - OCIO_CHECK_EQUAL(std::string(cs->getICCProfileName()), ""); + cs->setInterchangeAttribute("icc_profile_name", "something"); + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("icc_profile_name", nullptr)); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); // Test copy constructor preserves ICC profile name. - cs->setICCProfileName(profileName); + cs->setInterchangeAttribute("icc_profile_name", profileName); OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); - OCIO_CHECK_EQUAL(std::string(copy->getICCProfileName()), profileName); + OCIO_CHECK_EQUAL(std::string(copy->getInterchangeAttribute("icc_profile_name")), profileName); } OCIO_ADD_TEST(ColorSpace, icc_profile_name_serialization) @@ -2049,7 +2046,7 @@ OCIO_ADD_TEST(ColorSpace, icc_profile_name_serialization) const std::string profileName = "sRGB IEC61966-2.1"; - cs->setICCProfileName(profileName.c_str()); + cs->setInterchangeAttribute("icc_profile_name", profileName.c_str()); cfg->addColorSpace(cs); // Serialize the Config. @@ -2068,19 +2065,15 @@ OCIO_ADD_TEST(ColorSpace, icc_profile_name_serialization) // Verify IccProfileName is preserved. OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); - OCIO_CHECK_EQUAL(std::string(deserializedCs->getICCProfileName()), profileName); + OCIO_CHECK_EQUAL(std::string(deserializedCs->getInterchangeAttribute("icc_profile_name")), profileName); - // verify that that earlier versions reject amf_transform_ids. + // verify that that earlier versions retain amf_transform_ids. OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); cfgCopy->setVersion(2, 4); - OCIO_CHECK_THROW_WHAT( - cfgCopy->serialize(ss), - OCIO::Exception, - "Config failed validation. The color space 'test_colorspace' has non-empty " - "ICCProfileName and config version is less than 2.5."); + OCIO_CHECK_NO_THROW(cfgCopy->serialize(ss)); // Test with empty IccProfileName (should not appear in YAML). - cs->setICCProfileName(nullptr); + cs->setInterchangeAttribute("icc_profile_name", nullptr); cfg->addColorSpace(cs); // replace the existing CS ss.str(""); cfg->serialize(ss); diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index 72767db7b8..4195a48d05 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -43,8 +43,8 @@ def test_copy(self): self.colorspace.addAlias('alias') self.colorspace.addCategory('cat') self.colorspace.setInteropID('ACEScg') - self.colorspace.setAMFTransformIDs('urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3') - self.colorspace.setICCProfileName('sRGB IEC61966-2.1') + self.colorspace.setInterchangeAttribute('amf_transform_ids', 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3') + self.colorspace.setInterchangeAttribute('icc_profile_name', 'sRGB IEC61966-2.1') other = copy.deepcopy(self.colorspace) self.assertFalse(other is self.colorspace) @@ -63,8 +63,8 @@ def test_copy(self): self.assertEqual(list(other.getAliases()), list(self.colorspace.getAliases())) self.assertEqual(list(other.getCategories()), list(self.colorspace.getCategories())) self.assertEqual(other.getInteropID(), self.colorspace.getInteropID()) - self.assertEqual(other.getAMFTransformIDs(), self.colorspace.getAMFTransformIDs()) - self.assertEqual(other.getICCProfileName(), self.colorspace.getICCProfileName()) + self.assertEqual(other.getInterchangeAttribute('amf_transform_ids'), self.colorspace.getInterchangeAttribute('amf_transform_ids')) + self.assertEqual(other.getInterchangeAttribute('icc_profile_name'), self.colorspace.getInterchangeAttribute('icc_profile_name')) def test_allocation(self): """ @@ -286,8 +286,8 @@ def test_constructor_without_parameter(self): self.assertEqual(cs.getAllocation(), OCIO.ALLOCATION_UNIFORM) self.assertEqual(cs.getAllocationVars(), []) self.assertEqual(cs.getInteropID(), '') - self.assertEqual(cs.getAMFTransformIDs(), '') - self.assertEqual(cs.getICCProfileName(), '') + self.assertEqual(cs.getInterchangeAttribute("amf_transform_ids"), '') + self.assertEqual(cs.getInterchangeAttribute("icc_profile_name"), '') def test_data(self): """ @@ -699,101 +699,111 @@ def test_interop_id(self): self.colorspace.setInteropID('space±_name') # Contains ± (ANSI 0xB1) self.assertIn("is invalid: only ASCII characters [0x00..0x7F] are allowed.", str(context.exception)) - def test_amf_transform_ids(self): + def test_interchange_atttributes(self): """ - Test the setAMFTransformIDs() and getAMFTransformIDs() methods. + Test the setInterchangeAttribute() and getInterchangeAttribute() methods. """ - + # amf_transform_ids + # Test default value (should be empty). - self.assertEqual(self.colorspace.getAMFTransformIDs(), '') + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') - # Test setting and getting a single transform ID. + # Test setting and getting a single amf transform ID. single_id = 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3' - self.colorspace.setAMFTransformIDs(single_id) - self.assertEqual(self.colorspace.getAMFTransformIDs(), single_id) + self.colorspace.setInterchangeAttribute('amf_transform_ids', single_id) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), single_id) # Test setting and getting multiple transform IDs (newline-separated). multiple_ids = ('urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3\n' 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_CG.a1.0.3\n' 'urn:ampas:aces:transformId:v1.5:RRT.a1.0.3') - self.colorspace.setAMFTransformIDs(multiple_ids) - self.assertEqual(self.colorspace.getAMFTransformIDs(), multiple_ids) + self.colorspace.setInterchangeAttribute('amf_transform_ids', multiple_ids) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), multiple_ids) # Test setting empty string. - self.colorspace.setAMFTransformIDs('') - self.assertEqual(self.colorspace.getAMFTransformIDs(), '') + self.colorspace.setInterchangeAttribute('amf_transform_ids', '') + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') # Test setting None (should convert to empty string). - self.colorspace.setAMFTransformIDs('something') - self.colorspace.setAMFTransformIDs(None) - self.assertEqual(self.colorspace.getAMFTransformIDs(), '') + self.colorspace.setInterchangeAttribute('amf_transform_ids', 'something') + self.colorspace.setInterchangeAttribute('amf_transform_ids', None) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') # Test with different line endings. mixed_endings = 'id1\nid2\rid3\r\nid4' - self.colorspace.setAMFTransformIDs(mixed_endings) - self.assertEqual(self.colorspace.getAMFTransformIDs(), mixed_endings) + self.colorspace.setInterchangeAttribute('amf_transform_ids', mixed_endings) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), mixed_endings) # Test with leading/trailing whitespace. whitespace_ids = ' \n id1 \n id2 \n ' - self.colorspace.setAMFTransformIDs(whitespace_ids) - self.assertEqual(self.colorspace.getAMFTransformIDs(), whitespace_ids) + self.colorspace.setInterchangeAttribute('amf_transform_ids', whitespace_ids) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), whitespace_ids) # Test wrong type (should raise TypeError). with self.assertRaises(TypeError): - self.colorspace.setAMFTransformIDs(123) + self.colorspace.setInterchangeAttribute('amf_transform_ids', 123) with self.assertRaises(TypeError): - self.colorspace.setAMFTransformIDs(['list', 'of', 'ids']) + self.colorspace.setInterchangeAttribute('amf_transform_ids', ['list', 'of', 'ids']) + + # icc_profile_name - def test_icc_profile_name(self): - """ - Test the setICCProfileName() and getICCProfileName() methods. - """ - # Test default value (should be empty). - self.assertEqual(self.colorspace.getICCProfileName(), '') - + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') + # Test setting and getting a simple profile name. profile_name = 'sRGB IEC61966-2.1' - self.colorspace.setICCProfileName(profile_name) - self.assertEqual(self.colorspace.getICCProfileName(), profile_name) + self.colorspace.setInterchangeAttribute('icc_profile_name', profile_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), profile_name) # Test setting and getting a different profile name. profile_name2 = 'Adobe RGB (1998)' - self.colorspace.setICCProfileName(profile_name2) - self.assertEqual(self.colorspace.getICCProfileName(), profile_name2) + self.colorspace.setInterchangeAttribute('icc_profile_name', profile_name2) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), profile_name2) # Test with a more complex profile name. complex_name = 'Display P3 - Apple Cinema Display (Calibrated 2023-01-15)' - self.colorspace.setICCProfileName(complex_name) - self.assertEqual(self.colorspace.getICCProfileName(), complex_name) + self.colorspace.setInterchangeAttribute('icc_profile_name', complex_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), complex_name) # Test setting empty string. - self.colorspace.setICCProfileName('') - self.assertEqual(self.colorspace.getICCProfileName(), '') + self.colorspace.setInterchangeAttribute('icc_profile_name', '') + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') # Test setting None (should convert to empty string). - self.colorspace.setICCProfileName('something') - self.colorspace.setICCProfileName(None) - self.assertEqual(self.colorspace.getICCProfileName(), '') + self.colorspace.setInterchangeAttribute('icc_profile_name', 'something') + self.colorspace.setInterchangeAttribute('icc_profile_name', None) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') # Test with special characters and numbers. special_name = 'ProPhoto RGB v2.0 (γ=1.8) [Custom Profile #123]' - self.colorspace.setICCProfileName(special_name) - self.assertEqual(self.colorspace.getICCProfileName(), special_name) + self.colorspace.setInterchangeAttribute('icc_profile_name', special_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), special_name) # Test with Unicode characters. unicode_name = 'Профиль RGB γ=2.2' - self.colorspace.setICCProfileName(unicode_name) - self.assertEqual(self.colorspace.getICCProfileName(), unicode_name) + self.colorspace.setInterchangeAttribute('icc_profile_name', unicode_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), unicode_name) # Test wrong type (should raise TypeError). with self.assertRaises(TypeError): - self.colorspace.setICCProfileName(123) + self.colorspace.setInterchangeAttribute('icc_profile_name', 123) with self.assertRaises(TypeError): - self.colorspace.setICCProfileName(['profile', 'name']) + self.colorspace.setInterchangeAttribute('icc_profile_name', ['profile', 'name']) + + # unsupported interchange key + + # test that setting an unsupported key raises. + with self.assertRaises(Exception) as context: + self.colorspace.setInterchangeAttribute('this_should_fail', 'foo42') + self.assertIn("Unknown attribute name 'this_should_fail'", str(context.exception)) + # test that getting an unsupported key raises. + with self.assertRaises(Exception) as context: + self.colorspace.getInterchangeAttribute('this_should_fail') + self.assertIn("Unknown attribute name 'this_should_fail'", str(context.exception)) + def test_processor_to_known_colorspace(self): CONFIG = """ocio_profile_version: 2.5 From a9d45759a1a4a98949ee0e085c91a6d8f0dcb822 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Thu, 11 Sep 2025 15:04:40 -0700 Subject: [PATCH 11/15] - ocioconvert now writes the ColorIntropID attribute of of the output color space. - ociocheck now does extended validity check on the interopID string. - preliminary implementation of getColorSpaceFromInteropID(). Signed-off-by: cuneyt.ozdas --- include/OpenColorIO/OpenColorIO.h | 11 +++ src/OpenColorIO/Config.cpp | 21 +++++ src/apps/ociocheck/main.cpp | 132 ++++++++++++++++++++++-------- src/apps/ocioconvert/main.cpp | 8 ++ 4 files changed, 140 insertions(+), 32 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index b7cb7523c5..ee5724852d 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -622,6 +622,17 @@ class OCIOEXPORT Config */ ConstColorSpaceRcPtr getColorSpace(const char * name) const; + + enum InteropIDSearchMethod : uint8_t + { + FullID, ///< Search only for the full interop ID ([namespace:]colorspace); + NameOnlyFallback ///< If fullID fails, will search for name only (colorspace). + }; + /** + * \brief Get the color space from the provided interopID. + */ + ConstColorSpaceRcPtr getColorSpaceFromInteropID(const char * interopID, InteropIDSearchMethod method=FullID) const; + /** * Accepts an alias, role name, named transform name, or color space name and returns the * color space name or the named transform name. diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 5e894a2f7c..4c2a1333f9 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -2497,6 +2497,27 @@ const char * Config::getCanonicalName(const char * name) const return ""; } +ConstColorSpaceRcPtr Config::getColorSpaceFromInteropID(const char * interopID, InteropIDSearchMethod method) const +{ + // TODO: this currently searches for roles as well. Do we want that? /coz + + auto cs = getColorSpace(interopID); + + // Fall back to name-only if allowed. + if(!cs && method == NameOnlyFallback) + { + std::string id(interopID); + size_t pos = id.find(':'); + if(pos != std::string::npos) + { + std::string csname = id.substr(pos+1); + cs = getColorSpace(id.c_str()); + } + } + + return cs; +} + int Config::getNumColorSpaces() const { return getNumColorSpaces(SEARCH_REFERENCE_SPACE_ALL, COLORSPACE_ACTIVE); diff --git a/src/apps/ociocheck/main.cpp b/src/apps/ociocheck/main.cpp index 6db3b0cf54..06683a211a 100644 --- a/src/apps/ociocheck/main.cpp +++ b/src/apps/ociocheck/main.cpp @@ -27,6 +27,103 @@ const char * DESC_STRING = "\n\n" "Ociocheck can also be used to clean up formatting on an existing profile\n" "that has been manually edited, using the '-o' option.\n"; + +// returns true if the interopID is valid +bool isValidInteropID(const std::string& id) +{ + static const std::set cifTextureIDs = { + "lin_ap1_scene", + "lin_ap0_scene", + "lin_rec709_scene", + "lin_p3d65_scene", + "lin_rec2020_scene", + "lin_adobergb_scene", + "lin_ciexyzd65_scene", + "srgb_rec709_scene", + "g22_rec709_scene", + "g18_rec709_scene", + "srgb_ap1_scene", + "g22_ap1_scene", + "srgb_p3d65_scene", + "g22_adobergb_scene", + "data", + "unknown" + }; + + // empty is fine + if (id.empty()) + return true; + + // check if it only uses ASCII characters: 0-9, a-z, and the following characters (no spaces): + // - _ ~ / * # % ^ + ( ) [ ] | + auto allowed = [](char c) + { + return (c >= '0' && c <= '9') || + (c >= 'a' && c <= 'z') || + c=='-'||c=='_'||c=='~'||c=='/'||c=='*'||c=='#'||c=='%'|| + c=='^'||c=='+'||c=='('||c==')'||c=='['||c==']'||c=='|' || c==':'; + }; + + if (!std::all_of(id.begin(), id.end(), allowed)) + { + std::cout << "ERROR: InteropID '" << id << "' contains invalid characters. " + "Only lowercase a-z, 0-9 and - _ ~ / * # % ^ + ( ) [ ] | are allowed." << + std::endl; + return false; + } + + // Check if has a namespace. + size_t pos = id.find(':'); + if (pos == std::string::npos) + { + // No namespace, so id must be in the forumID list. + if (cifTextureIDs.count(id) == 0) + { + std::cout << "ERROR: InteropID '" << id << "' is not valid. " + "It should either be one of Color Interop Forum standard IDs or " + "it must contain a namespace followed by ':', e.g. 'mycompany:mycolorspace'." << + std::endl; + return false; + } + } + else + { + // Namespace found, split into namespace and id. + std::string ns = id.substr(0, pos); + std::string cs = id.substr(pos+1); + + // both should be non-empty + if (ns.empty() || cs.empty()) + { + std::cout << "ERROR: InteropID '" << id << "' is not valid. " + "If a namespace is used, it must be followed by a non-empty ID, e.g. 'mycompany:mycolorspace'." << + std::endl; + return false; + } + + // More than one ':' is an error. + if (cs.find(':') != std::string::npos) + { + std::cout << "ERROR: InteropID '" << id << "' is not valid. " + "Only one ':' is allowed to separate the namespace and ID, e.g. 'mycompany:mycolorspace'." << + std::endl; + return false; + } + + // id should not be in the cifID list. + if (cifTextureIDs.count(cs) > 0) + { + std::cout << "ERROR: InteropID '" << id << "' is not valid. " + "The ID part must not be one of the Color Interop Forum standard IDs when a namespace is used." << + std::endl; + return false; + } + } + + // all clear. + return true; +} + int main(int argc, const char **argv) { bool help = false; @@ -288,25 +385,7 @@ int main(int argc, const char **argv) std::cout << std::endl; std::cout << "** ColorSpaces **" << std::endl; - // Valid Color Interop Forum IDs - std::set validInteropIDs = { - "lin_ap1_scene", - "lin_ap0_scene", - "lin_rec709_scene", - "lin_p3d65_scene", - "lin_rec2020_scene", - "lin_adobergb_scene", - "lin_ciexyzd65_scene", - "srgb_rec709_scene", - "g22_rec709_scene", - "g18_rec709_scene", - "srgb_ap1_scene", - "g22_ap1_scene", - "srgb_p3d65_scene", - "g22_adobergb_scene", - "data", - "unknown" - }; + const int numCS = config->getNumColorSpaces( OCIO::SEARCH_REFERENCE_SPACE_ALL, // Iterate over scene & display color spaces. @@ -327,21 +406,10 @@ int main(int argc, const char **argv) std::string interopID = cs->getInteropID(); if (!interopID.empty()) { - // Check if the interopID contains a colon - if (interopID.find(':') == std::string::npos) + if (!isValidInteropID(interopID)) { - // No colon found, check if it's a valid Color Interop Forum ID (case insensitive) - std::string lowerInteropID = interopID; - std::transform(lowerInteropID.begin(), lowerInteropID.end(), lowerInteropID.begin(), ::tolower); - - if (validInteropIDs.find(lowerInteropID) == validInteropIDs.end()) - { - std::cout << "WARNING: Color space '" << cs->getName() - << "' has unknown interop_id '" << interopID - << "'. Expected one of the Color Interop Forum standard IDs or a namespaced ID with ':'." << std::endl; - } + errorcount += 1; } - // If it contains a colon, it's assumed to be a namespaced ID and is valid } // Try to load the transform for the to_ref direction -- this will load any LUTs. diff --git a/src/apps/ocioconvert/main.cpp b/src/apps/ocioconvert/main.cpp index e8071c76fa..31a5ed3542 100644 --- a/src/apps/ocioconvert/main.cpp +++ b/src/apps/ocioconvert/main.cpp @@ -658,6 +658,14 @@ int main(int argc, const char **argv) if (outputcolorspace) { imgOutput->attribute("oiio:ColorSpace", outputcolorspace); + + // Set the color space interopID if available. + auto cs = config->getColorSpace(outputcolorspace); + const char* interopID = cs ? cs->getInteropID() : nullptr; + if(interopID && *interopID) + { + imgOutput->attribute("colorInteropID", interopID); + } } imgOutput->write(outputimage, userOutputBitDepth); From 3ea30088921f4a4a25aeccd33340f3c8f83b3c2c Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Fri, 12 Sep 2025 17:00:36 -0700 Subject: [PATCH 12/15] - Another cleanup pass. After some deliberation, we decided to remove getColorSpaceFromInteropID() which will be done in a later pass. Signed-off-by: cuneyt.ozdas --- include/OpenColorIO/OpenColorIO.h | 21 +- src/OpenColorIO/ColorSpace.cpp | 87 +-- src/OpenColorIO/Config.cpp | 15 +- src/OpenColorIO/OCIOYaml.cpp | 18 +- src/apps/ociocheck/main.cpp | 69 +-- src/bindings/python/PyConfig.cpp | 2 + src/libutils/imageioapphelpers/imageio.cpp | 3 + tests/cpu/ColorSpace_tests.cpp | 661 ++++++++++++--------- tests/python/ColorSpaceTest.py | 330 +++++----- 9 files changed, 656 insertions(+), 550 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index ee5724852d..9a9bc792d8 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -631,7 +631,7 @@ class OCIOEXPORT Config /** * \brief Get the color space from the provided interopID. */ - ConstColorSpaceRcPtr getColorSpaceFromInteropID(const char * interopID, InteropIDSearchMethod method=FullID) const; + ConstColorSpaceRcPtr getColorSpaceFromInteropID(const char * interopID, InteropIDSearchMethod method) const; /** * Accepts an alias, role name, named transform name, or color space name and returns the @@ -1992,13 +1992,16 @@ class OCIOEXPORT ColorSpace void setDescription(const char * description); /** - * Get/Set the interop ID for the color space. - * - * The interop ID is a standardized identifier to uniquely identify commonly - * used color spaces. These IDs are defined by the Academy Software - * Foundation's Color Interop Forum project. If you create your own ID, you - * must prefix it with unique characters that will ensure it won't conflict - * with future Color Interop Forum IDs. + * Get/Set the interop ID for the color space. The interop ID is a + * structured string defined by the Color Interop Forum. It is intended to + * identify color spaces in a way that is portable across different configs, + * making it suitable for use in various file formats. The Color Interop + * Forum publishes ID strings for common color spaces. If you create your + * own IDs, they must be preceded by a namespace string. The setter will + * throw if the string does not follow certain rules (run ociocheck for a + * more complete validation). For more details, please review the interop ID + * white paper available from: + * https://github.com/AcademySoftwareFoundation/ColorInterop. */ const char * getInteropID() const noexcept; void setInteropID(const char * interopID); @@ -2008,7 +2011,7 @@ class OCIOEXPORT ColorSpace * * Currently supported attribute names are "amf_transform_ids" and * "icc_profile_name". Using any other name will throw. If the attribute is - * not defined, it'll return empty string. Similarly setting it to empty + * not defined, it'll return empty string. Setting the value to an empty * string will effectively delete the attribute. * * The AMF transform IDs are used to identify specific transforms in the diff --git a/src/OpenColorIO/ColorSpace.cpp b/src/OpenColorIO/ColorSpace.cpp index e1ca36926e..70b49dbda7 100644 --- a/src/OpenColorIO/ColorSpace.cpp +++ b/src/OpenColorIO/ColorSpace.cpp @@ -241,43 +241,53 @@ void ColorSpace::setInteropID(const char * interopID) if (!id.empty()) { - // Count the number of ':' characters in the string - // and check for non-ASCII characters - size_t colonCount = 0; - size_t lastColonPos = std::string::npos; - - for (size_t i = 0; i < id.length(); ++i) + // check if it only uses ASCII characters: 0-9, a-z, and the following characters (no spaces): + // - _ ~ / * # % ^ + ( ) [ ] | + auto allowed = [](char c) + { + return (c >= '0' && c <= '9')|| + (c >= 'a' && c <= 'z')|| + c=='.'||c=='-'||c=='_'||c=='~'||c=='/'||c=='*'||c=='#'||c=='%'|| + c=='^'||c=='+'||c=='('||c==')'||c=='['||c==']'||c=='|'||c==':'; + }; + + if (!std::all_of(id.begin(), id.end(), allowed)) { - if (id[i] == ':') + std::ostringstream oss; + oss << "InteropID '" << id << "' contains invalid characters. " + "Only lowercase a-z, 0-9 and . - _ ~ / * # % ^ + ( ) [ ] | are allowed." << + std::endl; + throw Exception(oss.str().c_str()); + } + + // Check if has a namespace. + size_t pos = id.find(':'); + if (pos != std::string::npos) + { + // Namespace found, split into namespace and color space. + std::string ns = id.substr(0, pos); + std::string cs = id.substr(pos+1); + + // both should be non-empty + if (ns.empty() || cs.empty()) { - colonCount++; - lastColonPos = i; + std::ostringstream oss; + oss << "InteropID '" << id << "' is not valid. " + "If ':' is used, both the namespace and the color space parts must be non-empty." << + std::endl; + throw Exception(oss.str().c_str()); } - else if (static_cast(id[i]) >= 0x80) + + // More than one ':' is an error. + if (cs.find(':') != std::string::npos) { std::ostringstream oss; - oss << "InteropID '" << id << "' is invalid: only ASCII characters [0x00..0x7F] are allowed."; + oss << "ERROR: InteropID '" << id << "' is not valid. " + "Only one ':' is allowed to separate the namespace and the color space." << + std::endl; throw Exception(oss.str().c_str()); } } - - // Validate: only zero or one ':' character allowed - if (colonCount > 1) - { - std::ostringstream oss; - oss << "InteropID '" << id << "' is invalid: only zero or one ':' character is allowed."; - throw Exception(oss.str().c_str()); - } - - // Validate: ':' cannot be the last character - if (colonCount == 1 && lastColonPos == id.length() - 1) - { - std::ostringstream oss; - oss << "InteropID '" << id << "' is invalid: ':' character cannot be the last character."; - throw Exception(oss.str().c_str()); - } - - // TODO: Do we want to verify the interopID against the CIF list here? } getImpl()->m_interopID = id; @@ -285,11 +295,12 @@ void ColorSpace::setInteropID(const char * interopID) const char * ColorSpace::getInterchangeAttribute(const char* attrName) const { - std::string nameTrimmed = StringUtils::Trim(attrName); + std::string name = attrName ? attrName : ""; + for (auto& key : knownInterchangeNames) { // do case-insensitive comparison. - if (StringUtils::Compare(key, nameTrimmed)) + if (StringUtils::Compare(key, name)) { auto it = m_impl->m_interchangeAttribs.find(key); if (it != m_impl->m_interchangeAttribs.end()) @@ -301,19 +312,20 @@ const char * ColorSpace::getInterchangeAttribute(const char* attrName) const } std::ostringstream oss; - oss << "Unknown attribute name '" << attrName << "'."; + oss << "Unknown attribute name '" << name << "'."; throw Exception(oss.str().c_str()); } void ColorSpace::setInterchangeAttribute(const char* attrName, const char* value) { - std::string nameTrimmed = StringUtils::Trim(attrName); + std::string name = attrName ? attrName : ""; + for (auto& key : knownInterchangeNames) { // Do case-insensitive comparison. - if (StringUtils::Compare(key, nameTrimmed)) + if (StringUtils::Compare(key, name)) { - // use key instead of nameTrim for storing in correct capitalization. + // use key instead of name for storing in correct capitalization. if (!value || !*value) { m_impl->m_interchangeAttribs.erase(key); @@ -327,7 +339,7 @@ void ColorSpace::setInterchangeAttribute(const char* attrName, const char* value } std::ostringstream oss; - oss << "Unknown attribute name '" << attrName << "'."; + oss << "Unknown attribute name '" << name << "'."; throw Exception(oss.str().c_str()); } @@ -336,7 +348,7 @@ std::map ColorSpace::getInterchangeAttributes() const return m_impl->m_interchangeAttribs; } -BitDepth ColorSpace::ColorSpace::getBitDepth() const noexcept +BitDepth ColorSpace::getBitDepth() const noexcept { return getImpl()->m_bitDepth; } @@ -555,7 +567,6 @@ std::ostream & operator<< (std::ostream & os, const ColorSpace & cs) { os << ", description=" << str; } - // TODO: Do we need to output the section name too? for (const auto& attr : cs.getInterchangeAttributes()) { os << ", " << attr.first << "=" << attr.second; diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 4c2a1333f9..69aeeed433 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -2499,8 +2499,7 @@ const char * Config::getCanonicalName(const char * name) const ConstColorSpaceRcPtr Config::getColorSpaceFromInteropID(const char * interopID, InteropIDSearchMethod method) const { - // TODO: this currently searches for roles as well. Do we want that? /coz - + // NOTE: This will search color space names, aliases, and roles. auto cs = getColorSpace(interopID); // Fall back to name-only if allowed. @@ -5682,11 +5681,13 @@ void Config::Impl::checkVersionConsistency() const } } - // Check for the ColorSpaces. + // Check ColorSpace properties. const int nbCS = m_allColorSpaces->getNumColorSpaces(); for (int i = 0; i < nbCS; ++i) { + // Check for display color spaces. + const auto & cs = m_allColorSpaces->getColorSpaceByIndex(i); if (m_majorVersion < 2) { @@ -5696,6 +5697,8 @@ void Config::Impl::checkVersionConsistency() const } } + // Check for new color space attributes. + if (m_majorVersion < 2) { if (*cs->getInteropID()) @@ -5705,11 +5708,15 @@ void Config::Impl::checkVersionConsistency() const os << "has non-empty InteropID and config version is less than 2.0."; throw Exception(os.str().c_str()); } + } + + if (hexVersion < 0x02050000) + { if (cs->getInterchangeAttributes().size()>0) { std::ostringstream os; os << "Config failed validation. The color space '" << cs->getName() << "' "; - os << "has non-empty interchange attributes and config version is less than 2.0."; + os << "has non-empty interchange attributes and config version is less than 2.5."; throw Exception(os.str().c_str()); } } diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 29ab436bbe..3d2fc66e9f 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -344,7 +344,7 @@ inline void loadCustomKeys(const YAML::Node& node, CustomKeysLoader & ck, const else { std::ostringstream ss; - ss << "The '" << sectionName << "' section need to be a YAML map."; + ss << "Expected a YAML map in the " << sectionName << " section."; throwError(node, ss.str().c_str()); } } @@ -3259,7 +3259,7 @@ inline void load(const YAML::Node& node, ColorSpaceRcPtr& cs, unsigned int major keyval.first.as().c_str(), val.c_str()); } - catch (Exception*) + catch (Exception &) { LogUnknownKeyWarning(iter->second, keyval.first); } @@ -3381,13 +3381,6 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major } out << YAML::Flow << YAML::Value << aliases; } - - const std::string interopID{ cs->getInteropID() }; - if (!interopID.empty()) - { - out << YAML::Key << "interop_id"; - out << YAML::Value << interopID; - } out << YAML::Key << "family" << YAML::Value << cs->getFamily(); @@ -3451,6 +3444,13 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major out << YAML::Key << "encoding"; out << YAML::Value << is; } + + const std::string interopID{ cs->getInteropID() }; + if (!interopID.empty()) + { + out << YAML::Key << "interop_id"; + out << YAML::Value << interopID; + } out << YAML::Key << "allocation" << YAML::Value; save(out, cs->getAllocation()); diff --git a/src/apps/ociocheck/main.cpp b/src/apps/ociocheck/main.cpp index 06683a211a..5c66a7eba0 100644 --- a/src/apps/ociocheck/main.cpp +++ b/src/apps/ociocheck/main.cpp @@ -31,6 +31,8 @@ const char * DESC_STRING = "\n\n" // returns true if the interopID is valid bool isValidInteropID(const std::string& id) { + // See https://github.com/AcademySoftwareFoundation/ColorInterop for the details. + static const std::set cifTextureIDs = { "lin_ap1_scene", "lin_ap0_scene", @@ -50,34 +52,30 @@ bool isValidInteropID(const std::string& id) "unknown" }; - // empty is fine - if (id.empty()) - return true; - - // check if it only uses ASCII characters: 0-9, a-z, and the following characters (no spaces): - // - _ ~ / * # % ^ + ( ) [ ] | - auto allowed = [](char c) - { - return (c >= '0' && c <= '9') || - (c >= 'a' && c <= 'z') || - c=='-'||c=='_'||c=='~'||c=='/'||c=='*'||c=='#'||c=='%'|| - c=='^'||c=='+'||c=='('||c==')'||c=='['||c==']'||c=='|' || c==':'; + static const std::set cifDisplayIDs = { + "srgb_rec709_display", + "g24_rec709_display", + "srgb_p3d65_display", + "srgbx_p3d65_display", + "pq_p3d65_display", + "pq_rec2020_display", + "hlg_rec2020_display", + "g22_rec709_display", + "g22_adobergb_display", + "g26_p3d65_display", + "g26_xyzd65_display", + "pq_xyzd65_display", }; - if (!std::all_of(id.begin(), id.end(), allowed)) - { - std::cout << "ERROR: InteropID '" << id << "' contains invalid characters. " - "Only lowercase a-z, 0-9 and - _ ~ / * # % ^ + ( ) [ ] | are allowed." << - std::endl; - return false; - } + if (id.empty()) + return true; // Check if has a namespace. size_t pos = id.find(':'); if (pos == std::string::npos) { - // No namespace, so id must be in the forumID list. - if (cifTextureIDs.count(id) == 0) + // No namespace, so id must be in the Color Interop Forum ID list. + if (cifTextureIDs.count(id) == 0 && cifDisplayIDs.count(id)==0) { std::cout << "ERROR: InteropID '" << id << "' is not valid. " "It should either be one of Color Interop Forum standard IDs or " @@ -91,27 +89,9 @@ bool isValidInteropID(const std::string& id) // Namespace found, split into namespace and id. std::string ns = id.substr(0, pos); std::string cs = id.substr(pos+1); - - // both should be non-empty - if (ns.empty() || cs.empty()) - { - std::cout << "ERROR: InteropID '" << id << "' is not valid. " - "If a namespace is used, it must be followed by a non-empty ID, e.g. 'mycompany:mycolorspace'." << - std::endl; - return false; - } - // More than one ':' is an error. - if (cs.find(':') != std::string::npos) - { - std::cout << "ERROR: InteropID '" << id << "' is not valid. " - "Only one ':' is allowed to separate the namespace and ID, e.g. 'mycompany:mycolorspace'." << - std::endl; - return false; - } - - // id should not be in the cifID list. - if (cifTextureIDs.count(cs) > 0) + // Id should not be in the Color Interop Forum ID list. + if (cifTextureIDs.count(cs) > 0 || cifDisplayIDs.count(cs)> 0) { std::cout << "ERROR: InteropID '" << id << "' is not valid. " "The ID part must not be one of the Color Interop Forum standard IDs when a namespace is used." << @@ -385,8 +365,6 @@ int main(int argc, const char **argv) std::cout << std::endl; std::cout << "** ColorSpaces **" << std::endl; - - const int numCS = config->getNumColorSpaces( OCIO::SEARCH_REFERENCE_SPACE_ALL, // Iterate over scene & display color spaces. OCIO::COLORSPACE_ALL); // Iterate over active & inactive color spaces. @@ -398,11 +376,6 @@ int main(int argc, const char **argv) OCIO::COLORSPACE_ALL, i)); - // Validate InteropID if present - // TODO: When the CIF list for display color spaces is ready consider - // splitting this validation into two parts so that display color spaces - // are tested against the display CIF list. - std::string interopID = cs->getInteropID(); if (!interopID.empty()) { diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index 104d017f89..706734b8e5 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -316,6 +316,8 @@ void bindPyConfig(py::module & m) { return ActiveColorSpaceIterator(self); }) + .def("getColorSpaceFromInteropID", &Config::getColorSpaceFromInteropID, "interopID"_a, "searchMethod"_a, + DOC(Config, getColorSpaceFromInteropID)) .def("getCanonicalName", &Config::getCanonicalName, "name"_a, DOC(Config, getCanonicalName)) .def("addColorSpace", &Config::addColorSpace, "colorSpace"_a, diff --git a/src/libutils/imageioapphelpers/imageio.cpp b/src/libutils/imageioapphelpers/imageio.cpp index b1701f8909..cda93b3e3a 100644 --- a/src/libutils/imageioapphelpers/imageio.cpp +++ b/src/libutils/imageioapphelpers/imageio.cpp @@ -192,6 +192,9 @@ ptrdiff_t ImageIO::getImageBytes() const void ImageIO::init(const ImageIO & img, BitDepth bitDepth) { m_impl->init(*img.m_impl, bitDepth); + + // Do not propagate colorInteropID. + attribute("colorInteropID", "unknown"); } void ImageIO::init(long width, long height, ChannelOrdering chanOrder, BitDepth bitDepth) diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index 3722a5c169..ceb9807039 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -47,7 +47,7 @@ OCIO_ADD_TEST(ColorSpace, basic) cs->setFamily("FAMILY"); cs->setEqualityGroup("EQGRP"); cs->setEncoding("ENC"); - cs->setInteropID("INTEROP"); + cs->setInteropID("interop"); OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("amf_transform_ids", "AMF")); OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("icc_profile_name", "ICC")); @@ -684,13 +684,13 @@ active_views: [] OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), "data"); } - // Test that the amf_transform_ids is valid in v2.0 config too. + // Test that the interchange is NOT valid in v2.0 config. { constexpr char End[]{R"(colorspaces: - ! name: raw interchange: - amf_transform_ids: should be valid in 2.0 config too + amf_transform_ids: should NOT be valid in 2.0 config family: raw equalitygroup: "" bitdepth: 32f @@ -701,19 +701,77 @@ active_views: [] std::string cfgString{Start}; cfgString += End; + std::istringstream is; + is.str(cfgString); + OCIO::ConstConfigRcPtr config; + OCIO_CHECK_THROW_WHAT( + config = OCIO::Config::CreateFromStream(is), + OCIO::Exception, + "Config failed validation. The color space 'raw' has non-empty " + "interchange attributes and config version is less than 2.5."); + } + + // Interchange tests in 2.5 + constexpr char Start2_5[]{R"(ocio_profile_version: 2.5 + +environment: + {} +search_path: "" +strictparsing: false +luma: [0.2126, 0.7152, 0.0722] + +roles: + default: raw + +file_rules: + - ! {name: ColorSpaceNamePathSearch} + - ! {name: Default, colorspace: default} + +displays: + sRGB: + - ! {name: Raw, colorspace: raw} + +active_displays: [] +active_views: [] + +)"}; + + // Test that the interchange is valid in v2.5 config. + { + constexpr char End_amf[]{R"( +colorspaces: + - ! + name: raw + interchange: + amf_transform_ids: This is valid in 2.5 config + family: raw + equalitygroup: "" + bitdepth: 32f + description: Some text. + isdata: true + allocation: uniform +)"}; + + std::string cfgString{Start2_5}; + cfgString += End_amf; + std::istringstream is; is.str(cfgString); OCIO::ConstConfigRcPtr config; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_REQUIRE_EQUAL(!config, false); + auto attrMap = config->getColorSpace("raw")->getInterchangeAttributes(); + OCIO_CHECK_EQUAL(attrMap.size(), 1); } - // Test that the icc_profile_name is valid in v2.0 config too. + // Test that the unknown interchange attrib will be ignored in 2.5. { - constexpr char End[]{R"(colorspaces: + constexpr char End_unkown[]{R"( +colorspaces: - ! name: raw interchange: - icc_profile_name: should be valid in 2.0 config too + unknown: will be ignored family: raw equalitygroup: "" bitdepth: 32f @@ -721,13 +779,17 @@ active_views: [] isdata: true allocation: uniform )"}; - std::string cfgString{Start}; - cfgString += End; + + std::string cfgString{Start2_5}; + cfgString += End_unkown; std::istringstream is; is.str(cfgString); OCIO::ConstConfigRcPtr config; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_REQUIRE_EQUAL(!config, false); + auto attrMap = config->getColorSpace("raw")->getInterchangeAttributes(); + OCIO_CHECK_EQUAL(attrMap.size(), 0); } } @@ -844,6 +906,320 @@ active_views: [] "colorspace"); } +OCIO_ADD_TEST(ColorSpace, interop_id) +{ + OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); + + // Test default value. + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); + + // Test setting and getting single profile name. + const char * interop_id = "srgb_p3d65_scene"; + cs->setInteropID(interop_id); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), interop_id); + + // Test setting empty string. + cs->setInteropID(""); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); + + // Test setting and getting another value. + const char * anotherID= "lin_rec2020_scene"; + cs->setInteropID(anotherID); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), anotherID); + + // Test setting null pointer (should be safe). + cs->setInteropID("something"); + OCIO_CHECK_NO_THROW(cs->setInteropID(nullptr)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); + + // Test copy constructor preserves InteropID. + cs->setInteropID(interop_id); + OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); + OCIO_CHECK_EQUAL(std::string(copy->getInteropID()), interop_id); + + // Test valid InteropID with colon in the middle. + const char * validColonMiddle = "namespace:colorspace_name"; + OCIO_CHECK_NO_THROW(cs->setInteropID(validColonMiddle)); + OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), validColonMiddle); + + // Test invalid InteropID with multiple colons. + OCIO_CHECK_THROW_WHAT(cs->setInteropID("name:space:cs_name"), OCIO::Exception, + "Only one ':' is allowed to separate the namespace and the color space."); + + // Test invalid InteropID with colon at the end. + OCIO_CHECK_THROW_WHAT(cs->setInteropID("namespace:"), OCIO::Exception, + " If ':' is used, both the namespace and the color space parts must be non-empty."); + + // Test invalid InteropID with empty namespace or color space. + OCIO_CHECK_THROW_WHAT(cs->setInteropID(":cs_name"), OCIO::Exception, + "If ':' is used, both the namespace and the color space parts must be non-empty."); + + // Test invalid InteropID with illegal characters. + OCIO_CHECK_THROW_WHAT(cs->setInteropID("café_scene"), OCIO::Exception, + "Only lowercase a-z, 0-9 and . - _ ~ / * # % ^ + ( ) [ ] | are allowed."); + + OCIO_CHECK_THROW_WHAT(cs->setInteropID("UPPERCASE"), OCIO::Exception, + "Only lowercase a-z, 0-9 and . - _ ~ / * # % ^ + ( ) [ ] | are allowed."); + + OCIO_CHECK_THROW_WHAT(cs->setInteropID("{curly_bracket}"), OCIO::Exception, + "Only lowercase a-z, 0-9 and . - _ ~ / * # % ^ + ( ) [ ] | are allowed."); + + OCIO_CHECK_THROW_WHAT(cs->setInteropID("\\backslash"), OCIO::Exception, + "Only lowercase a-z, 0-9 and . - _ ~ / * # % ^ + ( ) [ ] | are allowed."); + + OCIO_CHECK_THROW_WHAT(cs->setInteropID(" space "), OCIO::Exception, + "Only lowercase a-z, 0-9 and . - _ ~ / * # % ^ + ( ) [ ] | are allowed."); +} + +OCIO_ADD_TEST(ColorSpace, interop_id_serialization) +{ + // Test YAML serialization and deserialization of InteropID. + auto cfg = OCIO::Config::Create(); + auto cs = OCIO::ColorSpace::Create(); + cs->setName("test_colorspace"); + + const std::string interop_id = "lin_rec709_scene"; + + cs->setInteropID(interop_id.c_str()); + cfg->addColorSpace(cs); + + // Serialize the Config. + std::stringstream ss; + cfg->serialize(ss); + std::string yamlStr = ss.str(); + + // Verify interop_id appears in YAML. + OCIO_CHECK_NE(yamlStr.find("interop_id"), std::string::npos); + OCIO_CHECK_NE(yamlStr.find(interop_id), std::string::npos); + + // Deserialize and verify. + std::istringstream iss(yamlStr); + OCIO::ConstConfigRcPtr deserializedCfg; + OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); + + // Verify interop_id is preserved. + OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); + OCIO_CHECK_EQUAL(std::string(deserializedCs->getInteropID()), interop_id); + + // verify that that versions earlier than 2.0 reject interop_id. + OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); + cfgCopy->setVersion(2, 0); + OCIO_CHECK_NO_THROW(cfgCopy->serialize(ss)); + + cfgCopy->setVersion(1, 0); + OCIO_CHECK_THROW_WHAT( + cfgCopy->serialize(ss), + OCIO::Exception, + "Config failed validation. The color space 'test_colorspace' has non-empty " + "InteropID and config version is less than 2.0."); + + // Test with empty interop_id (should not appear in YAML). + cs->setInteropID(nullptr); + cfg->addColorSpace(cs); // Replace the existing CS. + ss.str(""); + cfg->serialize(ss); + std::string yamlStr2 = ss.str(); + + // Verify empty imterop_id does not appear in YAML. + OCIO_CHECK_EQUAL(yamlStr2.find("interop_id"), std::string::npos); +} + +OCIO_ADD_TEST(ColorSpace, amf_transform_ids) +{ + OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); + + // Test default value. + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); + + // Test setting and getting single ID. + const char * singleID = "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3"; + cs->setInterchangeAttribute("amf_transform_ids", singleID); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), singleID); + + // Test setting to empty string. + cs->setInterchangeAttribute("amf_transform_ids", ""); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); + + // Test setting and getting multiple IDs. + const char * multipleIDs = + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; + cs->setInterchangeAttribute("amf_transform_ids", multipleIDs); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), multipleIDs); + + // Test setting to null pointer (should be safe). + cs->setInterchangeAttribute("amf_transform_ids", "something"); + cs->setInterchangeAttribute("amf_transform_ids", nullptr); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); + + // Test copy constructor preserves AMF transform IDs. + cs->setInterchangeAttribute("amf_transform_ids", singleID); + OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); + OCIO_CHECK_EQUAL(std::string(copy->getInterchangeAttribute("amf_transform_ids")), singleID); +} + +OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) +{ + // Test YAML serialization and deserialization of AmfTransformIDs. + auto cfg = OCIO::Config::Create(); + auto cs = OCIO::ColorSpace::Create(); + cs->setName("test_colorspace"); + + const std::string amfIDs = + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" + "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; + + cs->setInterchangeAttribute("amf_transform_ids", amfIDs.c_str()); + cfg->addColorSpace(cs); + + // Serialize the Config. + std::stringstream ss; + cfg->serialize(ss); + std::string yamlStr = ss.str(); + + // Verify AmfTransformIDs appears in YAML. + OCIO_CHECK_NE(yamlStr.find("amf_transform_ids"), std::string::npos); + OCIO_CHECK_NE(yamlStr.find("ACEScsc.Academy.ACEScc_to_ACES"), std::string::npos); + + // Deserialize and verify. + std::istringstream iss(yamlStr); + OCIO::ConstConfigRcPtr deserializedCfg; + OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); + + // Verify AmfTransformIDs is preserved. + OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); + auto deserializedIDs = deserializedCs->getInterchangeAttribute("amf_transform_ids"); + OCIO_CHECK_EQUAL(deserializedIDs, amfIDs); + + // Verify that that earlier versions retain amf_transform_ids. + OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); + cfgCopy->setVersion(2,4); + OCIO_CHECK_THROW_WHAT(cfgCopy->serialize(ss), + OCIO::Exception, + "has non-empty interchange attributes and config version is less than 2.5."); + + // Test with empty AmfTransformIDs (should not appear in YAML). + cs->setInterchangeAttribute("amf_transform_ids", nullptr); + cfg->addColorSpace(cs); // Replace the existing CS. + ss.str(""); + cfg->serialize(ss); + std::string yamlStr2 = ss.str(); + + // Verify empty AmfTransformIDs does not appear in YAML. + OCIO_CHECK_EQUAL(yamlStr2.find("amf_transform_ids"), std::string::npos); +} + +OCIO_ADD_TEST(ColorSpace, icc_profile_name) +{ + OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); + + // Test default value. + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); + + // Test setting and getting single profile name. + const char * profileName = "sRGB IEC61966-2.1"; + cs->setInterchangeAttribute("icc_profile_name",profileName); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), profileName); + + // Test setting and getting another profile name. + const char * anotherProfile = "Adobe RGB (1998)"; + cs->setInterchangeAttribute("icc_profile_name", anotherProfile); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), anotherProfile); + + // Test setting empty string. + cs->setInterchangeAttribute("icc_profile_name", ""); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); + + // Test setting null pointer (should be safe). + cs->setInterchangeAttribute("icc_profile_name", "something"); + OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("icc_profile_name", nullptr)); + OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); + + // Test copy constructor preserves ICC profile name. + cs->setInterchangeAttribute("icc_profile_name", profileName); + OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); + OCIO_CHECK_EQUAL(std::string(copy->getInterchangeAttribute("icc_profile_name")), profileName); +} + +OCIO_ADD_TEST(ColorSpace, icc_profile_name_serialization) +{ + // Test YAML serialization and deserialization of IccProfileName. + auto cfg = OCIO::Config::Create(); + auto cs = OCIO::ColorSpace::Create(); + cs->setName("test_colorspace"); + + const std::string profileName = "sRGB IEC61966-2.1"; + + cs->setInterchangeAttribute("icc_profile_name", profileName.c_str()); + cfg->addColorSpace(cs); + + // Serialize the Config. + std::stringstream ss; + cfg->serialize(ss); + std::string yamlStr = ss.str(); + + // Verify IccProfileName appears in YAML. + OCIO_CHECK_NE(yamlStr.find("icc_profile_name"), std::string::npos); + OCIO_CHECK_NE(yamlStr.find(profileName), std::string::npos); + + // Deserialize and verify. + std::istringstream iss(yamlStr); + OCIO::ConstConfigRcPtr deserializedCfg; + OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); + + // Verify IccProfileName is preserved. + OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); + OCIO_CHECK_EQUAL(std::string(deserializedCs->getInterchangeAttribute("icc_profile_name")), profileName); + + // verify that that earlier versions reject icc_profile_name. + OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); + cfgCopy->setVersion(2, 4); + OCIO_CHECK_THROW_WHAT(cfgCopy->serialize(ss), + OCIO::Exception, + "has non-empty interchange attributes and config version is less than 2.5."); + + // Test with empty IccProfileName (should be valid in 2.4 and should not appear in YAML). + cs->setInterchangeAttribute("icc_profile_name", nullptr); + cfg->addColorSpace(cs); // replace the existing CS + ss.str(""); + cfg->serialize(ss); + std::string yamlStr2 = ss.str(); + + // Verify empty IccProfileName does not appear in YAML. + OCIO_CHECK_EQUAL(yamlStr2.find("icc_profile_name"), std::string::npos); +} + +OCIO_ADD_TEST(ColorSpace, unknown_interchange_attrib) +{ + OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); + + // Getting should throw. + OCIO_CHECK_THROW_WHAT(std::string(cs->getInterchangeAttribute("unknown_attrib")), + OCIO::Exception, + "Unknown attribute name"); + + // Empty name is not legal. + OCIO_CHECK_THROW_WHAT(std::string(cs->getInterchangeAttribute("")), OCIO::Exception, + "Unknown attribute name"); + OCIO_CHECK_THROW_WHAT(std::string(cs->getInterchangeAttribute(nullptr)), OCIO::Exception, + "Unknown attribute name"); + + // Setting should throw too. + OCIO_CHECK_THROW_WHAT(cs->setInterchangeAttribute("unknown_attribute1", "unknown"), + OCIO::Exception, + "Unknown attribute name"); + OCIO_CHECK_THROW_WHAT(cs->setInterchangeAttribute("unknown_attribute2", ""), + OCIO::Exception, + "Unknown attribute name"); + OCIO_CHECK_THROW_WHAT(cs->setInterchangeAttribute("unknown_attribute3", nullptr), + OCIO::Exception, + "Unknown attribute name"); + + // Make sure none of the above was stored. + auto attrMap = cs->getInterchangeAttributes(); + OCIO_CHECK_EQUAL(attrMap.size(), 0); +} + OCIO_ADD_TEST(Config, is_colorspace_linear) { @@ -1813,272 +2189,3 @@ inactive_colorspaces: [scene-linear Rec.709-sRGB, ACES2065-1] } } -OCIO_ADD_TEST(ColorSpace, interop_id) -{ - OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); - - // Test default value. - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); - - // Test setting and getting single profile name. - const char * interop_id = "srgb_p3d65_scene"; - cs->setInteropID(interop_id); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), interop_id); - - // Test setting empty string. - cs->setInteropID(""); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); - - // Test setting and getting another value. - const char * anotherID= "lin_rec2020_scene"; - cs->setInteropID(anotherID); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), anotherID); - - // Test setting null pointer (should be safe). - cs->setInteropID("something"); - OCIO_CHECK_NO_THROW(cs->setInteropID(nullptr)); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), ""); - - // Test copy constructor preserves InteropID. - cs->setInteropID(interop_id); - OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); - OCIO_CHECK_EQUAL(std::string(copy->getInteropID()), interop_id); - - // Test valid InteropID with colon in the middle. - const char * validColonMiddle = "namespace:colorspace_name"; - OCIO_CHECK_NO_THROW(cs->setInteropID(validColonMiddle)); - OCIO_CHECK_EQUAL(std::string(cs->getInteropID()), validColonMiddle); - - // Test invalid InteropID with multiple colons. - const char * invalidMultipleColons = "name:space:cs_name"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidMultipleColons), OCIO::Exception, - "only zero or one ':' character is allowed"); - - // Test invalid InteropID with colon at the end. - const char * invalidColonAtEnd = "namespace:"; - OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidColonAtEnd), OCIO::Exception, - "':' character cannot be the last character"); - - // Test invalid InteropID with non-ASCII characters. - const char * invalidNonASCII1 = "café_scene"; // Contains é (0xC3 0xA9) - OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidNonASCII1), OCIO::Exception, - "is invalid: only ASCII characters [0x00..0x7F] are allowed."); - - const char * invalidNonASCII2 = "space±_name"; // Contains ± (ANSI 0xB1) - OCIO_CHECK_THROW_WHAT(cs->setInteropID(invalidNonASCII2), OCIO::Exception, - "is invalid: only ASCII characters [0x00..0x7F] are allowed."); -} - -OCIO_ADD_TEST(ColorSpace, interop_id_serialization) -{ - // Test YAML serialization and deserialization of InteropID. - auto cfg = OCIO::Config::Create(); - auto cs = OCIO::ColorSpace::Create(); - cs->setName("test_colorspace"); - - const std::string interop_id = "lin_rec709_scene"; - - cs->setInteropID(interop_id.c_str()); - cfg->addColorSpace(cs); - - // Serialize the Config. - std::stringstream ss; - cfg->serialize(ss); - std::string yamlStr = ss.str(); - - // Verify interop_id appears in YAML. - OCIO_CHECK_NE(yamlStr.find("interop_id"), std::string::npos); - OCIO_CHECK_NE(yamlStr.find(interop_id), std::string::npos); - - // Deserialize and verify. - std::istringstream iss(yamlStr); - OCIO::ConstConfigRcPtr deserializedCfg; - OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); - - // Verify interop_id is preserved. - OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); - OCIO_CHECK_EQUAL(std::string(deserializedCs->getInteropID()), interop_id); - - // verify that that versions earlier than 2.0 reject interop_id. - OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); - cfgCopy->setVersion(2, 0); - OCIO_CHECK_NO_THROW(cfgCopy->serialize(ss)); - - cfgCopy->setVersion(1, 0); - OCIO_CHECK_THROW_WHAT( - cfgCopy->serialize(ss), - OCIO::Exception, - "Config failed validation. The color space 'test_colorspace' has non-empty " - "InteropID and config version is less than 2.0."); - - // Test with empty interop_id (should not appear in YAML). - cs->setInteropID(nullptr); - cfg->addColorSpace(cs); // Replace the existing CS. - ss.str(""); - cfg->serialize(ss); - std::string yamlStr2 = ss.str(); - - // Verify empty imterop_id does not appear in YAML. - OCIO_CHECK_EQUAL(yamlStr2.find("interop_id"), std::string::npos); -} - -OCIO_ADD_TEST(ColorSpace, amf_transform_ids) -{ - OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); - - // Test default value. - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); - - // Test setting and getting single ID. - const char * singleID = "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3"; - cs->setInterchangeAttribute("amf_transform_ids", singleID); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), singleID); - - // Test setting to empty string. - cs->setInterchangeAttribute("amf_transform_ids", ""); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); - - // Test setting and getting multiple IDs. - const char * multipleIDs = - "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" - "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; - cs->setInterchangeAttribute("amf_transform_ids", multipleIDs); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), multipleIDs); - - // Test setting to null pointer (should be safe). - cs->setInterchangeAttribute("amf_transform_ids", "something"); - cs->setInterchangeAttribute("amf_transform_ids", nullptr); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("amf_transform_ids")), ""); - - // Test copy constructor preserves AMF transform IDs. - cs->setInterchangeAttribute("amf_transform_ids", singleID); - OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); - OCIO_CHECK_EQUAL(std::string(copy->getInterchangeAttribute("amf_transform_ids")), singleID); -} - -OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) -{ - // Test YAML serialization and deserialization of AmfTransformIDs. - auto cfg = OCIO::Config::Create(); - auto cs = OCIO::ColorSpace::Create(); - cs->setName("test_colorspace"); - - const std::string amfIDs = - "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACEScc_to_ACES.a1.0.3\n" - "urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_ACEScc.a1.0.3"; - - cs->setInterchangeAttribute("amf_transform_ids", amfIDs.c_str()); - cfg->addColorSpace(cs); - - // Serialize the Config. - std::stringstream ss; - cfg->serialize(ss); - std::string yamlStr = ss.str(); - - // Verify AmfTransformIDs appears in YAML. - OCIO_CHECK_NE(yamlStr.find("amf_transform_ids"), std::string::npos); - OCIO_CHECK_NE(yamlStr.find("ACEScsc.Academy.ACEScc_to_ACES"), std::string::npos); - - // Deserialize and verify. - std::istringstream iss(yamlStr); - OCIO::ConstConfigRcPtr deserializedCfg; - OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); - - // Verify AmfTransformIDs is preserved. - OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); - auto deserializedIDs = deserializedCs->getInterchangeAttribute("amf_transform_ids"); - OCIO_CHECK_EQUAL(deserializedIDs, amfIDs); - - // Verify that that earlier versions retain amf_transform_ids. - OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); - cfgCopy->setVersion(2,4); - OCIO_CHECK_NO_THROW(cfgCopy->serialize(ss)); - - // Test with empty AmfTransformIDs (should not appear in YAML). - cs->setInterchangeAttribute("amf_transform_ids", nullptr); - cfg->addColorSpace(cs); // Replace the existing CS. - ss.str(""); - cfg->serialize(ss); - std::string yamlStr2 = ss.str(); - - // Verify empty AmfTransformIDs does not appear in YAML. - OCIO_CHECK_EQUAL(yamlStr2.find("amf_transform_ids"), std::string::npos); -} - -OCIO_ADD_TEST(ColorSpace, icc_profile_name) -{ - OCIO::ColorSpaceRcPtr cs = OCIO::ColorSpace::Create(); - - // Test default value. - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); - - // Test setting and getting single profile name. - const char * profileName = "sRGB IEC61966-2.1"; - cs->setInterchangeAttribute("icc_profile_name",profileName); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), profileName); - - // Test setting and getting another profile name. - const char * anotherProfile = "Adobe RGB (1998)"; - cs->setInterchangeAttribute("icc_profile_name", anotherProfile); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), anotherProfile); - - // Test setting empty string. - cs->setInterchangeAttribute("icc_profile_name", ""); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); - - // Test setting null pointer (should be safe). - cs->setInterchangeAttribute("icc_profile_name", "something"); - OCIO_CHECK_NO_THROW(cs->setInterchangeAttribute("icc_profile_name", nullptr)); - OCIO_CHECK_EQUAL(std::string(cs->getInterchangeAttribute("icc_profile_name")), ""); - - // Test copy constructor preserves ICC profile name. - cs->setInterchangeAttribute("icc_profile_name", profileName); - OCIO::ColorSpaceRcPtr copy = cs->createEditableCopy(); - OCIO_CHECK_EQUAL(std::string(copy->getInterchangeAttribute("icc_profile_name")), profileName); -} - -OCIO_ADD_TEST(ColorSpace, icc_profile_name_serialization) -{ - // Test YAML serialization and deserialization of IccProfileName. - auto cfg = OCIO::Config::Create(); - auto cs = OCIO::ColorSpace::Create(); - cs->setName("test_colorspace"); - - const std::string profileName = "sRGB IEC61966-2.1"; - - cs->setInterchangeAttribute("icc_profile_name", profileName.c_str()); - cfg->addColorSpace(cs); - - // Serialize the Config. - std::stringstream ss; - cfg->serialize(ss); - std::string yamlStr = ss.str(); - - // Verify IccProfileName appears in YAML. - OCIO_CHECK_NE(yamlStr.find("icc_profile_name"), std::string::npos); - OCIO_CHECK_NE(yamlStr.find(profileName), std::string::npos); - - // Deserialize and verify. - std::istringstream iss(yamlStr); - OCIO::ConstConfigRcPtr deserializedCfg; - OCIO_CHECK_NO_THROW(deserializedCfg = OCIO::Config::CreateFromStream(iss)); - - // Verify IccProfileName is preserved. - OCIO::ConstColorSpaceRcPtr deserializedCs = deserializedCfg->getColorSpace("test_colorspace"); - OCIO_CHECK_EQUAL(std::string(deserializedCs->getInterchangeAttribute("icc_profile_name")), profileName); - - // verify that that earlier versions retain amf_transform_ids. - OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); - cfgCopy->setVersion(2, 4); - OCIO_CHECK_NO_THROW(cfgCopy->serialize(ss)); - - // Test with empty IccProfileName (should not appear in YAML). - cs->setInterchangeAttribute("icc_profile_name", nullptr); - cfg->addColorSpace(cs); // replace the existing CS - ss.str(""); - cfg->serialize(ss); - std::string yamlStr2 = ss.str(); - - // Verify empty IccProfileName does not appear in YAML. - OCIO_CHECK_EQUAL(yamlStr2.find("icc_profile_name"), std::string::npos); -} \ No newline at end of file diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index 4195a48d05..38487fcbba 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -42,7 +42,7 @@ def test_copy(self): self.colorspace.setTransform(direction=OCIO.COLORSPACE_DIR_FROM_REFERENCE, transform=mat) self.colorspace.addAlias('alias') self.colorspace.addCategory('cat') - self.colorspace.setInteropID('ACEScg') + self.colorspace.setInteropID('data') self.colorspace.setInterchangeAttribute('amf_transform_ids', 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3') self.colorspace.setInterchangeAttribute('icc_profile_name', 'sRGB IEC61966-2.1') @@ -430,6 +430,169 @@ def test_aliases(self): self.assertEqual(len(aliases), 0) self.assertFalse(cs.hasAlias('alias1')) + def test_interop_id(self): + """ + Test the setInteropID() and getInteropID() methods. + """ + + # Test default value (should be empty). + self.assertEqual(self.colorspace.getInteropID(), '') + + # Test setting and getting a simple interop ID. + test_id = 'lin_ap0_scene' + self.colorspace.setInteropID(test_id) + self.assertEqual(self.colorspace.getInteropID(), test_id) + + # Test setting and getting a different interop ID. + test_id2 = 'srgb_ap1_scene' + self.colorspace.setInteropID(test_id2) + self.assertEqual(self.colorspace.getInteropID(), test_id2) + + # Test setting empty string. + self.colorspace.setInteropID('') + self.assertEqual(self.colorspace.getInteropID(), '') + + # Test setting None (should convert to empty string). + self.colorspace.setInteropID('something') + self.colorspace.setInteropID(None) + self.assertEqual(self.colorspace.getInteropID(), '') + + # Test wrong type (should raise TypeError). + with self.assertRaises(TypeError): + self.colorspace.setInteropID(123) + + with self.assertRaises(TypeError): + self.colorspace.setInteropID(['list']) + + # Test valid InteropID with one colon (not at the end). + valid_with_colon = 'namespace:cs_name' + self.colorspace.setInteropID(valid_with_colon) + self.assertEqual(self.colorspace.getInteropID(), valid_with_colon) + + # Test invalid InteropID with multiple colons. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('name:space:cs_name') + self.assertIn("Only one ':' is allowed to separate the namespace and the color space.", str(context.exception)) + + # Test invalid InteropID with colon at the end. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('namespace:') + self.assertIn("If ':' is used, both the namespace and the color space parts must be non-empty.", str(context.exception)) + + # Test invalid InteropID with non-ASCII characters. + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('café_scene') # Contains é (UTF-8) + self.assertIn("contains invalid characters.", str(context.exception)) + + with self.assertRaises(Exception) as context: + self.colorspace.setInteropID('space±_name') # Contains ± (ANSI 0xB1) + self.assertIn("contains invalid characters.", str(context.exception)) + + def test_interchange_atttributes(self): + """ + Test the setInterchangeAttribute() and getInterchangeAttribute() methods. + """ + # amf_transform_ids + + # Test default value (should be empty). + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') + + # Test setting and getting a single amf transform ID. + single_id = 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3' + self.colorspace.setInterchangeAttribute('amf_transform_ids', single_id) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), single_id) + + # Test setting and getting multiple transform IDs (newline-separated). + multiple_ids = ('urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3\n' + 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_CG.a1.0.3\n' + 'urn:ampas:aces:transformId:v1.5:RRT.a1.0.3') + self.colorspace.setInterchangeAttribute('amf_transform_ids', multiple_ids) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), multiple_ids) + + # Test setting empty string. + self.colorspace.setInterchangeAttribute('amf_transform_ids', '') + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') + + # Test setting None (should convert to empty string). + self.colorspace.setInterchangeAttribute('amf_transform_ids', 'something') + self.colorspace.setInterchangeAttribute('amf_transform_ids', None) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') + + # Test with different line endings. + mixed_endings = 'id1\nid2\rid3\r\nid4' + self.colorspace.setInterchangeAttribute('amf_transform_ids', mixed_endings) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), mixed_endings) + + # Test with leading/trailing whitespace. + whitespace_ids = ' \n id1 \n id2 \n ' + self.colorspace.setInterchangeAttribute('amf_transform_ids', whitespace_ids) + self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), whitespace_ids) + + # Test wrong type (should raise TypeError). + with self.assertRaises(TypeError): + self.colorspace.setInterchangeAttribute('amf_transform_ids', 123) + + with self.assertRaises(TypeError): + self.colorspace.setInterchangeAttribute('amf_transform_ids', ['list', 'of', 'ids']) + + # icc_profile_name + + # Test default value (should be empty). + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') + + # Test setting and getting a simple profile name. + profile_name = 'sRGB IEC61966-2.1' + self.colorspace.setInterchangeAttribute('icc_profile_name', profile_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), profile_name) + + # Test setting and getting a different profile name. + profile_name2 = 'Adobe RGB (1998)' + self.colorspace.setInterchangeAttribute('icc_profile_name', profile_name2) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), profile_name2) + + # Test with a more complex profile name. + complex_name = 'Display P3 - Apple Cinema Display (Calibrated 2023-01-15)' + self.colorspace.setInterchangeAttribute('icc_profile_name', complex_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), complex_name) + + # Test setting empty string. + self.colorspace.setInterchangeAttribute('icc_profile_name', '') + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') + + # Test setting None (should convert to empty string). + self.colorspace.setInterchangeAttribute('icc_profile_name', 'something') + self.colorspace.setInterchangeAttribute('icc_profile_name', None) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') + + # Test with special characters and numbers. + special_name = 'ProPhoto RGB v2.0 (γ=1.8) [Custom Profile #123]' + self.colorspace.setInterchangeAttribute('icc_profile_name', special_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), special_name) + + # Test with Unicode characters. + unicode_name = 'Профиль RGB γ=2.2' + self.colorspace.setInterchangeAttribute('icc_profile_name', unicode_name) + self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), unicode_name) + + # Test wrong type (should raise TypeError). + with self.assertRaises(TypeError): + self.colorspace.setInterchangeAttribute('icc_profile_name', 123) + + with self.assertRaises(TypeError): + self.colorspace.setInterchangeAttribute('icc_profile_name', ['profile', 'name']) + + # unsupported interchange key + + # test that setting an unsupported key raises. + with self.assertRaises(Exception) as context: + self.colorspace.setInterchangeAttribute('this_should_fail', 'foo42') + self.assertIn("Unknown attribute name 'this_should_fail'", str(context.exception)) + + # test that getting an unsupported key raises. + with self.assertRaises(Exception) as context: + self.colorspace.getInterchangeAttribute('this_should_fail') + self.assertIn("Unknown attribute name 'this_should_fail'", str(context.exception)) + def test_is_colorspace_linear(self): """ Test isColorSpaceLinear. @@ -641,172 +804,9 @@ def test_display_referred(self, cfg, cs_name, expected_value): test_display_referred(self, cfg, "scene_linear-trans-alias", False) test_display_referred(self, cfg, "scene_ref", False) - def test_interop_id(self): - """ - Test the setInteropID() and getInteropID() methods. - """ - - # Test default value (should be empty). - self.assertEqual(self.colorspace.getInteropID(), '') - - # Test setting and getting a simple interop ID. - test_id = 'lin_ap0_scene' - self.colorspace.setInteropID(test_id) - self.assertEqual(self.colorspace.getInteropID(), test_id) - - # Test setting and getting a different interop ID. - test_id2 = 'srgb_ap1_scene' - self.colorspace.setInteropID(test_id2) - self.assertEqual(self.colorspace.getInteropID(), test_id2) - - # Test setting empty string. - self.colorspace.setInteropID('') - self.assertEqual(self.colorspace.getInteropID(), '') - - # Test setting None (should convert to empty string). - self.colorspace.setInteropID('something') - self.colorspace.setInteropID(None) - self.assertEqual(self.colorspace.getInteropID(), '') - - # Test wrong type (should raise TypeError). - with self.assertRaises(TypeError): - self.colorspace.setInteropID(123) - - with self.assertRaises(TypeError): - self.colorspace.setInteropID(['list']) - - # Test valid InteropID with one colon (not at the end). - valid_with_colon = 'namespace:cs_name' - self.colorspace.setInteropID(valid_with_colon) - self.assertEqual(self.colorspace.getInteropID(), valid_with_colon) - - # Test invalid InteropID with multiple colons. - with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('name:space:cs_name') - self.assertIn("only zero or one ':' character is allowed", str(context.exception)) - - # Test invalid InteropID with colon at the end. - with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('namespace:') - self.assertIn("':' character cannot be the last character", str(context.exception)) - - # Test invalid InteropID with non-ASCII characters. - with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('café_scene') # Contains é (UTF-8) - self.assertIn("is invalid: only ASCII characters [0x00..0x7F] are allowed.", str(context.exception)) - - with self.assertRaises(Exception) as context: - self.colorspace.setInteropID('space±_name') # Contains ± (ANSI 0xB1) - self.assertIn("is invalid: only ASCII characters [0x00..0x7F] are allowed.", str(context.exception)) - - def test_interchange_atttributes(self): - """ - Test the setInterchangeAttribute() and getInterchangeAttribute() methods. - """ - # amf_transform_ids - - # Test default value (should be empty). - self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') - - # Test setting and getting a single amf transform ID. - single_id = 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3' - self.colorspace.setInterchangeAttribute('amf_transform_ids', single_id) - self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), single_id) - - # Test setting and getting multiple transform IDs (newline-separated). - multiple_ids = ('urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.CG_to_ACES.a1.0.3\n' - 'urn:ampas:aces:transformId:v1.5:ACEScsc.Academy.ACES_to_CG.a1.0.3\n' - 'urn:ampas:aces:transformId:v1.5:RRT.a1.0.3') - self.colorspace.setInterchangeAttribute('amf_transform_ids', multiple_ids) - self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), multiple_ids) - - # Test setting empty string. - self.colorspace.setInterchangeAttribute('amf_transform_ids', '') - self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') - - # Test setting None (should convert to empty string). - self.colorspace.setInterchangeAttribute('amf_transform_ids', 'something') - self.colorspace.setInterchangeAttribute('amf_transform_ids', None) - self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), '') - - # Test with different line endings. - mixed_endings = 'id1\nid2\rid3\r\nid4' - self.colorspace.setInterchangeAttribute('amf_transform_ids', mixed_endings) - self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), mixed_endings) - - # Test with leading/trailing whitespace. - whitespace_ids = ' \n id1 \n id2 \n ' - self.colorspace.setInterchangeAttribute('amf_transform_ids', whitespace_ids) - self.assertEqual(self.colorspace.getInterchangeAttribute('amf_transform_ids'), whitespace_ids) - - # Test wrong type (should raise TypeError). - with self.assertRaises(TypeError): - self.colorspace.setInterchangeAttribute('amf_transform_ids', 123) - - with self.assertRaises(TypeError): - self.colorspace.setInterchangeAttribute('amf_transform_ids', ['list', 'of', 'ids']) - - # icc_profile_name - - # Test default value (should be empty). - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') - - # Test setting and getting a simple profile name. - profile_name = 'sRGB IEC61966-2.1' - self.colorspace.setInterchangeAttribute('icc_profile_name', profile_name) - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), profile_name) - - # Test setting and getting a different profile name. - profile_name2 = 'Adobe RGB (1998)' - self.colorspace.setInterchangeAttribute('icc_profile_name', profile_name2) - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), profile_name2) - - # Test with a more complex profile name. - complex_name = 'Display P3 - Apple Cinema Display (Calibrated 2023-01-15)' - self.colorspace.setInterchangeAttribute('icc_profile_name', complex_name) - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), complex_name) - - # Test setting empty string. - self.colorspace.setInterchangeAttribute('icc_profile_name', '') - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') - - # Test setting None (should convert to empty string). - self.colorspace.setInterchangeAttribute('icc_profile_name', 'something') - self.colorspace.setInterchangeAttribute('icc_profile_name', None) - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), '') - - # Test with special characters and numbers. - special_name = 'ProPhoto RGB v2.0 (γ=1.8) [Custom Profile #123]' - self.colorspace.setInterchangeAttribute('icc_profile_name', special_name) - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), special_name) - - # Test with Unicode characters. - unicode_name = 'Профиль RGB γ=2.2' - self.colorspace.setInterchangeAttribute('icc_profile_name', unicode_name) - self.assertEqual(self.colorspace.getInterchangeAttribute('icc_profile_name'), unicode_name) - - # Test wrong type (should raise TypeError). - with self.assertRaises(TypeError): - self.colorspace.setInterchangeAttribute('icc_profile_name', 123) - - with self.assertRaises(TypeError): - self.colorspace.setInterchangeAttribute('icc_profile_name', ['profile', 'name']) - - # unsupported interchange key - - # test that setting an unsupported key raises. - with self.assertRaises(Exception) as context: - self.colorspace.setInterchangeAttribute('this_should_fail', 'foo42') - self.assertIn("Unknown attribute name 'this_should_fail'", str(context.exception)) - - # test that getting an unsupported key raises. - with self.assertRaises(Exception) as context: - self.colorspace.getInterchangeAttribute('this_should_fail') - self.assertIn("Unknown attribute name 'this_should_fail'", str(context.exception)) - def test_processor_to_known_colorspace(self): - CONFIG = """ocio_profile_version: 2.5 + CONFIG = """ocio_profile_version: 2.0 roles: default: raw From ab1bcbca1a7a38f4ed7bdcd2a8beecb5f356a633 Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Fri, 12 Sep 2025 19:30:05 -0700 Subject: [PATCH 13/15] - Config::getColorSpaceFromInteropID() function is removed. - another round of cleanup. Signed-off-by: cuneyt.ozdas --- include/OpenColorIO/OpenColorIO.h | 13 +------- src/OpenColorIO/Config.cpp | 20 ------------ src/OpenColorIO/OCIOYaml.cpp | 54 +++++++++++++++---------------- src/bindings/python/PyConfig.cpp | 2 -- tests/cpu/ColorSpace_tests.cpp | 8 +++-- tests/python/ColorSpaceTest.py | 6 ++-- 6 files changed, 37 insertions(+), 66 deletions(-) diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 9a9bc792d8..138f210c1e 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -622,17 +622,6 @@ class OCIOEXPORT Config */ ConstColorSpaceRcPtr getColorSpace(const char * name) const; - - enum InteropIDSearchMethod : uint8_t - { - FullID, ///< Search only for the full interop ID ([namespace:]colorspace); - NameOnlyFallback ///< If fullID fails, will search for name only (colorspace). - }; - /** - * \brief Get the color space from the provided interopID. - */ - ConstColorSpaceRcPtr getColorSpaceFromInteropID(const char * interopID, InteropIDSearchMethod method) const; - /** * Accepts an alias, role name, named transform name, or color space name and returns the * color space name or the named transform name. @@ -2011,7 +2000,7 @@ class OCIOEXPORT ColorSpace * * Currently supported attribute names are "amf_transform_ids" and * "icc_profile_name". Using any other name will throw. If the attribute is - * not defined, it'll return empty string. Setting the value to an empty + * not defined, it'll return an empty string. Setting the value to an empty * string will effectively delete the attribute. * * The AMF transform IDs are used to identify specific transforms in the diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 69aeeed433..67a699a553 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -2497,26 +2497,6 @@ const char * Config::getCanonicalName(const char * name) const return ""; } -ConstColorSpaceRcPtr Config::getColorSpaceFromInteropID(const char * interopID, InteropIDSearchMethod method) const -{ - // NOTE: This will search color space names, aliases, and roles. - auto cs = getColorSpace(interopID); - - // Fall back to name-only if allowed. - if(!cs && method == NameOnlyFallback) - { - std::string id(interopID); - size_t pos = id.find(':'); - if(pos != std::string::npos) - { - std::string csname = id.substr(pos+1); - cs = getColorSpace(id.c_str()); - } - } - - return cs; -} - int Config::getNumColorSpaces() const { return getNumColorSpaces(SEARCH_REFERENCE_SPACE_ALL, COLORSPACE_ACTIVE); diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 3d2fc66e9f..dd16064b75 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -3382,6 +3382,13 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major out << YAML::Flow << YAML::Value << aliases; } + const std::string interopID{ cs->getInteropID() }; + if (!interopID.empty()) + { + out << YAML::Key << "interop_id"; + out << YAML::Value << interopID; + } + out << YAML::Key << "family" << YAML::Value << cs->getFamily(); out << YAML::Key << "equalitygroup" << YAML::Value << cs->getEqualityGroup(); @@ -3391,6 +3398,26 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major saveDescription(out, cs->getDescription()); + out << YAML::Key << "isdata" << YAML::Value << cs->isData(); + + if(cs->getNumCategories() > 0) + { + StringUtils::StringVec categories; + for(int idx=0; idxgetNumCategories(); ++idx) + { + categories.push_back(cs->getCategory(idx)); + } + out << YAML::Key << "categories"; + out << YAML::Flow << YAML::Value << categories; + } + + const std::string is{ cs->getEncoding() }; + if (!is.empty()) + { + out << YAML::Key << "encoding"; + out << YAML::Value << is; + } + auto interchangemap = cs->getInterchangeAttributes(); if (interchangemap.size()) { @@ -3425,33 +3452,6 @@ inline void save(YAML::Emitter& out, ConstColorSpaceRcPtr cs, unsigned int major out << YAML::EndMap; } - out << YAML::Key << "isdata" << YAML::Value << cs->isData(); - - if(cs->getNumCategories() > 0) - { - StringUtils::StringVec categories; - for(int idx=0; idxgetNumCategories(); ++idx) - { - categories.push_back(cs->getCategory(idx)); - } - out << YAML::Key << "categories"; - out << YAML::Flow << YAML::Value << categories; - } - - const std::string is{ cs->getEncoding() }; - if (!is.empty()) - { - out << YAML::Key << "encoding"; - out << YAML::Value << is; - } - - const std::string interopID{ cs->getInteropID() }; - if (!interopID.empty()) - { - out << YAML::Key << "interop_id"; - out << YAML::Value << interopID; - } - out << YAML::Key << "allocation" << YAML::Value; save(out, cs->getAllocation()); if(cs->getAllocationNumVars() > 0) diff --git a/src/bindings/python/PyConfig.cpp b/src/bindings/python/PyConfig.cpp index 706734b8e5..104d017f89 100644 --- a/src/bindings/python/PyConfig.cpp +++ b/src/bindings/python/PyConfig.cpp @@ -316,8 +316,6 @@ void bindPyConfig(py::module & m) { return ActiveColorSpaceIterator(self); }) - .def("getColorSpaceFromInteropID", &Config::getColorSpaceFromInteropID, "interopID"_a, "searchMethod"_a, - DOC(Config, getColorSpaceFromInteropID)) .def("getCanonicalName", &Config::getCanonicalName, "name"_a, DOC(Config, getCanonicalName)) .def("addColorSpace", &Config::addColorSpace, "colorSpace"_a, diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index ceb9807039..982b4deda1 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -10,6 +10,7 @@ #include "testutils/UnitTest.h" #include "UnitTestUtils.h" +#include "UnitTestLogUtils.h" namespace OCIO = OCIO_NAMESPACE; @@ -295,7 +296,7 @@ active_views: [] OCIO_CHECK_EQUAL(cfgString, os.str()); } - // Adding a color space that uses all parameters. + // Adding a color space that uses all parameters (as of 2.0). { constexpr char End[]{ R"(colorspaces: - ! @@ -786,6 +787,7 @@ active_views: [] std::istringstream is; is.str(cfgString); OCIO::ConstConfigRcPtr config; + OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); OCIO_REQUIRE_EQUAL(!config, false); auto attrMap = config->getColorSpace("raw")->getInterchangeAttributes(); @@ -1091,7 +1093,7 @@ OCIO_ADD_TEST(ColorSpace, amf_transform_ids_serialization) auto deserializedIDs = deserializedCs->getInterchangeAttribute("amf_transform_ids"); OCIO_CHECK_EQUAL(deserializedIDs, amfIDs); - // Verify that that earlier versions retain amf_transform_ids. + // Verify that that earlier versions reject amf_transform_ids. OCIO::ConfigRcPtr cfgCopy = cfg->createEditableCopy(); cfgCopy->setVersion(2,4); OCIO_CHECK_THROW_WHAT(cfgCopy->serialize(ss), @@ -1178,7 +1180,7 @@ OCIO_ADD_TEST(ColorSpace, icc_profile_name_serialization) OCIO::Exception, "has non-empty interchange attributes and config version is less than 2.5."); - // Test with empty IccProfileName (should be valid in 2.4 and should not appear in YAML). + // Test with empty IccProfileName (should not appear in YAML, and so won't invalidate a 2.4 config). cs->setInterchangeAttribute("icc_profile_name", nullptr); cfg->addColorSpace(cs); // replace the existing CS ss.str(""); diff --git a/tests/python/ColorSpaceTest.py b/tests/python/ColorSpaceTest.py index 38487fcbba..7ad1edde9c 100644 --- a/tests/python/ColorSpaceTest.py +++ b/tests/python/ColorSpaceTest.py @@ -472,12 +472,14 @@ def test_interop_id(self): # Test invalid InteropID with multiple colons. with self.assertRaises(Exception) as context: self.colorspace.setInteropID('name:space:cs_name') - self.assertIn("Only one ':' is allowed to separate the namespace and the color space.", str(context.exception)) + self.assertIn("Only one ':' is allowed to separate the namespace and the color space.", + str(context.exception)) # Test invalid InteropID with colon at the end. with self.assertRaises(Exception) as context: self.colorspace.setInteropID('namespace:') - self.assertIn("If ':' is used, both the namespace and the color space parts must be non-empty.", str(context.exception)) + self.assertIn("If ':' is used, both the namespace and the color space parts must be non-empty.", + str(context.exception)) # Test invalid InteropID with non-ASCII characters. with self.assertRaises(Exception) as context: From bc9a750731fc77e1d31f8a47d389c236e36af1ce Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Fri, 12 Sep 2025 19:37:16 -0700 Subject: [PATCH 14/15] minor comment fix Signed-off-by: cuneyt.ozdas --- src/OpenColorIO/ColorSpace.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenColorIO/ColorSpace.cpp b/src/OpenColorIO/ColorSpace.cpp index 70b49dbda7..98f776fe18 100644 --- a/src/OpenColorIO/ColorSpace.cpp +++ b/src/OpenColorIO/ColorSpace.cpp @@ -242,7 +242,7 @@ void ColorSpace::setInteropID(const char * interopID) if (!id.empty()) { // check if it only uses ASCII characters: 0-9, a-z, and the following characters (no spaces): - // - _ ~ / * # % ^ + ( ) [ ] | + // . - _ ~ / * # % ^ + ( ) [ ] | auto allowed = [](char c) { return (c >= '0' && c <= '9')|| From fe9e2a9a59ac43c6ccc74c1595478895964e0dfe Mon Sep 17 00:00:00 2001 From: "cuneyt.ozdas" Date: Fri, 12 Sep 2025 22:01:20 -0700 Subject: [PATCH 15/15] minor fixes to unknown interchange key warning and its unit test. Signed-off-by: cuneyt.ozdas --- src/OpenColorIO/OCIOYaml.cpp | 13 ++++++------- tests/cpu/ColorSpace_tests.cpp | 3 ++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index dd16064b75..eea8eef99a 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -3249,19 +3249,18 @@ inline void load(const YAML::Node& node, ColorSpaceRcPtr& cs, unsigned int major for (const auto& keyval : kv.m_keyVals) { + std::string keystr = keyval.first.as(); + std::string valstr = keyval.second.as(); + SanitizeLoadedNewlines(valstr); + // OCIO exception means the key is not recognized. Convert that to a warning. try { - std::string val = keyval.second.as(); - SanitizeLoadedNewlines(val); - - cs->setInterchangeAttribute( - keyval.first.as().c_str(), - val.c_str()); + cs->setInterchangeAttribute(keystr.c_str(), valstr.c_str()); } catch (Exception &) { - LogUnknownKeyWarning(iter->second, keyval.first); + LogUnknownKeyWarning(key, keyval.first); } } } diff --git a/tests/cpu/ColorSpace_tests.cpp b/tests/cpu/ColorSpace_tests.cpp index 982b4deda1..abb3b25063 100644 --- a/tests/cpu/ColorSpace_tests.cpp +++ b/tests/cpu/ColorSpace_tests.cpp @@ -772,7 +772,7 @@ active_views: [] - ! name: raw interchange: - unknown: will be ignored + my-attrib: will be ignored family: raw equalitygroup: "" bitdepth: 32f @@ -789,6 +789,7 @@ active_views: [] OCIO::ConstConfigRcPtr config; OCIO::LogGuard logGuard; OCIO_CHECK_NO_THROW(config = OCIO::Config::CreateFromStream(is)); + OCIO_CHECK_EQUAL(logGuard.output(), "[OpenColorIO Warning]: Unknown key in interchange: 'my-attrib'.\n"); OCIO_REQUIRE_EQUAL(!config, false); auto attrMap = config->getColorSpace("raw")->getInterchangeAttributes(); OCIO_CHECK_EQUAL(attrMap.size(), 0);