Skip to content

Commit 5d2409b

Browse files
cozdasdoug-walker
andauthored
Remove [0,1] clamping from ICC transforms (#2224)
* Addressing issue #1915, remove [0, 1] clamping from ICC profiles. Currently the clamping is removed ONLY when the the type 0 (simple gamma) TRC is used. In this case the the mirroring around origin is used for the negative values. Other parametric curves are currently implemented as 1DLuts thus will not handle out of bound values properly. In a later wave we can either convert LUTs to half-domain ones or try to implement them with more complex ops (such as exponent with linear). Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> * - Fixing the op tests in the inverse direction - More detailed explanation for the clamp removal in pure-gamma TRC cases. Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> --------- Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com>
1 parent 2bc8759 commit 5d2409b

File tree

2 files changed

+44
-36
lines changed

2 files changed

+44
-36
lines changed

src/OpenColorIO/fileformats/FileFormatICC.cpp

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "ops/gamma/GammaOp.h"
1616
#include "ops/lut1d/Lut1DOp.h"
1717
#include "ops/matrix/MatrixOp.h"
18-
#include "ops/range/RangeOp.h"
1918
#include "Platform.h"
2019
#include "transforms/FileTransform.h"
2120

@@ -794,13 +793,23 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops,
794793
}
795794
}
796795

797-
// The matrix/TRC transform in the ICC profile converts display device code values to the
798-
// CIE XYZ based version of the ICC profile connection space (PCS).
799-
// However, in OCIO the most common use of an ICC monitor profile is as a display color space,
800-
// and in that usage it is more natural for the XYZ to display code value transform to be called
801-
// the forward direction.
796+
// The matrix/TRC transform in the ICC profile converts display device code
797+
// values to the CIE XYZ based version of the ICC profile connection space
798+
// (PCS). However, in OCIO the most common use of an ICC monitor profile is
799+
// as a display color space, and in that usage it is more natural for the
800+
// XYZ to display code value transform to be called the forward direction.
802801

803-
// Curves / ParaCurves operates in the range 0.0 to 1.0 as per ICC specifications.
802+
// The ICC spec states that the TRC tags should clamp to [0,1]. For curves
803+
// that are implemented in the ICC profile as LUTs and most parametric
804+
// curves (which become LUTs in OCIO), this is the case. However, as
805+
// floating-point and HDR workflows become more common, the clamping has
806+
// become a critical roadblock. For example, it is now common to have ICC
807+
// profiles for linear color spaces that need to pass values outside [0,1].
808+
// Therefore, OCIO now implements single entry 'curv' tags and type 0 'para'
809+
// tags without clamping using an ExponentTransform which extends above 1
810+
// and mirrors below 0. (Note that gamma values of 1 do not need to be
811+
// tested for here since they will be omitted as no-ops later by the
812+
// optimizer.)
804813

805814
switch (newDir)
806815
{
@@ -817,18 +826,12 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops,
817826
const GammaOpData::Params greenParams = { cachedFile->mGammaRGB[1] };
818827
const GammaOpData::Params blueParams = { cachedFile->mGammaRGB[2] };
819828
const GammaOpData::Params alphaParams = { cachedFile->mGammaRGB[3] };
820-
auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_FWD,
829+
auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_MIRROR_FWD,
821830
redParams,
822831
greenParams,
823832
blueParams,
824833
alphaParams);
825834

826-
// GammaOp will clamp at 0 so we don't do it in the RangeOp.
827-
CreateRangeOp(ops,
828-
RangeOpData::EmptyValue(), 1,
829-
RangeOpData::EmptyValue(), 1,
830-
TRANSFORM_DIR_FORWARD);
831-
832835
CreateGammaOp(ops, gamma, TRANSFORM_DIR_FORWARD);
833836
}
834837

@@ -859,18 +862,13 @@ LocalFileFormat::buildFileOps(OpRcPtrVec & ops,
859862
const GammaOpData::Params greenParams = { cachedFile->mGammaRGB[1] };
860863
const GammaOpData::Params blueParams = { cachedFile->mGammaRGB[2] };
861864
const GammaOpData::Params alphaParams = { cachedFile->mGammaRGB[3] };
862-
auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_REV,
865+
auto gamma = std::make_shared<GammaOpData>(GammaOpData::BASIC_MIRROR_REV,
863866
redParams,
864867
greenParams,
865868
blueParams,
866869
alphaParams);
867870

868871
CreateGammaOp(ops, gamma, TRANSFORM_DIR_FORWARD);
869-
870-
CreateRangeOp(ops,
871-
RangeOpData::EmptyValue(), 1,
872-
RangeOpData::EmptyValue(), 1,
873-
TRANSFORM_DIR_FORWARD);
874872
}
875873
break;
876874
}

tests/cpu/fileformats/FileFormatICC_tests.cpp

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
244244
{
245245
OCIO::ContextRcPtr context = OCIO::Context::Create();
246246
{
247+
// This test uses a profile where the TRC is a 1024 element LUT.
248+
247249
static const std::string iccFileName("icc-test-3.icm");
248250
OCIO::OpRcPtrVec ops;
249251
OCIO_CHECK_NO_THROW(BuildOpsTest(ops, iccFileName, context, OCIO::TRANSFORM_DIR_INVERSE));
@@ -287,7 +289,8 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
287289
op->apply(srcImage, 3);
288290
}
289291

290-
// Values outside [0.0, 1.0] are clamped and won't round-trip.
292+
// Currently the LUT-based TRC's clamp the values outside
293+
// [0.0, 1.0], thus those values won't round-trip.
291294
static constexpr float bckImage[] = {
292295
0.0f, 0.0f, 0.3f, 0.0f,
293296
0.4f, 0.5f, 0.6f, 0.5f,
@@ -301,34 +304,43 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
301304
}
302305

303306
{
307+
// This test uses a profile where the TRC is
308+
// a parametric curve of type 0 (basic gamma) with
309+
// gamma values {2.174, 2.174, 2.174, 1.0}.
310+
304311
static const std::string iccFileName("icc-test-2.pf");
305312
OCIO::OpRcPtrVec ops;
306313
OCIO_CHECK_NO_THROW(BuildOpsTest(ops, iccFileName, context, OCIO::TRANSFORM_DIR_INVERSE));
307314
OCIO_CHECK_NO_THROW(ops.finalize());
308315
OCIO_CHECK_NO_THROW(ops.optimize(OCIO::OPTIMIZATION_LOSSLESS));
316+
OCIO_REQUIRE_EQUAL(2, ops.size());
317+
OCIO_CHECK_EQUAL("<GammaOp>", ops[0]->getInfo());
318+
OCIO_CHECK_EQUAL("<MatrixOffsetOp>", ops[1]->getInfo());
309319

310320
// apply ops
311-
float srcImage[] = {
321+
const std::array<float, 12> srcImage{
312322
-0.1f, 0.0f, 0.3f, 0.0f,
313323
0.4f, 0.5f, 0.6f, 0.5f,
314324
0.7f, 1.0f, 1.9f, 1.0f };
315325

316-
const float dstImage[] = {
317-
0.012437f, 0.004702f, 0.070333f, 0.0f,
326+
const std::array<float, 12> dstImage{
327+
0.009241f, 0.003003f, 0.070198f, 0.0f,
318328
0.188392f, 0.206965f, 0.343595f, 0.5f,
319-
0.693246f, 0.863199f, 1.07867f , 1.0f };
329+
1.210462f, 1.058761f, 4.003706f, 1.0f };
330+
331+
std::array<float, 12> testImage = srcImage;
320332

321333
for (const auto & op : ops)
322334
{
323-
op->apply(srcImage, 3);
335+
op->apply(testImage.data(), 3);
324336
}
325337

326338
// Compare results
327339
const float error = 2e-5f;
328340

329341
for (unsigned int i = 0; i<12; ++i)
330342
{
331-
OCIO_CHECK_CLOSE(srcImage[i], dstImage[i], error);
343+
OCIO_CHECK_CLOSE(testImage[i], dstImage[i], error);
332344
}
333345

334346
// Invert the processing.
@@ -337,24 +349,22 @@ OCIO_ADD_TEST(FileFormatICC, test_apply)
337349
OCIO_CHECK_NO_THROW(BuildOpsTest(opsInv, iccFileName, context, OCIO::TRANSFORM_DIR_FORWARD));
338350
OCIO_CHECK_NO_THROW(opsInv.finalize());
339351
OCIO_CHECK_NO_THROW(opsInv.optimize(OCIO::OPTIMIZATION_LOSSLESS));
352+
OCIO_REQUIRE_EQUAL(2, opsInv.size());
353+
OCIO_CHECK_EQUAL("<MatrixOffsetOp>", opsInv[0]->getInfo());
354+
OCIO_CHECK_EQUAL("<GammaOp>", opsInv[1]->getInfo());
340355

341356
for (const auto & op : opsInv)
342357
{
343-
op->apply(srcImage, 3);
358+
op->apply(testImage.data(), 3);
344359
}
345360

346-
// Values outside [0.0, 1.0] are clamped and won't round-trip.
347-
const float bckImage[] = {
348-
0.0f, 0.0f, 0.3f, 0.0f,
349-
0.4f, 0.5f, 0.6f, 0.5f,
350-
0.7f, 1.0f, 1.0f, 1.0f };
351-
352-
// Compare results
361+
// For pure-gamma TRCs, values outside [0.0, 1.0] are NOT clamped
362+
// thus those values should round-trip correctly.
353363
const float error2 = 2e-4f;
354364

355365
for (unsigned int i = 0; i<12; ++i)
356366
{
357-
OCIO_CHECK_CLOSE(srcImage[i], bckImage[i], error2);
367+
OCIO_CHECK_CLOSE(testImage[i], srcImage[i], error2);
358368
}
359369
}
360370

0 commit comments

Comments
 (0)