Skip to content

Commit d2b672f

Browse files
cedrik-fuoco-adskdoug-walkerbartstyczen
authored
Integration - Port fixes from main to RB-2.0 - Adsk Contrib (#1752)
* Changing version release type for 2.0.5 official release. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Fix atan2 argument order for HLSL (#1712) Signed-off-by: Doug Walker <doug.walker@autodesk.com> Signed-off-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit 8a93946) Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * replace texture2D function with texture for GLSL 1.3 (#1723) Signed-off-by: Bart Styczen <bart.styczen@cine.dev> Signed-off-by: Bart Styczen <bart.styczen@cine.dev> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit d5cedbf) Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Adsk contrib - Processor cache does not detect changes in cccid (#1726) * For Looks that has a FileTransform and for Colorspace with FileTransfrom, add the CCCID to the processor's cache key for that transform. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Removing the workaround in the related unit tests and fixing the issue by adding the environment variable to the context using setStringVar. The processor's cache key is using the context cache ID which has all the context variables taken into account. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Now using addStringVars and creating a new context instead of reusing the one used for the filename. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Adding cccid to the context when there are no context variable. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Adding a few unit tests to test that the processor is different when changing the FileTransform's CCCID. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Using setStringVar to set CCNUM context variable in unit test. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Adding a test in FileTransform to test CollectContextVariables directly. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Minor tweaks for the unit test Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit 4fa7750) Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Fix inverse Lut1D optimization bug (#1743) Signed-off-by: Doug Walker <doug.walker@autodesk.com> Signed-off-by: Doug Walker <doug.walker@autodesk.com> (cherry picked from commit 5152635) Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> Signed-off-by: Doug Walker <doug.walker@autodesk.com> Signed-off-by: Bart Styczen <bart.styczen@cine.dev> Co-authored-by: Doug Walker <doug.walker@autodesk.com> Co-authored-by: bartstyczen <93516126+bartstyczen@users.noreply.github.com>
1 parent 7de13fc commit d2b672f

File tree

9 files changed

+299
-5
lines changed

9 files changed

+299
-5
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ project(OpenColorIO
3434
LANGUAGES CXX C)
3535

3636
# "dev", "beta1", rc1", etc or "" for an official release.
37-
set(OpenColorIO_VERSION_RELEASE_TYPE "dev")
37+
set(OpenColorIO_VERSION_RELEASE_TYPE "")
3838

3939
enable_testing()
4040

‎src/OpenColorIO/GpuShaderUtils.cpp‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,15 @@ std::string getTexSample(GpuLanguage lang,
118118
switch (lang)
119119
{
120120
case GPU_LANGUAGE_GLSL_1_2:
121-
case GPU_LANGUAGE_GLSL_1_3:
122121
{
123122
kw << "texture" << N << "D(" << samplerName << ", " << coords << ")";
124123
break;
125124
}
125+
case GPU_LANGUAGE_GLSL_1_3:
126+
{
127+
kw << "texture(" << samplerName << ", " << coords << ")";
128+
break;
129+
}
126130
case GPU_LANGUAGE_CG:
127131
{
128132
kw << "tex" << N << "D(" << samplerName << ", " << coords << ")";
@@ -868,8 +872,10 @@ std::string GpuShaderText::atan2(const std::string & y,
868872
}
869873
case GPU_LANGUAGE_HLSL_DX11:
870874
{
871-
// note: operand order is swapped in HLSL
872-
kw << "atan2(" << x << ", " << y << ")";
875+
// note: Various internet sources claim that the x & y arguments need to be
876+
// swapped for HLSL (relative to GLSL). However, recent testing on Windows
877+
// has revealed that the argument order needs to be the same as GLSL.
878+
kw << "atan2(" << y << ", " << x << ")";
873879
break;
874880
}
875881

‎src/OpenColorIO/OpOptimizers.cpp‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "Op.h"
1313
#include "ops/lut1d/Lut1DOp.h"
1414
#include "ops/lut3d/Lut3DOp.h"
15+
#include "ops/range/RangeOp.h"
1516

1617
namespace OCIO_NAMESPACE
1718
{
@@ -241,7 +242,38 @@ int RemoveInverseOps(OpRcPtrVec & opVec, OptimizationFlags oFlags)
241242
// When a pair of inverse ops is removed, we want the optimized ops to give the
242243
// same result as the original. For certain ops such as Lut1D or Log this may
243244
// mean inserting a Range to emulate the clamping done by the original ops.
244-
auto replacedBy = op1->getIdentityReplacement();
245+
246+
OpRcPtr replacedBy;
247+
if (type1 == OpData::Lut1DType)
248+
{
249+
// Lut1D gets special handling so that both halfs of the pair are available.
250+
// Only the inverse LUT has the values needed to generate the replacement.
251+
252+
ConstLut1DOpDataRcPtr lut1 = OCIO_DYNAMIC_POINTER_CAST<const Lut1DOpData>(op1->data());
253+
ConstLut1DOpDataRcPtr lut2 = OCIO_DYNAMIC_POINTER_CAST<const Lut1DOpData>(op2->data());
254+
255+
OpDataRcPtr opData = lut1->getPairIdentityReplacement(lut2);
256+
257+
OpRcPtrVec ops;
258+
if (opData->getType() == OpData::MatrixType)
259+
{
260+
// No-op that will be optimized.
261+
auto mat = OCIO_DYNAMIC_POINTER_CAST<MatrixOpData>(opData);
262+
CreateMatrixOp(ops, mat, TRANSFORM_DIR_FORWARD);
263+
}
264+
else if (opData->getType() == OpData::RangeType)
265+
{
266+
// Clamping op.
267+
auto range = OCIO_DYNAMIC_POINTER_CAST<RangeOpData>(opData);
268+
CreateRangeOp(ops, range, TRANSFORM_DIR_FORWARD);
269+
}
270+
replacedBy = ops[0];
271+
}
272+
else
273+
{
274+
replacedBy = op1->getIdentityReplacement();
275+
}
276+
245277
replacedBy->finalize();
246278
if (replacedBy->isNoOp())
247279
{

‎src/OpenColorIO/ops/lut1d/Lut1DOpData.cpp‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,86 @@ OpDataRcPtr Lut1DOpData::getIdentityReplacement() const
280280
return res;
281281
}
282282

283+
OpDataRcPtr Lut1DOpData::getPairIdentityReplacement(ConstLut1DOpDataRcPtr & lut2) const
284+
{
285+
OpDataRcPtr res;
286+
if (isInputHalfDomain())
287+
{
288+
// TODO: If a half-domain LUT has a flat spot, it would be more appropriate
289+
// to use a Range, since some areas would be clamped in a round-trip.
290+
// Currently leaving this a Matrix since it is a potential work-around
291+
// for situations where you want a pair identity of LUTs to be totally
292+
// removed, even if it omits some clamping at extreme values.
293+
res = std::make_shared<MatrixOpData>();
294+
}
295+
else
296+
{
297+
// Note that the ops have been finalized by the time this is called,
298+
// Therefore, for an inverse Lut1D, it means initializeFromForward() has
299+
// been called and so any reversals have been converted to flat regions.
300+
// Therefore, the first and last LUT entries are the extreme values and
301+
// the ComponentProperties has been initialized, but only for the op
302+
// whose direction is INVERSE.
303+
const Lut1DOpData * invLut = (m_direction == TRANSFORM_DIR_INVERSE)
304+
? this: lut2.get();
305+
const ComponentProperties & redProperties = invLut->getRedProperties();
306+
const unsigned long length = invLut->getArray().getLength();
307+
308+
// If the start or end of the LUT contains a flat region, that will cause
309+
// a round-trip to be limited.
310+
311+
double minValue = 0.;
312+
double maxValue = 1.;
313+
switch (m_direction)
314+
{
315+
case TRANSFORM_DIR_FORWARD: // Fwd Lut1D -> Inv Lut1D
316+
{
317+
// A round-trip in this order will impose at least a clamp to [0,1]
318+
// based on what happens entering the first Fwd Lut1D. However, the
319+
// clamping may be to an even narrower range if there are flat regions.
320+
//
321+
// The flat region limitation is imposed based on the where it falls
322+
// relative to the [0,1] input domain.
323+
324+
// TODO: A RangeOp has one min & max for all channels, whereas a Lut1D may
325+
// have three independent channels. Potentially could look at all chans
326+
// and take the extrema of each? For now, just using the first channel.
327+
const unsigned long minIndex = redProperties.startDomain;
328+
const unsigned long maxIndex = redProperties.endDomain;
329+
330+
minValue = (double)minIndex / (length - 1);
331+
maxValue = (double)maxIndex / (length - 1);
332+
break;
333+
}
334+
case TRANSFORM_DIR_INVERSE: // Inv Lut1D -> Fwd Lut1D
335+
{
336+
// A round-trip in this order will impose a clamp, but it may be to
337+
// bounds outside of [0,1] since the Fwd LUT may contain values outside
338+
// [0,1] and so the Inv LUT will accept inputs on that extended range.
339+
//
340+
// The flat region limitation is imposed based on the output range.
341+
342+
const bool isIncreasing = redProperties.isIncreasing;
343+
const unsigned long maxChannels = invLut->getArray().getMaxColorComponents();
344+
const unsigned long lastValIndex = (length - 1) * maxChannels;
345+
// Note that the array for the invLut has had initializeFromForward()
346+
// done and so any reversals have been converted to flat regions and
347+
// the extrema are at the beginning & end of the LUT.
348+
const Array::Values & lutValues = invLut->getArray().getValues();
349+
350+
// TODO: Currently only basing this on the red channel.
351+
minValue = isIncreasing ? lutValues[0] : lutValues[lastValIndex];
352+
maxValue = isIncreasing ? lutValues[lastValIndex] : lutValues[0];
353+
break;
354+
}
355+
}
356+
357+
res = std::make_shared<RangeOpData>(minValue, maxValue,
358+
minValue, maxValue);
359+
}
360+
return res;
361+
}
362+
283363
void Lut1DOpData::setInputHalfDomain(bool isHalfDomain) noexcept
284364
{
285365
m_halfFlags = (isHalfDomain) ?

‎src/OpenColorIO/ops/lut1d/Lut1DOpData.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ class Lut1DOpData : public OpData
179179

180180
OpDataRcPtr getIdentityReplacement() const override;
181181

182+
OpDataRcPtr getPairIdentityReplacement(ConstLut1DOpDataRcPtr & lut2) const;
183+
182184
inline const ComponentProperties & getRedProperties() const
183185
{
184186
return m_componentProperties[0];

‎src/OpenColorIO/transforms/FileTransform.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,16 @@ bool CollectContextVariables(const Config &,
240240
usedContextVars->addStringVars(ctxFilepath);
241241
}
242242

243+
// Check if the CCCID is using a context variable and add it to the context if that's the case.
244+
ContextRcPtr ctxCCCID = Context::Create();
245+
const char * cccid = tr.getCCCId();
246+
std::string resolvedCCCID = context.resolveStringVar(cccid, ctxCCCID);
247+
if (0 != strcmp(resolvedCCCID.c_str(), cccid))
248+
{
249+
foundContextVars = true;
250+
usedContextVars->addStringVars(ctxCCCID);
251+
}
252+
243253
return foundContextVars;
244254
}
245255

‎tests/cpu/Config_tests.cpp‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7528,6 +7528,61 @@ OCIO_ADD_TEST(Config, context_variables_typical_use_cases)
75287528
cfg->getProcessor(ctx2, "cs1", "cs2").get());
75297529
}
75307530
}
7531+
7532+
7533+
// Case 7 - Context variables in the FileTransform's CCCID.
7534+
{
7535+
static const std::string CONFIG =
7536+
"ocio_profile_version: 2\n"
7537+
"\n"
7538+
"environment:\n"
7539+
" CCPREFIX: cc\n"
7540+
"\n"
7541+
"search_path: " + OCIO::GetTestFilesDir() + "\n"
7542+
"\n"
7543+
"roles:\n"
7544+
" default: cs1\n"
7545+
"\n"
7546+
"displays:\n"
7547+
" disp1:\n"
7548+
" - !<View> {name: view1, colorspace: cs2}\n"
7549+
"\n"
7550+
"colorspaces:\n"
7551+
" - !<ColorSpace>\n"
7552+
" name: cs1\n"
7553+
"\n"
7554+
" - !<ColorSpace>\n"
7555+
" name: cs2\n"
7556+
" from_scene_reference: !<FileTransform> {src: cdl_test1.ccc, cccid: $CCPREFIX00$CCNUM}\n";
7557+
7558+
std::istringstream iss;
7559+
iss.str(CONFIG);
7560+
7561+
OCIO::ConfigRcPtr cfg;
7562+
OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss)->createEditableCopy());
7563+
OCIO_CHECK_NO_THROW(cfg->validate());
7564+
7565+
OCIO::ConstTransformRcPtr ctf = cfg->getColorSpace("cs2")->getTransform(
7566+
OCIO::COLORSPACE_DIR_FROM_REFERENCE
7567+
);
7568+
OCIO_REQUIRE_ASSERT(ctf);
7569+
7570+
OCIO::ContextRcPtr ctx = cfg->getCurrentContext()->createEditableCopy();
7571+
7572+
ctx->setStringVar("CCNUM", "01");
7573+
OCIO::ConstProcessorRcPtr p1 = cfg->getProcessor(ctx, ctf, OCIO::TRANSFORM_DIR_FORWARD);
7574+
7575+
ctx->setStringVar("CCNUM", "02");
7576+
OCIO::ConstProcessorRcPtr p2 = cfg->getProcessor(ctx, ctf, OCIO::TRANSFORM_DIR_FORWARD);
7577+
7578+
ctx->setStringVar("CCNUM", "03");
7579+
OCIO::ConstProcessorRcPtr p3 = cfg->getProcessor(ctx, ctf, OCIO::TRANSFORM_DIR_FORWARD);
7580+
7581+
// All three processors should be different.
7582+
OCIO_CHECK_NE(p1.get(), p2.get());
7583+
OCIO_CHECK_NE(p1.get(), p3.get());
7584+
OCIO_CHECK_NE(p2.get(), p3.get());
7585+
}
75317586
}
75327587

75337588
OCIO_ADD_TEST(Config, virtual_display)

‎tests/cpu/OpOptimizers_tests.cpp‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,63 @@ OCIO_ADD_TEST(OpOptimizers, lut1d_identity_replacement)
645645
}
646646
}
647647

648+
OCIO_ADD_TEST(OpOptimizers, lut1d_identity_replacement_order)
649+
{
650+
// See issue #1737, https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/1737.
651+
652+
// This CTF contains a single LUT1D, inverse direction, normal (not half) domain.
653+
// It contains values from -6 to +3.4.
654+
const std::string fileName("lut1d_inverse_gpu.ctf");
655+
OCIO::ContextRcPtr context = OCIO::Context::Create();
656+
657+
OCIO::OpRcPtrVec inv_ops;
658+
OCIO_CHECK_NO_THROW(OCIO::BuildOpsTest(inv_ops, fileName, context,
659+
// FWD direction simply means don't swap the direction, the
660+
// file contains an inverse LUT1D and leave it that way.
661+
OCIO::TRANSFORM_DIR_FORWARD));
662+
OCIO::OpRcPtrVec fwd_ops;
663+
OCIO_CHECK_NO_THROW(OCIO::BuildOpsTest(fwd_ops, fileName, context,
664+
OCIO::TRANSFORM_DIR_INVERSE));
665+
666+
// Check forward LUT1D followed by inverse LUT1D.
667+
{
668+
OCIO::OpRcPtrVec fwd_inv_ops = fwd_ops;
669+
fwd_inv_ops += inv_ops;
670+
671+
OCIO_CHECK_NO_THROW(fwd_inv_ops.finalize());
672+
OCIO_CHECK_NO_THROW(fwd_inv_ops.optimize(OCIO::OPTIMIZATION_NONE));
673+
OCIO_CHECK_EQUAL(fwd_inv_ops.size(), 2); // no optmization was done
674+
675+
OCIO::OpRcPtrVec optOps = fwd_inv_ops.clone();
676+
OCIO_CHECK_NO_THROW(optOps.finalize());
677+
OCIO_CHECK_NO_THROW(optOps.optimize(OCIO::OPTIMIZATION_DEFAULT));
678+
OCIO_CHECK_EQUAL(optOps.size(), 1);
679+
OCIO_CHECK_EQUAL(optOps[0]->getInfo(), "<RangeOp>");
680+
681+
// Compare renders.
682+
CompareRender(fwd_inv_ops, optOps, __LINE__, 1e-6f);
683+
}
684+
685+
// Check inverse LUT1D followed by forward LUT1D.
686+
{
687+
OCIO::OpRcPtrVec inv_fwd_ops = inv_ops;
688+
inv_fwd_ops += fwd_ops;
689+
690+
OCIO_CHECK_NO_THROW(inv_fwd_ops.finalize());
691+
OCIO_CHECK_NO_THROW(inv_fwd_ops.optimize(OCIO::OPTIMIZATION_NONE));
692+
OCIO_CHECK_EQUAL(inv_fwd_ops.size(), 2); // no optmization was done
693+
694+
OCIO::OpRcPtrVec optOps = inv_fwd_ops.clone();
695+
OCIO_CHECK_NO_THROW(optOps.finalize());
696+
OCIO_CHECK_NO_THROW(optOps.optimize(OCIO::OPTIMIZATION_DEFAULT));
697+
OCIO_CHECK_EQUAL(optOps.size(), 1);
698+
OCIO_CHECK_EQUAL(optOps[0]->getInfo(), "<RangeOp>");
699+
700+
// Compare renders.
701+
CompareRender(inv_fwd_ops, optOps, __LINE__, 1e-6f);
702+
}
703+
}
704+
648705
OCIO_ADD_TEST(OpOptimizers, lut1d_half_domain_keep_prior_range)
649706
{
650707
// A half-domain LUT should not allow removal of a prior range op.

‎tests/cpu/transforms/FileTransform_tests.cpp‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,4 +431,56 @@ OCIO_ADD_TEST(FileTransform, context_variables)
431431

432432
// A basic check to validate that context variables are correctly used.
433433
OCIO_CHECK_NO_THROW(cfg->getProcessor(ctx, file, OCIO::TRANSFORM_DIR_FORWARD));
434+
435+
436+
{
437+
// Case 4 - The 'cccid' now contains a context variable
438+
static const std::string CONFIG =
439+
"ocio_profile_version: 2\n"
440+
"\n"
441+
"environment:\n"
442+
" CCPREFIX: cc\n"
443+
" CCNUM: 02\n"
444+
"\n"
445+
"search_path: " + OCIO::GetTestFilesDir() + "\n"
446+
"\n"
447+
"roles:\n"
448+
" default: cs1\n"
449+
"\n"
450+
"displays:\n"
451+
" disp1:\n"
452+
" - !<View> {name: view1, colorspace: cs2}\n"
453+
"\n"
454+
"colorspaces:\n"
455+
" - !<ColorSpace>\n"
456+
" name: cs1\n"
457+
"\n"
458+
" - !<ColorSpace>\n"
459+
" name: cs2\n"
460+
" from_scene_reference: !<FileTransform> {src: cdl_test1.ccc, cccid: $CCPREFIX00$CCNUM}\n";
461+
462+
std::istringstream iss;
463+
iss.str(CONFIG);
464+
465+
OCIO::ConstConfigRcPtr cfg;
466+
OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss));
467+
OCIO_CHECK_NO_THROW(cfg->validate());
468+
469+
ctx = cfg->getCurrentContext()->createEditableCopy();
470+
OCIO_CHECK_NO_THROW(ctx->setStringVar("CCNUM", "01"));
471+
472+
usedContextVars = OCIO::Context::Create(); // New & empty instance.
473+
OCIO::ConstTransformRcPtr tr1 = cfg->getColorSpace("cs2")->getTransform(
474+
OCIO::COLORSPACE_DIR_FROM_REFERENCE
475+
);
476+
OCIO::ConstFileTransformRcPtr fTr1 = OCIO::DynamicPtrCast<const OCIO::FileTransform>(tr1);
477+
OCIO_CHECK_ASSERT(fTr1);
478+
479+
OCIO_CHECK_ASSERT(CollectContextVariables(*cfg, *ctx, *fTr1, usedContextVars));
480+
OCIO_CHECK_EQUAL(2, usedContextVars->getNumStringVars());
481+
OCIO_CHECK_EQUAL(std::string("CCPREFIX"), usedContextVars->getStringVarNameByIndex(0));
482+
OCIO_CHECK_EQUAL(std::string("cc"), usedContextVars->getStringVarByIndex(0));
483+
OCIO_CHECK_EQUAL(std::string("CCNUM"), usedContextVars->getStringVarNameByIndex(1));
484+
OCIO_CHECK_EQUAL(std::string("01"), usedContextVars->getStringVarByIndex(1));
485+
}
434486
}

0 commit comments

Comments
 (0)