Merge branch 'master' into fix-static-pystring

docs/quick_start/installation.rst

@@ -122,8 +122,8 @@ When using existing system libraries, the following CMake variables can be
 defined to hint at non-standard install locations and preference of shared
 or static linking:
 
-- ``-DExpat_ROOT=<path>`` (include and/or library root dir)
-- ``-DExpat_STATIC_LIBRARY=ON`` (prefer static lib)
+- ``-Dexpat_ROOT=<path>`` (include and/or library root dir)
+- ``-Dexpat_STATIC_LIBRARY=ON`` (prefer static lib)
 - ``-Dyaml-cpp_ROOT=<path>`` (include and/or library root dir)
 - ``-Dyaml-cpp_STATIC_LIBRARY=ON`` (prefer static lib)
 - ``-DHalf_ROOT=<path>`` (include and/or library root dir)
@@ -132,7 +132,7 @@ or static linking:
 - ``-Dpystring_STATIC_LIBRARY=ON`` (prefer static lib)
 - ``-Dlcms2_ROOT=<path>`` (include and/or library root dir)
 - ``-Dlcms2_STATIC_LIBRARY=ON`` (prefer static lib)
-- ``-pybind11_ROOT=<path>`` (include and/or library root dir)
+- ``-Dpybind11_ROOT=<path>`` (include and/or library root dir)
 - ``-DPython_EXECUTABLE=<path>`` (Python executable)
 
 To hint at Python package locations, add paths to the ``PYTHONPATH`` 
@@ -292,9 +292,9 @@ for compiling on Windows as provided by `@hodoulp <https://github.com/hodoulp>`_
         -DOCIO_USE_SSE=ON ^
         -DOCIO_WARNING_AS_ERROR=ON ^
         -DOCIO_BUILD_PYTHON=%OCIO_BUILD_PYTHON% ^
-        -DPython_LIBRARY=%PYTHONPATH%\libs\python27.lib ^
-        -DPython_INCLUDE_DIR=%PYTHONPATH%\include ^
-        -DPython_EXECUTABLE=%PYTHONPATH%\python.exe ^
+        -DPython_LIBRARY=%PYTHON_PATH%\libs\python27.lib ^
+        -DPython_INCLUDE_DIR=%PYTHON_PATH%\include ^
+        -DPython_EXECUTABLE=%PYTHON_PATH%\python.exe ^
         -DOCIO_BUILD_JAVA=OFF ^
         -DCMAKE_PREFIX_PATH=%COMPILED_THIRD_PARTY_HOME%\OpenImageIO-1.9.0;%COMPILED_THIRD_PARTY_HOME%/ilmbase-2.2.0 ^
         %OCIO_PATH%

share/clf/compare_clf_test_frames.py

@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright Contributors to the OpenColorIO Project.
+
+# Use OpenImageIO's oiiotool to compare the reference image set to the actual image set produced
+# by a CLF implementation.  
+#
+# The script iterates over the images in the directory and prints the oiiotool command being run.
+# It will then print either 'Test passed' or 'Test failed' after each command and then at the end
+# will summarize with 'All tests passed' or 'These tests failed' with a list of the failed images.
+#
+# Usage:
+# > python compare_clf_test_frames.py <PATH-TO-REFERENCE-IMAGES> <PATH-TO-ACTUAL-IMAGES>
+
+# This script is python 2.7 and python 3 compatible.
+
+import os
+import subprocess
+import tempfile
+
+def process_frames( ref_path, act_path ):
+
+    # Check the arguments are as expected.
+    if not os.path.isdir( ref_path ):
+        raise ValueError( "Reference image directory must exist: " + ref_path )
+    if not os.path.isdir( act_path ):
+        raise ValueError( "Actual image directory must exist: " + act_path )
+
+    # Take the aim and actual images and compute abs(aim - act) / max(abs(aim), 0.1).
+    base_cmd = 'oiiotool %s --dup %s --absdiff --swap --abs --maxc 0.1 --div '
+
+    # Mask out the Inf and NaN values in the CLF test kit target image.
+    box_cmd = '--box:color=0,0,0:fill=1 1008,771,1023,798 --box:color=0,0,0:fill=1 0,1023,1,1023 '
+
+    # Error out if there are any new NaNs generated (there should not be).
+    nan_err_cmd = '--fixnan error '
+
+    # Print out how many pixels are greater than the test threshold of 0.002.
+    range_cmd = '--rangecheck 0,0,0 .002,.002,.002 '
+
+    # Oiiotool seems to need a -o to avoid issuing a useless warning, so add an output file
+    # even though it is not used.  (Note that trying to write to /dev/null here doesn't work.)
+    avoid_warning_cmd = '-o ' + os.path.join(tempfile.gettempdir(), 'tmp.exr') + ' '
+
+    oiio_cmd = base_cmd + box_cmd + nan_err_cmd + range_cmd + avoid_warning_cmd
+
+    # Iterate over each pair of test images.
+    failed_tests = []
+    for f in sorted(os.listdir( ref_path )):
+        fname, ext = os.path.splitext( f )
+        if ext == '.exr':
+
+            # Build the full path to the files.
+            ref = os.path.join( ref_path, f )
+            act = os.path.join( act_path, f )
+
+            # Build the command.
+            cmd = oiio_cmd % (ref, act)
+
+            print('');  print( cmd )
+
+            # Process the image.
+            try:
+                result = subprocess.check_output(cmd, shell=True)
+
+                ind = str(result).find('0  > .002,.002,.002')
+                if ind > -1:
+                    print('** Test passed **')
+                else:
+                    failed_tests.append(f)
+                    print('\n** TEST FAILED **')
+                    print(result)
+
+            except:
+                failed_tests.append(f)
+                print('\n** TEST FAILED **')
+                print(result)
+
+    if len(failed_tests) == 0:
+        print("\n\nALL TESTS PASSED SUCCESSFULLY!\n\n")
+    else:
+        print("\n\nTHESE TESTS FAILED!\n")
+        for s in failed_tests:
+            print(s)
+        print('')
+
+
+if __name__=='__main__':
+    import sys
+    if len( sys.argv ) != 3:
+        raise ValueError( "USAGE: python  compare_clf_test_frames.py  <REF_IMAGE_DIR>  <ACTUAL_IMAGE_DIR>" )
+    process_frames( sys.argv[1], sys.argv[2] )

share/clf/process_clf_test_frames.py

@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright Contributors to the OpenColorIO Project.
+
+# Use OpenColorIO to apply the CLF files from the CLF test kit to the CLF target image
+# and produce a directory of processed OpenEXR images at the specified location.
+# Run the script with "-h" for usage information.
+
+# This script is python 2.7 and python 3 compatible.
+
+import os
+import argparse
+
+def process_frames( options ):
+
+    dst_path = options.dst_dir
+    use_gpu = options.gpu
+    opt_level = options.opt
+
+    # Check the arguments are as expected.
+    if not os.path.exists( dst_path ):
+        os.mkdir( dst_path )
+    if not os.path.isdir( dst_path ):
+        raise ValueError( "Destination path must be a directory: " + dst_path )
+
+    # Get the path to the CLF target image, relative to the path of this script.
+    script_path = os.path.abspath( __file__ )
+    parts = script_path.split( os.sep )
+    ocio_base_path = os.path.join( os.sep, *parts[0:-3] )
+    src_image = os.path.join( ocio_base_path, 'share', 'clf', 'CLF_testimage.exr' )
+
+    # Get the path to the CLF files, relative to the path of this script.
+    clf_path = os.path.join( ocio_base_path, 'tests', 'data', 'files', 'clf' )
+
+    # Set the optimization level. None or lossless avoids the fast SSE log/exponent.
+    # (Note that the decimal value is available by simply printing the enum in Python.)
+    if (opt_level == 'none') or (opt_level is None):
+        # For default for this script, use no optimization rather than OCIO's default optimization
+        # in order to apply the operators exactly as they appear in the CLF file with no attempt
+        # to speed up the processing.
+        print( 'Optimization level: None' )
+        os.environ["OCIO_OPTIMIZATION_FLAGS"] = "0"
+    elif opt_level == 'lossless':
+        print( 'Optimization level: Lossless' )
+        os.environ["OCIO_OPTIMIZATION_FLAGS"] = "144457667"
+    elif opt_level == 'default':
+        print( 'Optimization level: Default' )
+    else:
+        raise ValueError( 'Unexpected --opt argument.' )
+
+    # TODO:  Add an option to turn on only SSE without removing any ops.
+
+    if use_gpu:
+        print( 'Processing on the GPU\n' )
+        cmd_base = 'ocioconvert --gpu --lut %s %s %s'
+    else:
+        print( 'Processing on the CPU\n' )
+        cmd_base = 'ocioconvert --lut %s %s %s'
+
+    # Iterate over each legal CLF file in the suite.
+    for f in os.listdir( clf_path ):
+        fname, ext = os.path.splitext( f )
+        if ext == '.clf':
+
+            # Build the full path to the file.
+            p = os.path.join( clf_path, f )
+
+            # Build the name of the destination image.
+            dst_image = os.path.join( dst_path, fname + '.exr' )
+
+            # Build the command.
+            cmd = cmd_base % (p, src_image, dst_image)
+            print('=================');  print( cmd )
+
+            # Process the image.
+            os.system( cmd )
+
+
+if __name__=='__main__':
+    import sys
+
+    import argparse
+    parser = argparse.ArgumentParser(description='Process CLF test images using OCIO.')
+    parser.add_argument('dst_dir',
+                        help='Path to a directory where the result images will be stored.')
+    parser.add_argument('--gpu', action='store_true',
+                        help='Process using the GPU rather than the CPU.')
+    parser.add_argument('--opt', choices=['none','lossless','default'],
+                        help='Specify the OCIO optimization level. If not specified, "none" will be used.')
+    options = parser.parse_args(sys.argv[1:])
+
+    process_frames(options)
+

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
-

src/OpenColorIO/ops/lut1d/Lut1DOpGPU.cpp

@@ -262,9 +262,9 @@ void GetLut1DGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator,
             }
             else
             {
-                // Need min() to protect against f > 1 causing a bogus x value.
-                // min( f, 1.) * (dim - 1)
-                ss.newLine() << "float dep = min(f, 1.0) * " << float(length - 1) << ";";
+                // Need clamp() to protect against f outside [0,1] causing a bogus x value.
+                // clamp( f, 0., 1.) * (dim - 1)
+                ss.newLine() << "float dep = clamp(f, 0.0, 1.0) * " << float(length - 1) << ";";
 
                 ss.newLine() << ss.float2Decl("retVal") << ";";
                 // float(int( dep / (width-1) ))

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