Skip to content

Commit a712243

Browse files
BernardLefebvredoug-walker
BernardLefebvre
and
doug-walker
authored
Adsk Contrib - Data bypass fix (#1229)
* Data bypass fix Signed-off-by: Bernard Lefebvre <[email protected]> * beta 2 Signed-off-by: Bernard Lefebvre <[email protected]> * Adjust some comments Signed-off-by: Bernard Lefebvre <[email protected]> Co-authored-by: doug-walker <[email protected]>
1 parent e412d8a commit a712243

File tree

8 files changed

+56
-15
lines changed

8 files changed

+56
-15
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 "beta1")
37+
set(OpenColorIO_VERSION_RELEASE_TYPE "beta2")
3838

3939
enable_testing()
4040

‎src/OpenColorIO/Config.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3622,17 +3622,21 @@ void Config::setFileRules(ConstFileRulesRcPtr fileRules)
36223622

36233623
const char * Config::getColorSpaceFromFilepath(const char * filePath) const
36243624
{
3625-
return getImpl()->m_fileRules->getImpl()->getColorSpaceFromFilepath(*this, filePath);
3625+
return getImpl()->m_fileRules->getImpl()->getColorSpaceFromFilepath(*this,
3626+
filePath ? filePath : "");
36263627
}
36273628

36283629
const char * Config::getColorSpaceFromFilepath(const char * filePath, size_t & ruleIndex) const
36293630
{
3630-
return getImpl()->m_fileRules->getImpl()->getColorSpaceFromFilepath(*this, filePath, ruleIndex);
3631+
return getImpl()->m_fileRules->getImpl()->getColorSpaceFromFilepath(*this,
3632+
filePath ? filePath : "",
3633+
ruleIndex);
36313634
}
36323635

36333636
bool Config::filepathOnlyMatchesDefaultRule(const char * filePath) const
36343637
{
3635-
return getImpl()->m_fileRules->getImpl()->filepathOnlyMatchesDefaultRule(*this, filePath);
3638+
return getImpl()->m_fileRules->getImpl()->filepathOnlyMatchesDefaultRule(*this,
3639+
filePath ? filePath : "");
36363640
}
36373641

36383642

‎src/OpenColorIO/Processor.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,9 @@ void Processor::Impl::setColorSpaceConversion(const Config & config,
583583
throw Exception("Internal error: Processor should be empty");
584584
}
585585

586-
BuildColorSpaceOps(m_ops, config, context, srcColorSpace, dstColorSpace, false);
586+
// Default behavior is to bypass data color space. ColorSpaceTransform can be used to not bypass
587+
// data color spaces.
588+
BuildColorSpaceOps(m_ops, config, context, srcColorSpace, dstColorSpace, true);
587589

588590
std::ostringstream desc;
589591
desc << "Color space conversion from " << srcColorSpace->getName()

‎src/OpenColorIO/transforms/LookTransform.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,9 @@ void RunLookTokens(OpRcPtrVec & ops,
289289
if (!skipColorSpaceConversion &&
290290
currentColorSpace.get() != processColorSpace.get())
291291
{
292+
// Default behavior is to bypass data color space.
292293
BuildColorSpaceOps(ops, config, context,
293-
currentColorSpace, processColorSpace, false);
294+
currentColorSpace, processColorSpace, true);
294295
currentColorSpace = processColorSpace;
295296
}
296297

@@ -352,8 +353,8 @@ void BuildLookOps(OpRcPtrVec & ops,
352353
// If current color space is already the dst space skip the conversion.
353354
if (!skipColorSpaceConversion && currentColorSpace.get() != dst.get())
354355
{
355-
BuildColorSpaceOps(ops, config, context,
356-
currentColorSpace, dst, false);
356+
// Default behavior is to bypass data color space.
357+
BuildColorSpaceOps(ops, config, context, currentColorSpace, dst, true);
357358
}
358359
}
359360

‎src/apps/ociochecklut/main.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,8 @@ int main (int argc, const char* argv[])
218218
{
219219
// What are the allowed formats?
220220
std::cout << "Formats supported:" << std::endl;
221-
const auto nbFromats = OCIO::FileTransform::GetNumFormats();
222-
for (int i = 0; i < nbFromats; ++i)
221+
const auto nbFormats = OCIO::FileTransform::GetNumFormats();
222+
for (int i = 0; i < nbFormats; ++i)
223223
{
224224
std::cout << OCIO::FileTransform::GetFormatNameByIndex(i);
225225
std::cout << " (." << OCIO::FileTransform::GetFormatExtensionByIndex(i) << ")";

‎tests/cpu/FileRules_tests.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,10 @@ OCIO_ADD_TEST(FileRules, rules_filepattern)
489489
OCIO_CHECK_EQUAL(rulePosition, 1); // Default rule.
490490
config->getColorSpaceFromFilepath("/An/Arbitrary.exr/Path/MyFileexr", rulePosition);
491491
OCIO_CHECK_EQUAL(rulePosition, 1); // Default rule.
492+
config->getColorSpaceFromFilepath("", rulePosition);
493+
OCIO_CHECK_EQUAL(rulePosition, 1); // Default rule.
494+
config->getColorSpaceFromFilepath(nullptr, rulePosition);
495+
OCIO_CHECK_EQUAL(rulePosition, 1); // Default rule.
492496

493497
OCIO_CHECK_NO_THROW(rules->setPattern(0, "gamma"));
494498
config->setFileRules(rules);
@@ -986,7 +990,11 @@ strictparsing: true
986990
auto rules = config->getFileRules();
987991
OCIO_REQUIRE_ASSERT(rules);
988992
OCIO_REQUIRE_EQUAL(rules->getNumEntries(), 1);
989-
// File defined default role is preserved.
993+
OCIO_CHECK_EQUAL(std::string(rules->getName(0)), OCIO::FileRuleUtils::DefaultName);
994+
OCIO_CHECK_EQUAL(std::string(rules->getColorSpace(0)), "cs1");
995+
OCIO_CHECK_EQUAL(std::string(config->getColorSpaceFromFilepath("anything")), "cs1");
996+
997+
// The color space of the default role is preserved.
990998
auto cs = config->getColorSpace(OCIO::ROLE_DEFAULT);
991999
OCIO_CHECK_EQUAL(std::string("raw"), cs->getName());
9921000
}

‎tests/cpu/transforms/ColorSpaceTransform_tests.cpp‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,27 @@ OCIO_ADD_TEST(ColorSpaceTransform, build_colorspace_ops)
137137
*cst, OCIO::TRANSFORM_DIR_FORWARD));
138138
OCIO_CHECK_NO_THROW(ops.validate());
139139
OCIO_CHECK_EQUAL(ops.size(), 0);
140+
ops.clear();
141+
142+
OCIO::ConstProcessorRcPtr proc;
143+
config->setProcessorCacheFlags(OCIO::PROCESSOR_CACHE_OFF); // Cache disabled
144+
OCIO_CHECK_NO_THROW(proc = config->getProcessor(cst));
145+
OCIO_CHECK_EQUAL(proc->getNumTransforms(), 0);
140146

141147
cst->setDataBypass(false);
142148
OCIO_CHECK_NO_THROW(OCIO::BuildColorSpaceOps(ops, *config, config->getCurrentContext(),
143149
*cst, OCIO::TRANSFORM_DIR_FORWARD));
144150
OCIO_CHECK_NO_THROW(ops.validate());
145151
OCIO_CHECK_EQUAL(ops.size(), 4);
152+
ops.clear();
153+
154+
// Some of the ops are no-ops, processor has 2 transforms.
155+
OCIO_CHECK_NO_THROW(proc = config->getProcessor(cst));
156+
OCIO_CHECK_EQUAL(proc->getNumTransforms(), 2);
157+
158+
// Similar test with color space, data by-pass can't be controlled.
159+
OCIO_CHECK_NO_THROW(proc = config->getProcessor(src.c_str(), dst.c_str()));
160+
OCIO_CHECK_EQUAL(proc->getNumTransforms(), 0);
146161

147162
// Restore default data flags.
148163
csSceneToRef->setIsData(false);
@@ -153,17 +168,28 @@ OCIO_ADD_TEST(ColorSpaceTransform, build_colorspace_ops)
153168
csSceneFromRef->setIsData(true);
154169
config->addColorSpace(csSceneFromRef);
155170

156-
ops.clear();
157171
OCIO_CHECK_NO_THROW(OCIO::BuildColorSpaceOps(ops, *config, config->getCurrentContext(),
158172
*cst, OCIO::TRANSFORM_DIR_FORWARD));
159173
OCIO_CHECK_NO_THROW(ops.validate());
160174
OCIO_CHECK_EQUAL(ops.size(), 0);
175+
ops.clear();
176+
177+
OCIO_CHECK_NO_THROW(proc = config->getProcessor(cst));
178+
OCIO_CHECK_EQUAL(proc->getNumTransforms(), 0);
161179

162180
cst->setDataBypass(false);
163181
OCIO_CHECK_NO_THROW(OCIO::BuildColorSpaceOps(ops, *config, config->getCurrentContext(),
164182
*cst, OCIO::TRANSFORM_DIR_FORWARD));
165183
OCIO_CHECK_NO_THROW(ops.validate());
166184
OCIO_CHECK_EQUAL(ops.size(), 4);
185+
ops.clear();
186+
187+
OCIO_CHECK_NO_THROW(proc = config->getProcessor(cst));
188+
OCIO_CHECK_EQUAL(proc->getNumTransforms(), 2);
189+
190+
// Similar test with color space, data bypass can't be controlled.
191+
OCIO_CHECK_NO_THROW(proc = config->getProcessor(src.c_str(), dst.c_str()));
192+
OCIO_CHECK_EQUAL(proc->getNumTransforms(), 0);
167193

168194
// Restore default data flags.
169195
csSceneFromRef->setIsData(false);

‎tests/cpu/transforms/LookTransform_tests.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ search_path: luts
624624

625625
OCIO::OpRcPtrVec ops;
626626
OCIO_CHECK_NO_THROW(BuildColorSpaceOps(ops, *config, config->getCurrentContext(),
627-
srcColorSpace, dstColorSpace, false));
627+
srcColorSpace, dstColorSpace, true));
628628
OCIO_CHECK_NO_THROW(ops.validate());
629629
OCIO_REQUIRE_EQUAL(ops.size(), 11);
630630
OCIO::ConstOpRcPtr op = ops[0];
@@ -665,7 +665,7 @@ search_path: luts
665665
// Test in inverse direction.
666666
ops.clear();
667667
OCIO_CHECK_NO_THROW(BuildColorSpaceOps(ops, *config, config->getCurrentContext(),
668-
dstColorSpace, srcColorSpace, false));
668+
dstColorSpace, srcColorSpace, true));
669669
OCIO_REQUIRE_EQUAL(ops.size(), 11);
670670
OCIO_CHECK_NO_THROW(ops.validate());
671671
op = ops[0];
@@ -711,7 +711,7 @@ search_path: luts
711711

712712
OCIO::OpRcPtrVec ops2;
713713
OCIO_CHECK_NO_THROW(BuildColorSpaceOps(ops2, *config, config->getCurrentContext(),
714-
dstColorSpaceInv, srcColorSpace, false));
714+
dstColorSpaceInv, srcColorSpace, true));
715715
OCIO_REQUIRE_EQUAL(ops2.size(), ops.size());
716716
OCIO_CHECK_NO_THROW(ops2.validate());
717717
for (std::size_t i = 0; i < ops2.size(); ++i)

0 commit comments

Comments
 (0)