Fix regression in cccid handling when no value is supplied (AcademySo…

…ftwareFoundation#1855) 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 <[email protected]> Co-authored-by: Doug Walker <[email protected]> Signed-off-by: Brooke <[email protected]>
brkglvn01 · Oct 23, 2023 · 9f9e94b · 9f9e94b
1 parent df6caa2
commit 9f9e94b
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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"
+    "    - !<View> {name: basic, colorspace: basic }\n"
+    "    - !<View> {name: cdl, colorspace: basic_cdl }\n"
+    "\n"
+    "colorspaces:\n"
+    "  - !<ColorSpace>\n"
+    "    name: basic\n"
+    "\n"
+    "  - !<ColorSpace>\n"
+    "    name: basic_cdl\n";
+
+    {
+        static const std::string CONFIG = BASE_CONFIG +
+        "    from_reference: !<FileTransform> { 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<const OCIO::FileTransform>(tr1);
+        OCIO_CHECK_ASSERT(fTr1);
+        OCIO_CHECK_NO_THROW(cfg->getProcessor(tr1));
+    }
+    {
+        static const std::string CONFIG = BASE_CONFIG +
+        "    from_reference: !<FileTransform> { 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<const OCIO::FileTransform>(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
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ColorCorrection>
+   <SOPNode>
+      <Description>No Op CDL</Description>
+      <Slope>1.00000 1.00000 1.00000</Slope>
+      <Offset>0.00000 0.00000 0.00000</Offset>
+      <Power>1.00000 1.00000 1.00000</Power>
+   </SOPNode>
+   <SatNode>
+      <Saturation>1.00000</Saturation>
+   </SatNode>
+</ColorCorrection>
diff --git a/tests/data/files/cdl_test_cc_file_with_extension.cdl b/tests/data/files/cdl_test_cc_file_with_extension.cdl
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ColorCorrection>
+   <SOPNode>
+      <Description>No Op CDL</Description>
+      <Slope>1.00000 1.00000 1.00000</Slope>
+      <Offset>0.00000 0.00000 0.00000</Offset>
+      <Power>1.00000 1.00000 1.00000</Power>
+   </SOPNode>
+   <SatNode>
+      <Saturation>1.00000</Saturation>
+   </SatNode>
+</ColorCorrection>