Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Underflow bugfix & fix off-by-one error in the texture height calculation #1909

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hannes-vernooij
Copy link

@hannes-vernooij hannes-vernooij commented Nov 17, 2023

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.

EDIT:
In the initial commit I made the mistake of not taking into account that the last value of a row is repeated in the first value of the next row, this is now fixed. I validated the result by running a shaper LUT with a size of 4096, 4097, 8191 and 8192. I then applied the transform LUT on syntheticChart.01.exr and validated a 0 pixel difference.

Underflow bugfix

This also seems to solve an existing underflow bug that appeared with a 1D LUT with a size of 8191. When the height is calculated the value is not ceiled, which results in 8191 / 4096 = 2, while it should be 3.

This caused an underflow later in the code in:
unsigned long missingEntries = width * height - (unsigned long)paddedChannel.size();

where width * height is less than (unsigned long)paddedChannel.size(); causing an underflow that results in a very high value that is iterated.

Copy link

linux-foundation-easycla bot commented Nov 17, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@hannes-vernooij hannes-vernooij force-pushed the lut1d-off-by-one branch 3 times, most recently from 24a3863 to b078247 Compare November 17, 2023 15:25
@hannes-vernooij hannes-vernooij marked this pull request as draft December 1, 2023 14:15
@hannes-vernooij hannes-vernooij force-pushed the lut1d-off-by-one branch 6 times, most recently from 0b2fee9 to d03be01 Compare December 5, 2023 15:06
@hannes-vernooij hannes-vernooij changed the title Fix off-by-one error in the texture height calculation Underflow bugfix & Fix off-by-one error in the texture height calculation Dec 5, 2023
@hannes-vernooij hannes-vernooij marked this pull request as ready for review December 5, 2023 15:24
@hannes-vernooij hannes-vernooij changed the title Underflow bugfix & Fix off-by-one error in the texture height calculation Underflow bugfix & fix off-by-one error in the texture height calculation Dec 6, 2023
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]>
(131071U / 4096U) + 1 = 32

Signed-off-by: Hannes Vernooij <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant