Skip to content

Commit

Permalink
fixed off-by-one error in the pixel height calculation
Browse files Browse the repository at this point in the history
It should be max index / max size because 4096 / 4096 + 1 = 2, which means that two rows are allocated when a LUT1D with a size of 4096 fits in a 1D texture of 4096 pixels.

Signed-off-by: Hannes Vernooij <[email protected]>
  • Loading branch information
hannes-vernooij committed Dec 5, 2023
1 parent 41441bb commit 3e89f13
Showing 1 changed file with 29 additions and 6 deletions.
35 changes: 29 additions & 6 deletions src/OpenColorIO/ops/lut1d/Lut1DOpGPU.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ void CreatePaddedLutChannels(unsigned long width,
leftover -= step;
}

// Skip the duplicate entry if the last value is the last pixel on a row
if ((currWidth + (height -1)) % width == 0) {
leftover -= 1;
}

// If there are still texels to fill, add them to the texture data.
if (leftover > 0)
{
Expand Down Expand Up @@ -111,6 +116,11 @@ void CreatePaddedRedChannel(unsigned long width,
leftover -= step;
}

// Skip the duplicate entry if the last value is the last pixel on a row
if ((currWidth + (height -1)) % width == 0) {
leftover -= 1;
}

// If there are still texels to fill, add them to the texture data.
if (leftover > 0)
{
Expand Down Expand Up @@ -154,13 +164,27 @@ void GetLut1DGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator,

const unsigned long length = lutData->getArray().getLength();
const unsigned long width = std::min(length, defaultMaxWidth);
const unsigned long height = (length / defaultMaxWidth) + 1;
const unsigned long heightWithDuplicates = (length - 1) / (defaultMaxWidth - 1) + 1;

// The height is calculated based on the amount of rows without duplicate data.
// ceil(length / defaultMaxWidth)
const unsigned long heightWithoutDuplicates = (std::max(2UL, length) - 1) / defaultMaxWidth + 1;

// The result of this determines how many duplicate values are really needed.
// When the amount of rows is one, the amount of duplicate entries will be zero.
// ceil(length / defaultMaxWidth) -1)
const unsigned long numDuplicateEntries = heightWithoutDuplicates - 1;

// Once we know the amount of duplicate entries, we can calculate if the last
// value ends on the same scanline, if that is the case we won't need an extra line.
// ((ceil(length / defaultMaxWidth) -1 + length) % defaultMaxWidth == 0)
const unsigned long height = heightWithDuplicates - ((numDuplicateEntries + length) % defaultMaxWidth == 0);
const unsigned long numChannels = lutData->getArray().getNumColorComponents();

// Note: The 1D LUT needs a GPU texture for the Look-up table implementation.
// Note: The 1D LUT needs a GPU texture for the Look-up table implementation.
// However, the texture type & content may vary based on the number of channels
// i.e. when all channels are identical a F32 Red GPU texture is enough.

const bool singleChannel = (numChannels == 1);

// Adjust LUT texture to allow for correct 2d linear interpolation, if needed.
Expand Down Expand Up @@ -335,13 +359,13 @@ void GetLut1DGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator,
{
const std::string str = name + "_computePos(" + shaderCreator->getPixelName();

ss.newLine() << shaderCreator->getPixelName() << ".r = "
ss.newLine() << shaderCreator->getPixelName() << ".r = "
<< ss.sampleTex2D(name, str + ".r)") << ".r;";

ss.newLine() << shaderCreator->getPixelName() << ".g = "
<< ss.sampleTex2D(name, str + ".g)") << (singleChannel ? ".r;" : ".g;");

ss.newLine() << shaderCreator->getPixelName() << ".b = "
ss.newLine() << shaderCreator->getPixelName() << ".b = "
<< ss.sampleTex2D(name, str + ".b)") << (singleChannel ? ".r;" : ".b;");
}
else
Expand Down Expand Up @@ -387,4 +411,3 @@ void GetLut1DGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator,
}

} // namespace OCIO_NAMESPACE

0 comments on commit 3e89f13

Please sign in to comment.