Protect against empty look result color space (#1437) (#1443)

Signed-off-by: Rémi Achard <[email protected]>

Co-authored-by: Patrick Hodoul <[email protected]>

Co-authored-by: Rémi Achard <[email protected]>

Author: hodoulp

src/OpenColorIO/apphelpers/LegacyViewingPipeline.cpp

@@ -216,7 +216,7 @@ ConstProcessorRcPtr LegacyViewingPipelineImpl::getProcessor(const ConstConfigRcP
         viewTransform = config->getViewTransform(viewTransformName.c_str());
     }
 
-    // NB: If the viewTranform is present, then displayColorSpace is a true display color space
+    // NB: If the viewTransform is present, then displayColorSpace is a true display color space
     // rather than a traditional color space.
     const std::string name{ config->getDisplayViewColorSpaceName(display.c_str(), view.c_str()) };
     // A shared view containing a view transform may set the color space to USE_DISPLAY_NAME,
@@ -344,16 +344,21 @@ ConstProcessorRcPtr LegacyViewingPipelineImpl::getProcessor(const ConstConfigRcP
         const char * outCS = skipColorSpaceConversions ? inCS :
                              LookTransform::GetLooksResultColorSpace(configIn, context,
                                                                      looks.c_str());
-        auto lt = LookTransform::Create();
-        lt->setSrc(inCS);
-        lt->setDst(outCS);
-        lt->setLooks(looks.c_str());
-        lt->setSkipColorSpaceConversion(skipColorSpaceConversions);
 
-        group->appendTransform(lt);
+        // Resulting color space could be null in case of a noop look.
+        if (outCS && *outCS)
+        {
+            auto lt = LookTransform::Create();
+            lt->setSrc(inCS);
+            lt->setDst(outCS);
+            lt->setLooks(looks.c_str());
+            lt->setSkipColorSpaceConversion(skipColorSpaceConversions);
+
+            group->appendTransform(lt);
 
-        // Adjust display transform input color space.
-        dt->setSrc(outCS);
+            // Adjust display transform input color space.
+            dt->setSrc(outCS);
+        }
     }
 
     if (m_channelView)
@@ -425,7 +430,7 @@ std::ostream & operator<<(std::ostream & os, const LegacyViewingPipeline & pipel
         {
             os << ", ";
         }
-        os << "LooksOveerideEnabled";
+        os << "LooksOverrideEnabled";
         first = false;
     }
     const std::string lo{ pipeline.getLooksOverride() };
@@ -435,11 +440,10 @@ std::ostream & operator<<(std::ostream & os, const LegacyViewingPipeline & pipel
         {
             os << ", ";
         }
-        os << "LooksOveeride: " << lo;
+        os << "LooksOverride: " << lo;
         first = false;
     }
     return os;
 }
 
 } // namespace OCIO_NAMESPACE
-

tests/cpu/apphelpers/LegacyViewingPipeline_tests.cpp

@@ -900,3 +900,56 @@ OCIO_ADD_TEST(LegacyViewingPipeline, fullPipelineNoLook)
         OCIO_REQUIRE_ASSERT(ec);
     }
 }
+
+OCIO_ADD_TEST(LegacyViewingPipeline, processorWithNoOpLook)
+{
+    //
+    // Validate LegacyViewingPipeline::getProcessor when a noop look override
+    // is specified.
+    //
+
+    std::istringstream is(category_test_config);
+
+    OCIO::ConstConfigRcPtr cfg;
+    OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(is));
+    OCIO_CHECK_NO_THROW(cfg->validate());
+
+    OCIO::DisplayViewTransformRcPtr dt = OCIO::DisplayViewTransform::Create();
+    dt->setDisplay("DISP_2");
+    dt->setView("VIEW_2");
+    dt->setSrc("in_1");
+
+    OCIO::LegacyViewingPipelineRcPtr vp = OCIO::LegacyViewingPipeline::Create();
+    vp->setDisplayViewTransform(dt);
+    vp->setLooksOverrideEnabled(true);
+    vp->setLooksOverride("look_noop");
+
+    // Processor in forward direction.
+
+    OCIO::ConstProcessorRcPtr proc;
+    OCIO_CHECK_NO_THROW(proc = vp->getProcessor(cfg, cfg->getCurrentContext()));
+    OCIO_REQUIRE_ASSERT(proc);
+
+    OCIO::GroupTransformRcPtr groupTransform;
+    OCIO_CHECK_NO_THROW(groupTransform = proc->createGroupTransform());
+
+    OCIO_REQUIRE_ASSERT(groupTransform);
+    OCIO_CHECK_NO_THROW(groupTransform->validate());
+
+    // Repeat in inverse direction.
+
+    dt->setDirection(OCIO::TRANSFORM_DIR_INVERSE);
+    vp->setDisplayViewTransform(dt);
+    vp->setLooksOverrideEnabled(true);
+    vp->setLooksOverride("look_noop");
+
+    // Processor in inverse direction.
+
+    OCIO_CHECK_NO_THROW(proc = vp->getProcessor(cfg, cfg->getCurrentContext()));
+    OCIO_REQUIRE_ASSERT(proc);
+
+    OCIO_CHECK_NO_THROW(groupTransform = proc->createGroupTransform());
+
+    OCIO_REQUIRE_ASSERT(groupTransform);
+    OCIO_CHECK_NO_THROW(groupTransform->validate());
+}

tests/cpu/apphelpers/configs.data

@@ -49,6 +49,11 @@ looks:
     process_space: log_1
     transform: !<CDLTransform> {slope: [1.2, 2.2, 1.2]}
 
+  - !<Look>
+    name: look_noop
+    process_space: log_1
+    transform: !<CDLTransform> {slope: [1, 1, 1], offset: [0, 0, 0], power: [1, 1, 1]}
+
 colorspaces:
   - !<ColorSpace>
     name: raw