From 803c453d58863a31e2c740759490f85c7bd84cad Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Wed, 13 Sep 2023 17:08:34 +0100 Subject: [PATCH] Fix regression in cccid handling when no value is supplied In v1 of OCIO FileTransforms are able to load .cc files without specifying a cccid. In v2 this broke causing an exception to be raised instead of using the first cc found in the file. Signed-off-by: Kevin Wheatley --- src/OpenColorIO/fileformats/FileFormatCCC.cpp | 2 +- src/OpenColorIO/fileformats/FileFormatCDL.cpp | 4 +- tests/cpu/transforms/FileTransform_tests.cpp | 71 +++++++++++++++++++ .../files/cdl_test_cc_file_with_extension.ccc | 12 ++++ .../files/cdl_test_cc_file_with_extension.cdl | 12 ++++ 5 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 tests/data/files/cdl_test_cc_file_with_extension.ccc create mode 100644 tests/data/files/cdl_test_cc_file_with_extension.cdl diff --git a/src/OpenColorIO/fileformats/FileFormatCCC.cpp b/src/OpenColorIO/fileformats/FileFormatCCC.cpp index 3670c57bca..bad7723efa 100755 --- a/src/OpenColorIO/fileformats/FileFormatCCC.cpp +++ b/src/OpenColorIO/fileformats/FileFormatCCC.cpp @@ -217,7 +217,7 @@ void LocalFileFormat::buildFileOps(OpRcPtrVec & ops, { // Use 0 for empty string. int cccindex=0; - if (StringToInt(&cccindex, cccid.c_str(), true)) + if (cccid.empty() || StringToInt(&cccindex, cccid.c_str(), true)) { int maxindex = ((int)cachedFile->m_transformVec.size())-1; if (cccindex<0 || cccindex>maxindex) diff --git a/src/OpenColorIO/fileformats/FileFormatCDL.cpp b/src/OpenColorIO/fileformats/FileFormatCDL.cpp index 3a687a1c5f..5da6aae953 100755 --- a/src/OpenColorIO/fileformats/FileFormatCDL.cpp +++ b/src/OpenColorIO/fileformats/FileFormatCDL.cpp @@ -247,7 +247,7 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops, { // Use 0 for empty string. int cccindex=0; - if (StringToInt(&cccindex, cccid.c_str(), true)) + if (cccid.empty() || StringToInt(&cccindex, cccid.c_str(), true)) { int maxindex = ((int)cachedFile->m_transformVec.size())-1; if (cccindex<0 || cccindex>maxindex) @@ -274,7 +274,7 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops, if (!success) { std::ostringstream os; - os << "You must specify a valid cccid to load from the ccc file"; + os << "You must specify a valid cccid to load from the cdl file"; os << " (either by name or index). id='" << cccid << "' "; os << "is not found in the file, and is not parsable as an "; os << "integer index."; diff --git a/tests/cpu/transforms/FileTransform_tests.cpp b/tests/cpu/transforms/FileTransform_tests.cpp index 55315822cf..0301ac02dc 100644 --- a/tests/cpu/transforms/FileTransform_tests.cpp +++ b/tests/cpu/transforms/FileTransform_tests.cpp @@ -484,3 +484,74 @@ OCIO_ADD_TEST(FileTransform, context_variables) OCIO_CHECK_EQUAL(std::string("01"), usedContextVars->getStringVarByIndex(1)); } } + +OCIO_ADD_TEST(FileTransform, cc_file_with_different_file_extension) +{ + static const std::string BASE_CONFIG = + "ocio_profile_version: 1\n" + "description: Minimal\n" + "search_path: " + OCIO::GetTestFilesDir() + "\n" + "\n" + "roles:\n" + " default: basic\n" + " scene_linear: basic\n" + " data: basic\n" + " reference: basic\n" + " compositing_log: basic\n" + " color_timing: basic\n" + " color_picking: basic\n" + " texture_paint: basic\n" + " matte_paint: basic\n" + " rendering: basic\n" + " aces_interchange: basic\n" + " cie_xyz_d65_interchange: basic\n" + "\n" + "displays:\n" + " display:\n" + " - ! {name: basic, colorspace: basic }\n" + " - ! {name: cdl, colorspace: basic_cdl }\n" + "\n" + "colorspaces:\n" + " - !\n" + " name: basic\n" + "\n" + " - !\n" + " name: basic_cdl\n"; + + { + static const std::string CONFIG = BASE_CONFIG + + " from_reference: ! { src: cdl_test_cc_file_with_extension.cdl }\n"; + std::istringstream iss; + iss.str(CONFIG); + + OCIO::ConstConfigRcPtr cfg; + OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss)); + OCIO_CHECK_NO_THROW(cfg->validate()); + + + OCIO::ConstTransformRcPtr tr1 = cfg->getColorSpace("basic_cdl")->getTransform( + OCIO::COLORSPACE_DIR_FROM_REFERENCE + ); + OCIO::ConstFileTransformRcPtr fTr1 = OCIO::DynamicPtrCast(tr1); + OCIO_CHECK_ASSERT(fTr1); + OCIO_CHECK_NO_THROW(cfg->getProcessor(tr1)); + } + { + static const std::string CONFIG = BASE_CONFIG + + " from_reference: ! { src: cdl_test_cc_file_with_extension.ccc }\n"; + std::istringstream iss; + iss.str(CONFIG); + + OCIO::ConstConfigRcPtr cfg; + OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss)); + OCIO_CHECK_NO_THROW(cfg->validate()); + + + OCIO::ConstTransformRcPtr tr2 = cfg->getColorSpace("basic_cdl")->getTransform( + OCIO::COLORSPACE_DIR_FROM_REFERENCE + ); + OCIO::ConstFileTransformRcPtr fTr2 = OCIO::DynamicPtrCast(tr2); + OCIO_CHECK_ASSERT(fTr2); + OCIO_CHECK_NO_THROW(cfg->getProcessor(tr2)); + } +} diff --git a/tests/data/files/cdl_test_cc_file_with_extension.ccc b/tests/data/files/cdl_test_cc_file_with_extension.ccc new file mode 100644 index 0000000000..70ad93687e --- /dev/null +++ b/tests/data/files/cdl_test_cc_file_with_extension.ccc @@ -0,0 +1,12 @@ + + + + No Op CDL + 1.00000 1.00000 1.00000 + 0.00000 0.00000 0.00000 + 1.00000 1.00000 1.00000 + + + 1.00000 + + diff --git a/tests/data/files/cdl_test_cc_file_with_extension.cdl b/tests/data/files/cdl_test_cc_file_with_extension.cdl new file mode 100644 index 0000000000..70ad93687e --- /dev/null +++ b/tests/data/files/cdl_test_cc_file_with_extension.cdl @@ -0,0 +1,12 @@ + + + + No Op CDL + 1.00000 1.00000 1.00000 + 0.00000 0.00000 0.00000 + 1.00000 1.00000 1.00000 + + + 1.00000 + +