Skip to content

Commit

Permalink
Merge pull request #25 from autodesk-forks/walker/bug/invlut-fix
Browse files Browse the repository at this point in the history 
Fix bit-depth attributes on inverse LUT writing to CTF format
  • Loading branch information
@doug-walker
doug-walker authored Nov 11, 2024
2 parents 45d3aba + 4949960 commit 2a60d37
Show file tree
Hide file tree
Showing 2 changed files with 180 additions and 21 deletions.
66 changes: 58 additions & 8 deletions src/OpenColorIO/fileformats/ctf/CTFTransform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ const char * BitDepthToCLFString(BitDepth bitDepth)
else if (bitDepth == BIT_DEPTH_F16) return "16f";
else if (bitDepth == BIT_DEPTH_F32) return "32f";

throw Exception("Bitdepth has been validated before calling this.");
throw Exception("Bitdepth has not been validated before calling this.");
}

BitDepth GetValidatedFileBitDepth(BitDepth bd, OpData::Type type)
Expand Down Expand Up @@ -2125,7 +2125,9 @@ void Lut1DWriter::writeContent() const

// To avoid needing to duplicate the const objects,
// we scale the values on-the-fly while writing.
const float scale = (float)GetBitDepthMaxValue(m_outBitDepth);
const BitDepth arrayBitDepth = m_lut->getDirection() == TRANSFORM_DIR_INVERSE ?
m_inBitDepth : m_outBitDepth;
const float scale = (float) GetBitDepthMaxValue(arrayBitDepth);

if (m_lut->isOutputRawHalfs())
{
Expand Down Expand Up @@ -2155,7 +2157,7 @@ void Lut1DWriter::writeContent() const
values.begin(),
values.end(),
array.getNumColorComponents(),
m_outBitDepth,
arrayBitDepth,
array.getNumColorComponents() == 1 ? 3 : 1,
scale);
}
Expand Down Expand Up @@ -2249,12 +2251,15 @@ void Lut3DWriter::writeContent() const

// To avoid needing to duplicate the const objects,
// we scale the values on-the-fly while writing.
const float scale = (float)GetBitDepthMaxValue(m_outBitDepth);
const BitDepth arrayBitDepth = m_lut->getDirection() == TRANSFORM_DIR_INVERSE ?
m_inBitDepth : m_outBitDepth;
const float scale = (float) GetBitDepthMaxValue(arrayBitDepth);

WriteValues(m_formatter,
array.getValues().begin(),
array.getValues().end(),
3,
m_outBitDepth,
arrayBitDepth,
1,
scale);

Expand Down Expand Up @@ -2647,13 +2652,38 @@ BitDepth GetInputFileBD(ConstOpDataRcPtr op)
const auto bd = range->getFileInputBitDepth();
return GetValidatedFileBitDepth(bd, type);
}
else if (type == OpData::Lut1DType)
{
// For an InverseLut1D, the file "out" depth, which determines the array scaling,
// must actually be set as the in-depth.
auto lut = OCIO_DYNAMIC_POINTER_CAST<const Lut1DOpData>(op);
if (lut->getDirection() == TRANSFORM_DIR_INVERSE)
{
const auto bd = lut->getFileOutputBitDepth();
return GetValidatedFileBitDepth(bd, type);
}
}
else if (type == OpData::Lut3DType)
{
// For an InverseLut3D, the file "out" depth, which determines the array scaling,
// must actually be set as the in-depth.
auto lut = OCIO_DYNAMIC_POINTER_CAST<const Lut3DOpData>(op);
if (lut->getDirection() == TRANSFORM_DIR_INVERSE)
{
const auto bd = lut->getFileOutputBitDepth();
return GetValidatedFileBitDepth(bd, type);
}
}
return BIT_DEPTH_F32;
}

}

void TransformWriter::writeOps(const CTFVersion & version) const
{
// If the ops have a specific file input/output bit-depth that was set (e.g., if the
// ops were originally read in from a CTF/CLF file), then we try to preserve those
// values on write. Otherwise, default to 32f.
BitDepth inBD = BIT_DEPTH_F32;
BitDepth outBD = BIT_DEPTH_F32;

Expand All @@ -2662,15 +2692,27 @@ void TransformWriter::writeOps(const CTFVersion & version) const
size_t numSavedOps = 0;
if (numOps)
{
// Initialize the input bit-depth from the first op. Thereafter, the input depth
// will be determined from the output depth of the previous op that was written.
inBD = GetInputFileBD(ops[0]);

for (size_t i = 0; i < numOps; ++i)
{
auto & op = ops[i];

// Each op is allowed to set its preferred output depth, so that is always
// respected. However, we try to honour preferred input depth too, where possible.
// If the next op has a preferred file input bit-depth, use that as the default
// out-depth for the current op. However, if the current op has a preferred
// file out depth, that will take precedence below. If an op's preferred file
// depth cannot be honoured, it still results in a perfectly valid CTF file,
// it's only that the scaling of array values may be adjusted relative to the
// original CTF file it was created from (if any).
// See FileFormatCTF_tests.cpp test bitdepth_ctf.
if (i + 1 < numOps)
{
auto & nextOp = ops[i + 1];
// Return file input bit-depth for Matrix & Range, F32 for others.
// Return file input bit-depth for ops with a preference, F32 for others.
outBD = GetInputFileBD(nextOp);
}

Expand Down Expand Up @@ -2841,7 +2883,11 @@ void TransformWriter::writeOps(const CTFVersion & version) const
// Avoid copying LUT, write will take bit-depth into account.
Lut1DWriter opWriter(m_formatter, lut);

outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type);
if (lut->getDirection() == TRANSFORM_DIR_FORWARD)
{
// For an inverse Lut1D, the fileOutDepth is used for the inBitDepth above.
outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type);
}
opWriter.setInputBitdepth(inBD);
opWriter.setOutputBitdepth(outBD);

Expand All @@ -2861,7 +2907,11 @@ void TransformWriter::writeOps(const CTFVersion & version) const
// Avoid copying LUT, write will take bit-depth into account.
Lut3DWriter opWriter(m_formatter, lut);

outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type);
if (lut->getDirection() == TRANSFORM_DIR_FORWARD)
{
// For an inverse Lut3D, the fileOutDepth is used for the inBitDepth above.
outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type);
}
opWriter.setInputBitdepth(inBD);
opWriter.setOutputBitdepth(outBD);

Expand Down
135 changes: 122 additions & 13 deletions tests/cpu/fileformats/FileFormatCTF_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,41 @@ OCIO_ADD_TEST(FileFormatCTF, lut1d_inv)
OCIO_CHECK_CLOSE(a2.getValues()[50], 1.0f, error);
}

OCIO_ADD_TEST(FileFormatCTF, lut1d_inv_scaling)
{
// Validate that the InverseLUT1D array values are scaled based on inBitDepth.
// (The previous example had inBitDepth=32f, so it does not validate that.)

OCIO::LocalCachedFileRcPtr cachedFile;
const std::string ctfFile("lut1d_inverse_halfdom_slog_fclut.ctf");
OCIO_CHECK_NO_THROW(cachedFile = LoadCLFFile(ctfFile));
OCIO_REQUIRE_ASSERT((bool)cachedFile);

const OCIO::ConstOpDataVec & opList = cachedFile->m_transform->getOps();
OCIO_REQUIRE_EQUAL(opList.size(), 1);

auto pLut = std::dynamic_pointer_cast<const OCIO::Lut1DOpData>(opList[0]);
OCIO_REQUIRE_ASSERT(pLut);
// For an InverseLUT1D, the file "out" depth is actually taken from inBitDepth.
OCIO_CHECK_EQUAL(pLut->getFileOutputBitDepth(), OCIO::BIT_DEPTH_UINT16);
OCIO_CHECK_EQUAL(pLut->getDirection(), OCIO::TRANSFORM_DIR_INVERSE);

const OCIO::Array & a2 = pLut->getArray();
OCIO_CHECK_EQUAL(a2.getNumColorComponents(), 1);

OCIO_CHECK_EQUAL(a2.getLength(), 65536);
OCIO_CHECK_EQUAL(a2.getNumValues(),
a2.getLength()*a2.getMaxColorComponents());

const float error = 1e-6f;
OCIO_REQUIRE_EQUAL(a2.getValues().size(), a2.getNumValues());

// Input value 17830 scaled by 65535.
OCIO_CHECK_CLOSE(a2.getValues()[0], 0.27206836f, error);
// Input value 55070 scaled by 65535.
OCIO_CHECK_CLOSE(a2.getValues()[31743 * 3], 0.84031434f, error);
}

namespace
{
OCIO::LocalCachedFileRcPtr ParseString(const std::string & str)
Expand Down Expand Up @@ -1056,7 +1091,9 @@ OCIO_ADD_TEST(FileFormatCTF, lut3d_inv)
auto pLut = std::dynamic_pointer_cast<const OCIO::Lut3DOpData>(opList[0]);
OCIO_REQUIRE_ASSERT(pLut);

// For an InverseLUT3D, the file "out" depth is set by the inBitDepth of the file.
OCIO_CHECK_EQUAL(pLut->getFileOutputBitDepth(), OCIO::BIT_DEPTH_UINT12);

OCIO_CHECK_EQUAL(pLut->getInterpolation(), OCIO::INTERP_TETRAHEDRAL);
OCIO_CHECK_EQUAL(pLut->getDirection(), OCIO::TRANSFORM_DIR_INVERSE);

Expand All @@ -1067,6 +1104,7 @@ OCIO_ADD_TEST(FileFormatCTF, lut3d_inv)
*array.getLength()*array.getMaxColorComponents());
OCIO_REQUIRE_EQUAL(array.getValues().size(), array.getNumValues());

// Validate that the array was scaled by the inBitDepth of the file.
OCIO_CHECK_EQUAL(array.getLength(), 17);
OCIO_CHECK_CLOSE(array.getValues()[0], 25.0f / 4095.0f, 1e-8f);
OCIO_CHECK_CLOSE(array.getValues()[1], 30.0f / 4095.0f, 1e-8f);
Expand Down Expand Up @@ -7644,9 +7682,12 @@ OCIO_ADD_TEST(CTFTransform, lut1d_inverse_ctf)
OCIO_CHECK_NO_THROW(WriteGroupCTF(group, outputTransform));

// Note the type of the node.
//
// For an InverseLUT1D, the scaling of array values is based on the inBitDepth.
//
const std::string expected{ R"(<?xml version="1.0" encoding="UTF-8"?>
<ProcessList version="1.3" id="UIDLUT42">
<InverseLUT1D id="lut01" name="test-lut" inBitDepth="32f" outBitDepth="10i">
<InverseLUT1D id="lut01" name="test-lut" inBitDepth="10i" outBitDepth="32f">
<Array dim="16 3">
0 1 2
3 4 5
Expand Down Expand Up @@ -7812,9 +7853,12 @@ OCIO_ADD_TEST(CTFTransform, lut3d_inverse_ctf)
OCIO_CHECK_NO_THROW(WriteGroupCTF(group, outputTransform));

// Note the type of the node.
//
// For an InverseLUT3D, the scaling of array values is based on the inBitDepth.
//
const std::string expected{ R"(<?xml version="1.0" encoding="UTF-8"?>
<ProcessList version="1.6" id="UIDLUT42">
<InverseLUT3D id="lut01" name="test-lut3d" inBitDepth="32f" outBitDepth="10i">
<InverseLUT3D id="lut01" name="test-lut3d" inBitDepth="10i" outBitDepth="32f">
<Array dim="3 3 3 3">
0 1 2
3 4 5
Expand Down Expand Up @@ -7946,39 +7990,80 @@ OCIO_ADD_TEST(CTFTransform, bitdepth_ctf)
auto range = OCIO::RangeTransform::Create();
range->setFileInputBitDepth(OCIO::BIT_DEPTH_F16);
range->setFileOutputBitDepth(OCIO::BIT_DEPTH_UINT12);
range->setMinInValue(0.);
range->setMinOutValue(0.);
range->setMinInValue(0.1);
range->setMinOutValue(-0.1);
range->setMaxInValue(0.9);
range->setMaxOutValue(1.1);

auto log = OCIO::LogTransform::Create();

auto invlut = OCIO::Lut1DTransform::Create();
invlut->setDirection(OCIO::TRANSFORM_DIR_INVERSE);
invlut->setFileOutputBitDepth(OCIO::BIT_DEPTH_UINT16);
invlut->setLength(3);

auto mat2 = OCIO::MatrixTransform::Create();
mat2->setFileInputBitDepth(OCIO::BIT_DEPTH_UINT8);
mat2->setFileOutputBitDepth(OCIO::BIT_DEPTH_UINT10);

auto log = OCIO::LogTransform::Create();
mat2->setDirection(OCIO::TRANSFORM_DIR_INVERSE);

OCIO::GroupTransformRcPtr group = OCIO::GroupTransform::Create();
group->getFormatMetadata().addAttribute(OCIO::METADATA_ID, "UID42");

// First op keeps bit-depth
// Transforms are setup as follows:
//
// Matrix fileIn = 8i, fileOut = 10i
// Lut1D fileOut = 10i
// Exponent
// Range fileIn = 16f, fileOut = 12i
// Matrix fileIn = 8i, fileOut = 10i
// Log
// InvLut1D fileOut = 16i
// InvMatrix fileIn = 8i, fileOut = 10i
// InvLut1D fileOut = 16i

// First op keeps its in & out bit-depth.
// <Matrix inBitDepth="8i" outBitDepth="10i">
group->appendTransform(mat);

// Previous op out bit-depth used for in bit-depth.
// <LUT1D inBitDepth="10i" outBitDepth="10i">
group->appendTransform(lut);

// Previous op out bit-depth used for in bit-depth.
// And next op (range) in bit-depth used for out bit-depth.
// <Exponent inBitDepth="10i" outBitDepth="16f">
group->appendTransform(exp);

// In bit-depth preserved and has been used for out bit-depth of previous op.
// Next op is a matrix, but current op is range, first op out bit-depth
// is preserved and used for next op in bit-depth.
// is preserved and overrides the next op's in in bit-depth.
// <Range inBitDepth="16f" outBitDepth="12i">
group->appendTransform(range);

// Previous op out bit-depth used for in bit-depth.
group->appendTransform(mat2);
// <Matrix inBitDepth="12i" outBitDepth="10i">
group->appendTransform(mat); // 2);

// Previous op out bit-depth used for in bit-depth.
// Previous op out bit-depth used for in bit-depth. Out depth is set by preference
// of the next op.
// <Log inBitDepth="10i" outBitDepth="16i">
group->appendTransform(log);

// Preferred in bit-depth is preserved. Out depth is set by the next op.
// <InverseLUT1D inBitDepth="16i" outBitDepth="32f">
group->appendTransform(invlut);

// Sets both its preferred in and out depth. Note that because the transform
// direction is inverse, the mapping of file depths is swapped.
// <Matrix inBitDepth="10i" outBitDepth="8i">
group->appendTransform(mat2);

// This time it doesn't get its preferred in depth, since the previous op has priority.
// The array values are scaled accordingly.
// <InverseLUT1D inBitDepth="8i" outBitDepth="32f">
group->appendTransform(invlut);

std::ostringstream outputTransform;
OCIO_CHECK_NO_THROW(WriteGroupCTF(group, outputTransform));

Expand All @@ -8002,8 +8087,10 @@ OCIO_ADD_TEST(CTFTransform, bitdepth_ctf)
<ExponentParams exponent="1" />
</Exponent>
<Range inBitDepth="16f" outBitDepth="12i">
<minInValue> 0 </minInValue>
<minOutValue> 0 </minOutValue>
<minInValue> 0.1 </minInValue>
<maxInValue> 0.9 </maxInValue>
<minOutValue> -409.5 </minOutValue>
<maxOutValue> 4504.5 </maxOutValue>
</Range>
<Matrix inBitDepth="12i" outBitDepth="10i">
<Array dim="3 3">
Expand All @@ -8012,10 +8099,32 @@ OCIO_ADD_TEST(CTFTransform, bitdepth_ctf)
0 0 0.24981684981685
</Array>
</Matrix>
<Log inBitDepth="10i" outBitDepth="32f" style="log2">
<Log inBitDepth="10i" outBitDepth="16i" style="log2">
</Log>
<InverseLUT1D inBitDepth="16i" outBitDepth="10i">
<Array dim="3 1">
0
32767.5
65535
</Array>
</InverseLUT1D>
<Matrix inBitDepth="10i" outBitDepth="8i">
<Array dim="3 3">
0.249266862170088 0 0
0 0.249266862170088 0
0 0 0.249266862170088
</Array>
</Matrix>
<InverseLUT1D inBitDepth="8i" outBitDepth="32f">
<Array dim="3 1">
0
127.5
255
</Array>
</InverseLUT1D>
</ProcessList>
)" };

OCIO_CHECK_EQUAL(expected.size(), outputTransform.str().size());
OCIO_CHECK_EQUAL(expected, outputTransform.str());
}
Expand Down

0 comments on commit 2a60d37

Please sign in to comment.