[python][bug] `GradingPrimary.saturation` broken when used in solo.

[MrLixm]

Hello,
I have noticed that when using a GradingPrimary instance with only the saturation modified, when ported to a GradingPrimaryTransform and then applied to an image, the saturation change is not registered and the output image is unchanged.
Now if I only modified by a very little another attribute like GradingPrimary.exposure, the saturation is registered and applied to the input.

You can find a demo in the following snippet, where the test_sat_only will be the only one to fail.
(only the TestGradingPrimaryTransform.config_path need to be updated with any valid OCIO config)

# python>3
import unittest
from pathlib import Path
from typing import Tuple

import PyOpenColorIO as ocio
import numpy
import numpy.testing


def make_img(color: Tuple[float, float, float]):
    """Create a 64x64 RGB image with the given color."""
    return numpy.full((64, 64, 3), color, dtype=numpy.float32)


class TestGradingPrimaryTransform(unittest.TestCase):

    config_path = Path(
        r"YOURCONFIGPATH\config.ocio"
    )

    def setUp(self) -> None:
        self.config: ocio.Config = ocio.Config().CreateFromFile(str(self.config_path))
        self.img1 = make_img((0.5, 0.1, 0.1))
        self.gp: ocio.GradingPrimary = ocio.GradingPrimary(ocio.GRADING_LIN)
        return

    def tearDown(self) -> None:
        self.config = None
        self.img1 = None
        return

    def _apply_gp_on_img(self):

        tsfm_gp = ocio.GradingPrimaryTransform(
            self.gp,
            ocio.GRADING_LIN,
            False,
        )

        proc: ocio.Processor = self.config.getProcessor(tsfm_gp)
        proc: ocio.CPUProcessor = proc.getDefaultCPUProcessor()

        proc.applyRGB(self.img1)

        return

    def test_sat_only(self):

        # MODIFY
        p_sat = 2.0

        self.gp.saturation = p_sat

        self._apply_gp_on_img()

        expected = make_img((0.815, 0.015, 0.015))
        numpy.testing.assert_almost_equal(
            self.img1,
            expected,
            4,
            f"img1 is actually {self.img1[1][1]} while expected is {expected[1][1]}",
        )

    def test_sat_expo(self):

        # MODIFY
        p_sat = 2.0
        p_expo = 0.02  # in "stops", passthrough is 0

        self.gp.saturation = p_sat
        self.gp.exposure = ocio.GradingRGBM(0, 0, 0, p_expo)

        self._apply_gp_on_img()

        expected = make_img((0.82637, 0.01521, 0.01521))
        numpy.testing.assert_almost_equal(
            self.img1,
            expected,
            4,
            f"img1 is actually {self.img1[1][1]} while expected is {expected[1][1]}",
        )

    def test_offset_only(self):

        # MODIFY
        p_offset = 0.4

        self.gp.offset = ocio.GradingRGBM(0, 0, 0, p_offset)

        self._apply_gp_on_img()

        expected = make_img((0.9, 0.5, 0.5))
        numpy.testing.assert_almost_equal(
            self.img1,
            expected,
            4,
            f"img1 is actually {self.img1[1][1]} while expected is {expected[1][1]}",
        )

    def test_clampwhite_only(self):

        # MODIFY
        p_clampwhite = 0.4

        self.gp.clampWhite = p_clampwhite

        self._apply_gp_on_img()

        expected = make_img((0.4, 0.1, 0.1))
        numpy.testing.assert_almost_equal(
            self.img1,
            expected,
            4,
            f"img1 is actually {self.img1[1][1]} while expected is {expected[1][1]}",
        )


if __name__ == "__main__":
    unittest.main()

CONTEXT: Windows10, OCIO 2.1.0

Cheers.
Liam.

[MrLixm]

As a workaround for now I am detecting if GradingPrimary has been modified from default value and if only the saturation
is non-default I am simply doing settings an extreme value for clampBlack (which trigger the saturation) :

        gp = ocio.GradingPrimary(self.grading_space)
        gp.contrast = to_rgbm_ocio(self.contrast)
        gp.lift = to_rgbm_ocio(self.lift)
        gp.offset = to_rgbm_ocio(self.offset)
        gp.pivot = self.pivot
        gp.saturation = self.saturation

        # HACK saturation :
        # https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/1642
        if self.is_modified_sat_only:
            gp.clampBlack = -150

It would be cool by the way to have a method like GradingPrimary.is_default() to check if some of its value has been modified.

[MrLixm]

Disclaimer: I don't speak C++, I can't unfortunately make a PR for this

So I had a look at the source code, I don't know if this is intentional but seems saturation is excluded from the localBypass
checks where value affecting it are only clampBlack, clampWhite, contrast, exposure, offset.

OpenColorIO/src/OpenColorIO/ops/gradingprimary/GradingPrimary.cpp

Lines 195 to 197 in b008779

	m_localBypass = m_localBypass && m_isPowerIdentity &&
	m_exposure[0] == 1.f && m_exposure[1] == 1.f && m_exposure[2] == 1.f &&
	m_offset[0] == 0.f && m_offset[1] == 0.f && m_offset[2] == 0.f;

This is of course for the GRADING_LINEAR mode which from the doc state :

Linear controls: Exposure, Contrast, Pivot, Offset, Saturation, Black Clip, White Clip.

So indeed saturation which is supported seems to be forgotten.

This seems to also be valid for GRADING_LOG and GRADING_VIDEO.

Cheers.
Liam.

[doug-walker]

Thank you @MrLixm for your thorough work investigating this issue. We will make sure to look into this!

[remia]

We ran into this while using GradingPrimary to implement a CDL with only saturation adjustment.

Looks like we missed the GradingPrimary fixes on the latest round of patch releases.