Fix NamedTransform context var issue (#1905)

Signed-off-by: Doug Walker <doug.walker@autodesk.com>
AcademySoftwareFoundation · Nov 17, 2023 · 4d64b32 · 4d64b32
1 parent ffd0f70
commit 4d64b32
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 7 deletions.
diff --git a/src/OpenColorIO/transforms/ColorSpaceTransform.cpp b/src/OpenColorIO/transforms/ColorSpaceTransform.cpp
@@ -184,15 +184,15 @@ void BuildColorSpaceOps(OpRcPtrVec & ops,
 
     if (!src)
     {
-        srcNamedTransform = config.getNamedTransform(srcName.c_str());
+        srcNamedTransform = config.getNamedTransform( context->resolveStringVar(srcName.c_str()) );
         if (!srcNamedTransform)
         {
             ThrowMissingCS(srcName.c_str());
         }
     }
     if (!dst)
     {
-        dstNamedTransform = config.getNamedTransform(dstName.c_str());
+        dstNamedTransform = config.getNamedTransform( context->resolveStringVar(dstName.c_str()) );
         if (!dstNamedTransform)
         {
             ThrowMissingCS(dstName.c_str());
@@ -393,6 +393,31 @@ void BuildReferenceConversionOps(OpRcPtrVec & ops,
     }
 }
 
+bool CollectContextVariables(const Config & config, 
+                             const Context & context,
+                             ConstNamedTransformRcPtr & nt,
+                             ContextRcPtr & usedContextVars)
+{
+    bool foundContextVars = false;
+
+    if (nt)
+    {
+        ConstTransformRcPtr to = nt->getTransform(TRANSFORM_DIR_FORWARD);
+        if (to && CollectContextVariables(config, context, to, usedContextVars))
+        {
+            foundContextVars = true;
+        }
+
+        ConstTransformRcPtr from = nt->getTransform(TRANSFORM_DIR_INVERSE);
+        if (from && CollectContextVariables(config, context, from, usedContextVars))
+        {
+            foundContextVars = true;
+        }
+    }
+
+    return foundContextVars;
+}
+
 bool CollectContextVariables(const Config & config, 
                              const Context & context,
                              ConstColorSpaceRcPtr & cs,
@@ -441,15 +466,37 @@ bool CollectContextVariables(const Config & config,
     }
 
     ConstColorSpaceRcPtr src = config.getColorSpace(srcName.c_str());
-    if (CollectContextVariables(config, context, src, usedContextVars))
+    if (src)
     {
-        foundContextVars = true;
+        if (CollectContextVariables(config, context, src, usedContextVars))
+        {
+            foundContextVars = true;
+        }
+    }
+    else
+    {
+        ConstNamedTransformRcPtr nt_src = config.getNamedTransform(srcName.c_str());
+        if (CollectContextVariables(config, context, nt_src, usedContextVars))
+        {
+            foundContextVars = true;
+        }
     }
 
     ConstColorSpaceRcPtr dst = config.getColorSpace(dstName.c_str());
-    if (CollectContextVariables(config, context, dst, usedContextVars))
+    if (dst)
     {
-        foundContextVars = true;
+        if (CollectContextVariables(config, context, dst, usedContextVars))
+        {
+            foundContextVars = true;
+        }
+    }
+    else
+    {
+        ConstNamedTransformRcPtr nt_dst = config.getNamedTransform(dstName.c_str());
+        if (CollectContextVariables(config, context, nt_dst, usedContextVars))
+        {
+            foundContextVars = true;
+        }
     }
 
     return foundContextVars;

diff --git a/tests/cpu/Config_tests.cpp b/tests/cpu/Config_tests.cpp
@@ -1452,7 +1452,7 @@ OCIO_ADD_TEST(Config, context_variable_with_colorspacename)
 
         // Set $VAR3 and check again.
 
-        OCIO_CHECK_NO_THROW(cfg->addEnvironmentVar("VAR3", "cs1"));
+        OCIO_CHECK_NO_THROW(cfg->addEnvironmentVar("VAR3", "file.clf"));
         OCIO_CHECK_NO_THROW(cfg->validate());
     }
 
@@ -1537,6 +1537,31 @@ OCIO_ADD_TEST(Config, context_variable_with_colorspacename)
                               OCIO::Exception,
                               "Color space '$VAR3' could not be found.");
     }
+
+    // Repeat the test using a NamedTransform for one of the color spaces.
+
+    {
+        std::string configStr 
+            = std::string(CONFIG)
+            + "    from_scene_reference: !<ColorSpaceTransform> {src: $VAR3, dst: cs1}\n"
+            + "named_transforms:\n"
+            + "  - !<NamedTransform>\n"
+            + "    name: nt1\n"
+            + "    transform: !<RangeTransform> {min_in_value: 0, min_out_value: 0}\n";
+
+        std::istringstream iss;
+        iss.str(configStr);
+
+        OCIO::ConfigRcPtr cfg;
+        OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss)->createEditableCopy());
+
+        OCIO_CHECK_NO_THROW(cfg->addEnvironmentVar("VAR3", "nt1"));
+        OCIO_CHECK_NO_THROW(cfg->validate());
+
+        OCIO::ContextRcPtr ctx;
+        OCIO_CHECK_NO_THROW(ctx = cfg->getCurrentContext()->createEditableCopy());
+        OCIO_CHECK_NO_THROW(cfg->getProcessor(ctx, "cs1", "cs2"));
+    }
 }
 
 OCIO_ADD_TEST(Config, context_variable_with_role)

diff --git a/tests/cpu/transforms/ColorSpaceTransform_tests.cpp b/tests/cpu/transforms/ColorSpaceTransform_tests.cpp
@@ -834,6 +834,24 @@ OCIO_ADD_TEST(ColorSpaceTransform, context_variables)
     OCIO_CHECK_EQUAL(std::string("ENV1"), usedContextVars->getStringVarNameByIndex(0));
     OCIO_CHECK_EQUAL(std::string("exposure_contrast_linear.ctf"),
                      usedContextVars->getStringVarByIndex(0));
+
+    // Case 5 - Context variable indirectly used via a NamedTransform.
+
+    OCIO::NamedTransformRcPtr namedTransform = OCIO::NamedTransform::Create();
+    namedTransform->setName("nt");
+    OCIO::FileTransformRcPtr file2 = OCIO::FileTransform::Create();
+    file2->setSrc("$ENV1");
+    namedTransform->setTransform(file2, OCIO::TRANSFORM_DIR_FORWARD);
+    OCIO_CHECK_NO_THROW(cfg->addNamedTransform(namedTransform));
+
+    // 'cst' now uses 'nt' which is a NamedTransform whose transform uses a context variable.
+    cst->setSrc("nt");
+    usedContextVars = OCIO::Context::Create(); // New & empty instance.
+    OCIO_CHECK_ASSERT(OCIO::CollectContextVariables(*cfg, *ctx, *cst, usedContextVars));
+    OCIO_CHECK_EQUAL(1, usedContextVars->getNumStringVars());
+    OCIO_CHECK_EQUAL(std::string("ENV1"), usedContextVars->getStringVarNameByIndex(0));
+    OCIO_CHECK_EQUAL(std::string("exposure_contrast_linear.ctf"),
+                     usedContextVars->getStringVarByIndex(0));
 }
 
 // Please see (Config, named_transform_processor) in NamedTransform_tests.cpp for coverage of