Adsk contrib - Processor cache does not detect changes in cccid (#1726)

* For Looks that has a FileTransform and for Colorspace with FileTransfrom, add the CCCID to the processor's cache key for that transform. Signed-off-by: Cédrik Fuoco <[email protected]> * Removing the workaround in the related unit tests and fixing the issue by adding the environment variable to the context using setStringVar. The processor's cache key is using the context cache ID which has all the context variables taken into account. Signed-off-by: Cédrik Fuoco <[email protected]> * Now using addStringVars and creating a new context instead of reusing the one used for the filename. Signed-off-by: Cédrik Fuoco <[email protected]> * Adding cccid to the context when there are no context variable. Signed-off-by: Cédrik Fuoco <[email protected]> * Adding a few unit tests to test that the processor is different when changing the FileTransform's CCCID. Signed-off-by: Cédrik Fuoco <[email protected]> * Using setStringVar to set CCNUM context variable in unit test. Signed-off-by: Cédrik Fuoco <[email protected]> * Adding a test in FileTransform to test CollectContextVariables directly. Signed-off-by: Cédrik Fuoco <[email protected]> * Minor tweaks for the unit test Signed-off-by: Cédrik Fuoco <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]> Co-authored-by: Doug Walker <[email protected]> (cherry picked from commit 4fa7750) Signed-off-by: Cédrik Fuoco <[email protected]>
AcademySoftwareFoundation · Jan 5, 2023 · 833a003 · 833a003
1 parent a5b91ce
commit 833a003
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 0 deletions.
diff --git a/src/OpenColorIO/transforms/FileTransform.cpp b/src/OpenColorIO/transforms/FileTransform.cpp
@@ -240,6 +240,16 @@ bool CollectContextVariables(const Config &,
         usedContextVars->addStringVars(ctxFilepath);
     }
 
+    // Check if the CCCID is using a context variable and add it to the context if that's the case.
+    ContextRcPtr ctxCCCID = Context::Create();
+    const char * cccid = tr.getCCCId();
+    std::string resolvedCCCID = context.resolveStringVar(cccid, ctxCCCID);
+    if (0 != strcmp(resolvedCCCID.c_str(), cccid))
+    {
+        foundContextVars = true;
+        usedContextVars->addStringVars(ctxCCCID);
+    }
+
     return foundContextVars;
 }
 

diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp
@@ -7610,6 +7610,61 @@ OCIO_ADD_TEST(Config, context_variables_typical_use_cases)
                           cfg->getProcessor(ctx2, "cs1", "cs2").get());
         }
     }
+
+
+    // Case 7 - Context variables in the FileTransform's CCCID.
+    {
+        static const std::string CONFIG = 
+            "ocio_profile_version: 2\n"
+            "\n"
+            "environment:\n"
+            "  CCPREFIX: cc\n"
+            "\n"
+            "search_path: " + OCIO::GetTestFilesDir() + "\n"
+            "\n"
+            "roles:\n"
+            "  default: cs1\n"
+            "\n"
+            "displays:\n"
+            "  disp1:\n"
+            "    - !<View> {name: view1, colorspace: cs2}\n"
+            "\n"
+            "colorspaces:\n"
+            "  - !<ColorSpace>\n"
+            "    name: cs1\n"
+            "\n"
+            "  - !<ColorSpace>\n"
+            "    name: cs2\n"
+            "    from_scene_reference: !<FileTransform> {src: cdl_test1.ccc, cccid: $CCPREFIX00$CCNUM}\n";
+
+        std::istringstream iss;
+        iss.str(CONFIG);
+
+        OCIO::ConfigRcPtr cfg;
+        OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss)->createEditableCopy());
+        OCIO_CHECK_NO_THROW(cfg->validate());
+
+        OCIO::ConstTransformRcPtr ctf = cfg->getColorSpace("cs2")->getTransform(
+            OCIO::COLORSPACE_DIR_FROM_REFERENCE
+        );
+        OCIO_REQUIRE_ASSERT(ctf);
+
+        OCIO::ContextRcPtr ctx = cfg->getCurrentContext()->createEditableCopy();
+
+        ctx->setStringVar("CCNUM", "01");
+        OCIO::ConstProcessorRcPtr p1 = cfg->getProcessor(ctx, ctf, OCIO::TRANSFORM_DIR_FORWARD);
+
+        ctx->setStringVar("CCNUM", "02");
+        OCIO::ConstProcessorRcPtr p2 = cfg->getProcessor(ctx, ctf, OCIO::TRANSFORM_DIR_FORWARD);
+
+        ctx->setStringVar("CCNUM", "03");
+        OCIO::ConstProcessorRcPtr p3 = cfg->getProcessor(ctx, ctf, OCIO::TRANSFORM_DIR_FORWARD);
+
+        // All three processors should be different.
+        OCIO_CHECK_NE(p1.get(), p2.get());
+        OCIO_CHECK_NE(p1.get(), p3.get());
+        OCIO_CHECK_NE(p2.get(), p3.get());
+    }
 }
 
 OCIO_ADD_TEST(Config, virtual_display)

diff --git a/tests/cpu/transforms/FileTransform_tests.cpp b/tests/cpu/transforms/FileTransform_tests.cpp
@@ -431,4 +431,56 @@ OCIO_ADD_TEST(FileTransform, context_variables)
 
     // A basic check to validate that context variables are correctly used. 
     OCIO_CHECK_NO_THROW(cfg->getProcessor(ctx, file, OCIO::TRANSFORM_DIR_FORWARD));
+
+
+    {
+        // Case 4 - The 'cccid' now contains a context variable
+        static const std::string CONFIG = 
+            "ocio_profile_version: 2\n"
+            "\n"
+            "environment:\n"
+            "  CCPREFIX: cc\n"
+            "  CCNUM: 02\n"
+            "\n"
+            "search_path: " + OCIO::GetTestFilesDir() + "\n"
+            "\n"
+            "roles:\n"
+            "  default: cs1\n"
+            "\n"
+            "displays:\n"
+            "  disp1:\n"
+            "    - !<View> {name: view1, colorspace: cs2}\n"
+            "\n"
+            "colorspaces:\n"
+            "  - !<ColorSpace>\n"
+            "    name: cs1\n"
+            "\n"
+            "  - !<ColorSpace>\n"
+            "    name: cs2\n"
+            "    from_scene_reference: !<FileTransform> {src: cdl_test1.ccc, cccid: $CCPREFIX00$CCNUM}\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());
+
+            ctx = cfg->getCurrentContext()->createEditableCopy();
+            OCIO_CHECK_NO_THROW(ctx->setStringVar("CCNUM", "01"));
+
+            usedContextVars = OCIO::Context::Create(); // New & empty instance.
+            OCIO::ConstTransformRcPtr tr1 = cfg->getColorSpace("cs2")->getTransform(
+                OCIO::COLORSPACE_DIR_FROM_REFERENCE
+            );
+            OCIO::ConstFileTransformRcPtr fTr1 = OCIO::DynamicPtrCast<const OCIO::FileTransform>(tr1);
+            OCIO_CHECK_ASSERT(fTr1);
+
+            OCIO_CHECK_ASSERT(CollectContextVariables(*cfg, *ctx, *fTr1, usedContextVars));
+            OCIO_CHECK_EQUAL(2, usedContextVars->getNumStringVars());
+            OCIO_CHECK_EQUAL(std::string("CCPREFIX"), usedContextVars->getStringVarNameByIndex(0));
+            OCIO_CHECK_EQUAL(std::string("cc"), usedContextVars->getStringVarByIndex(0));
+            OCIO_CHECK_EQUAL(std::string("CCNUM"), usedContextVars->getStringVarNameByIndex(1));
+            OCIO_CHECK_EQUAL(std::string("01"), usedContextVars->getStringVarByIndex(1));
+    }
 }