Skip to content

Adsk Contrb - Fix pass-thru exponent composition issue #2154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
doug-walker merged 6 commits into from
May 18, 2025
Merged

Adsk Contrb - Fix pass-thru exponent composition issue #2154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
58565ca
Fix gamma composition issue
doug-walker May 3, 2025
d1d4e2d
Adjust docs build settings to fix CI
doug-walker May 4, 2025
bbd3368
Adjust docs build settings to fix CI
doug-walker May 4, 2025
a956836
Revert CI change
doug-walker May 17, 2025
49af1f1
Minor comment tweak
doug-walker May 17, 2025
d36e862
Merge branch 'main' into walker/gamma-comp-fix
doug-walker May 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/OpenColorIO/OpenColorIO.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ class OCIOEXPORT Config
const char * getDefaultView(const char * display, const char * colorspaceName) const;

/**
* Return the number of views attached to the display including the number of
* Return the number of active views attached to the display including the number of
* shared views if any. Return 0 if display does not exist.
*/
int getNumViews(const char * display) const;
Expand Down
17 changes: 11 additions & 6 deletions src/OpenColorIO/ops/gamma/GammaOpData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,20 +659,24 @@ namespace
GammaOpData::Style CombineBasicStyles(GammaOpData::Style a, GammaOpData::Style b)
{
// This function assumes that mayCompose was called on the inputs and returned true.
// The logic here is only valid for that situation.
// The logic here is only valid for that situation. There is no intent to preserve
// the direction, a forward style is always returned.
if (a == GammaOpData::BASIC_FWD || a == GammaOpData::BASIC_REV ||
b == GammaOpData::BASIC_FWD || b == GammaOpData::BASIC_REV)
{
// If either a or b is a BASIC style, that is the combined style since the
// BASIC style clamps negatives, so the combination must also clamp.
return GammaOpData::BASIC_FWD;
}
else if (a == GammaOpData::BASIC_MIRROR_FWD || a == GammaOpData::BASIC_MIRROR_REV)
{
// b BASIC_MIRROR.
// Neither a or b is a BASIC style, so it may only be MIRROR or PASS_THRU, but
// mayCompose will not allow b to be PASS_THRU in this case, both are MIRROR.
return GammaOpData::BASIC_MIRROR_FWD;
}
else // a is BASIC_PASS_THRU (ensured by mayCompose).
else
{
// b BASIC_PASS_THRU.
// Both a and b are BASIC_PASS_THRU as a consequence of mayCompose being true.
return GammaOpData::BASIC_PASS_THRU_FWD;
}
}
Expand Down Expand Up @@ -715,7 +719,7 @@ GammaOpDataRcPtr GammaOpData::compose(const GammaOpData & B) const
double b1 = getBlueParams()[0];
double a1 = getAlphaParams()[0];

if (styleA == BASIC_REV || styleA == BASIC_MIRROR_REV || styleA == BASIC_PASS_THRU_FWD)
if (styleA == BASIC_REV || styleA == BASIC_MIRROR_REV || styleA == BASIC_PASS_THRU_REV)
{
r1 = 1. / r1;
g1 = 1. / g1;
Expand All @@ -727,7 +731,7 @@ GammaOpDataRcPtr GammaOpData::compose(const GammaOpData & B) const
double g2 = B.getGreenParams()[0];
double b2 = B.getBlueParams()[0];
double a2 = B.getAlphaParams()[0];
if (styleB == BASIC_REV || styleB == BASIC_MIRROR_REV || styleB == BASIC_PASS_THRU_FWD)
if (styleB == BASIC_REV || styleB == BASIC_MIRROR_REV || styleB == BASIC_PASS_THRU_REV)
{
r2 = 1. / r2;
g2 = 1. / g2;
Expand All @@ -747,6 +751,7 @@ GammaOpDataRcPtr GammaOpData::compose(const GammaOpData & B) const
RoundAround1(bOut);
RoundAround1(aOut);

// NB: This always returns a forward style.
Style style = CombineBasicStyles(styleA, styleB);

// By convention, we try to keep the gamma parameter > 1.
Expand Down
65 changes: 65 additions & 0 deletions tests/cpu/OpOptimizers_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ void CompareRender(OCIO::OpRcPtrVec & ops1, OCIO::OpRcPtrVec & ops2,

for (const auto & op : ops1)
{
// NB: This hard-codes OPTIMIZATION_FAST_LOG_EXP_POW to off, see Op.h.
op->apply(&img1[0], &img1[0], nbPixels);
}

Expand Down Expand Up @@ -1070,6 +1071,70 @@ OCIO_ADD_TEST(OpOptimizers, gamma_comp)
CompareRender(ops, optOps, __LINE__, 1e-4f, true);
}

OCIO_ADD_TEST(OpOptimizers, gamma_comp_test2)
{
// This transform has a pair of gammas separated by a pair of matrices that
// compose into an identity matrix and get optimized out. Then the gammas
// get composed into a non-identity gamma. Finally the exponent is inverted
// (to follow the convention of keeping it > 1) and the direction is inverted.

const std::string fileName("gamma_comp_test2.ctf");
OCIO::OpRcPtrVec ops;
OCIO::ContextRcPtr context = OCIO::Context::Create();
OCIO_CHECK_NO_THROW(OCIO::BuildOpsTest(ops, fileName, context,
OCIO::TRANSFORM_DIR_FORWARD));

// First one is the file no op.
OCIO_CHECK_EQUAL(ops.size(), 5);

// Remove no ops & finalize for computation.
OCIO_CHECK_NO_THROW(ops.finalize());
OCIO_CHECK_NO_THROW(ops.optimize(OCIO::OPTIMIZATION_NONE));

OCIO_CHECK_EQUAL(ops.size(), 4);

OCIO::OpRcPtrVec optOps = ops.clone();
OCIO::OpRcPtrVec optOps_noComp = ops.clone();

OCIO_CHECK_EQUAL(optOps_noComp.size(), 4);
OCIO_CHECK_NO_THROW(optOps_noComp.finalize());
// NB: The op->apply function used here hard-codes OPTIMIZATION_FAST_LOG_EXP_POW to off.
OCIO_CHECK_NO_THROW(optOps_noComp.optimize(AllBut(OCIO::OPTIMIZATION_COMP_GAMMA)));
OCIO_CHECK_EQUAL(optOps_noComp.size(), 2);
OCIO_CHECK_EQUAL(optOps_noComp[0]->getInfo(), "<GammaOp>");
OCIO_CHECK_EQUAL(optOps_noComp[1]->getInfo(), "<GammaOp>");

// Due to rounding error in the two 3x3 matrix multiplies with much larger values, the
// 1.52e-4 input value is off by 60% going into the second gamma (see ociochecklut -s).
// Therefore the optOps_noComp and optOps are actually more accurate than ops here.
CompareRender(ops, optOps_noComp, __LINE__, 1e-4f);

OCIO_CHECK_NO_THROW(optOps.finalize());
OCIO_CHECK_NO_THROW(optOps.optimize(OCIO::OPTIMIZATION_DEFAULT));

// Now check that the optimized transform renders the same as the original.
CompareRender(ops, optOps, __LINE__, 1e-4f);

// Check the op is as expected.
OCIO::GroupTransformRcPtr group = OCIO::GroupTransform::Create();
OCIO_REQUIRE_EQUAL(optOps.size(), 1);
OCIO::ConstOpRcPtr op(optOps[0]);
OCIO::CreateGammaTransform(group, op);
OCIO_REQUIRE_EQUAL(group->getNumTransforms(), 1);
auto transform = group->getTransform(0);
OCIO_REQUIRE_ASSERT(transform);
auto gTransform = OCIO_DYNAMIC_POINTER_CAST<OCIO::ExponentTransform>(transform);
OCIO_REQUIRE_ASSERT(gTransform);
OCIO_CHECK_EQUAL(gTransform->getNegativeStyle(), OCIO::NEGATIVE_PASS_THRU);
OCIO_CHECK_EQUAL(gTransform->getDirection(), OCIO::TRANSFORM_DIR_INVERSE);
double vals[4];
gTransform->getValue(vals);
OCIO_CHECK_CLOSE(vals[0], 2.2/1.8, 1e-6f);
OCIO_CHECK_CLOSE(vals[1], 2.2/1.8, 1e-6f);
OCIO_CHECK_CLOSE(vals[2], 2.2/1.8, 1e-6f);
OCIO_CHECK_EQUAL(vals[3], 1.);
}

OCIO_ADD_TEST(OpOptimizers, gamma_comp_identity)
{
OCIO::OpRcPtrVec ops;
Expand Down
95 changes: 85 additions & 10 deletions tests/cpu/ops/gamma/GammaOpData_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,8 @@ void CheckGammaCompose(OCIO::GammaOpData::Style style1,
OCIO::GammaOpData::Style style2,
const OCIO::GammaOpData::Params & params2,
OCIO::GammaOpData::Style refStyle,
const OCIO::GammaOpData::Params & refParams)
const OCIO::GammaOpData::Params & refParams,
unsigned line)
{
static const OCIO::GammaOpData::Params paramsA = { 1. };

Expand All @@ -591,12 +592,12 @@ void CheckGammaCompose(OCIO::GammaOpData::Style style1,

const OCIO::GammaOpDataRcPtr g3 = g1.compose(g2);

OCIO_CHECK_EQUAL(g3->getStyle(), refStyle);
OCIO_CHECK_EQUAL_FROM(g3->getStyle(), refStyle, line);

OCIO_CHECK_ASSERT(g3->getRedParams() == refParams);
OCIO_CHECK_ASSERT(g3->getGreenParams() == refParams);
OCIO_CHECK_ASSERT(g3->getBlueParams() == refParams);
OCIO_CHECK_ASSERT(g3->getAlphaParams() == paramsA);
OCIO_CHECK_ASSERT_FROM(g3->getRedParams() == refParams, line);
OCIO_CHECK_ASSERT_FROM(g3->getGreenParams() == refParams, line);
OCIO_CHECK_ASSERT_FROM(g3->getBlueParams() == refParams, line);
OCIO_CHECK_ASSERT_FROM(g3->getAlphaParams() == paramsA, line);
}

};
Expand All @@ -610,7 +611,8 @@ OCIO_ADD_TEST(GammaOpData, compose)

CheckGammaCompose(OCIO::GammaOpData::BASIC_FWD, params1,
OCIO::GammaOpData::BASIC_FWD, params2,
OCIO::GammaOpData::BASIC_FWD, refParams);
OCIO::GammaOpData::BASIC_FWD, refParams,
__LINE__);
}

{
Expand All @@ -620,7 +622,8 @@ OCIO_ADD_TEST(GammaOpData, compose)

CheckGammaCompose(OCIO::GammaOpData::BASIC_REV, params1,
OCIO::GammaOpData::BASIC_REV, params2,
OCIO::GammaOpData::BASIC_REV, refParams);
OCIO::GammaOpData::BASIC_REV, refParams,
__LINE__);
}

{
Expand All @@ -630,7 +633,8 @@ OCIO_ADD_TEST(GammaOpData, compose)

CheckGammaCompose(OCIO::GammaOpData::BASIC_REV, params1,
OCIO::GammaOpData::BASIC_FWD, params2,
OCIO::GammaOpData::BASIC_REV, refParams);
OCIO::GammaOpData::BASIC_REV, refParams,
__LINE__);
}

{
Expand All @@ -640,7 +644,78 @@ OCIO_ADD_TEST(GammaOpData, compose)

CheckGammaCompose(OCIO::GammaOpData::BASIC_REV, params1,
OCIO::GammaOpData::BASIC_FWD, params2,
OCIO::GammaOpData::BASIC_FWD, refParams);
OCIO::GammaOpData::BASIC_FWD, refParams,
__LINE__);
}

{
const OCIO::GammaOpData::Params params1 = { 2. };
const OCIO::GammaOpData::Params params2 = { 4. };
const OCIO::GammaOpData::Params refParams = { 2. };

CheckGammaCompose(OCIO::GammaOpData::BASIC_FWD, params1,
OCIO::GammaOpData::BASIC_REV, params2,
OCIO::GammaOpData::BASIC_REV, refParams,
__LINE__);
}

{
const OCIO::GammaOpData::Params params1 = { 2. };
const OCIO::GammaOpData::Params params2 = { 4. };
const OCIO::GammaOpData::Params refParams = { 2. };

CheckGammaCompose(OCIO::GammaOpData::BASIC_PASS_THRU_FWD, params1,
OCIO::GammaOpData::BASIC_PASS_THRU_REV, params2,
OCIO::GammaOpData::BASIC_PASS_THRU_REV, refParams,
__LINE__);
}

{
const OCIO::GammaOpData::Params params1 = { 2. };
const OCIO::GammaOpData::Params params2 = { 4. };
const OCIO::GammaOpData::Params refParams = { 2. };

CheckGammaCompose(OCIO::GammaOpData::BASIC_MIRROR_FWD, params1,
OCIO::GammaOpData::BASIC_MIRROR_REV, params2,
OCIO::GammaOpData::BASIC_MIRROR_REV, refParams,
__LINE__);
}

{
const OCIO::GammaOpData::Params params1 = { 2. };
const OCIO::GammaOpData::Params params2 = { 4. };
const OCIO::GammaOpData::Params refParams = { 2. };

CheckGammaCompose(OCIO::GammaOpData::BASIC_MIRROR_FWD, params1,
OCIO::GammaOpData::BASIC_REV, params2,
OCIO::GammaOpData::BASIC_REV, refParams,
__LINE__);
}

{
const OCIO::GammaOpData::Params params1 = { 2. };
const OCIO::GammaOpData::Params params2 = { 4. };
const OCIO::GammaOpData::Params refParams = { 2. };

CheckGammaCompose(OCIO::GammaOpData::BASIC_PASS_THRU_FWD, params1,
OCIO::GammaOpData::BASIC_REV, params2,
OCIO::GammaOpData::BASIC_REV, refParams,
__LINE__);
}

{
const OCIO::GammaOpData::Params params1 = { 4. };
OCIO::GammaOpData::Params paramsA = { 1. };
OCIO::GammaOpData g1(OCIO::GammaOpData::BASIC_MIRROR_FWD,
params1, params1, params1, paramsA);

OCIO::GammaOpData::Params params2 = { 2. };
OCIO::GammaOpData g2(OCIO::GammaOpData::BASIC_PASS_THRU_FWD,
params2, params2, params2, paramsA);

OCIO_CHECK_THROW_WHAT(g1.compose(g2),
OCIO::Exception,
"GammaOp can only be combined with some GammaOps");
}

{
Expand Down
32 changes: 32 additions & 0 deletions tests/data/files/gamma_comp_test2.ctf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>
<ProcessList id="none" version="2.0">

<Description>Transform should optimize into a basicPassThruRev with gamma=2.2/1.8</Description>

<Gamma inBitDepth="16f" outBitDepth="16f" style="basicPassThruFwd">
<GammaParams gamma="1.8" />
</Gamma>

<Matrix inBitDepth="16f" outBitDepth="16f">
<Description>Rec.709 to XYZ</Description>
<Array dim="3 3 3">
0.412390799266 0.357584339384 0.180480788402
0.212639005872 0.715168678768 0.072192315361
0.019330818716 0.119194779795 0.95053215225
</Array>
</Matrix>

<Matrix inBitDepth="16f" outBitDepth="16f">
<Description>XYZ to Rec.709</Description>
<Array dim="3 3 3">
3.240969941905 -1.53738317757 -0.498610760293
-0.969243636281 1.875967501508 0.041555057407
0.055630079697 -0.203976958889 1.056971514243
</Array>
</Matrix>

<Gamma inBitDepth="16f" outBitDepth="16f" style="basicPassThruRev">
<GammaParams gamma="2.2" />
</Gamma>

</ProcessList>