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.

Sign up for GitHub

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

Jump to bottom

Lut-free built-ins #18

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

Lut-free built-ins #18

wants to merge 17 commits into from

Conversation

cozdas
Copy link

@cozdas cozdas commented Jul 20, 2024

  • WIP Fixed-function for PQ<->Linear function. Doesn't have the GPU implementation yet and the rest may need tweaks.

@cozdas cozdas requested a review from doug-walker July 20, 2024 01:24
doug-walker
doug-walker reviewed Jul 20, 2024
View reviewed changes
Copy link

@doug-walker doug-walker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to review more thoroughly, but you're on the right track.

There's a lot more to adding a new FixedFunction though. CTF support and Python support are required. Version checks must be updated to not allow the additions in config versions lower than 2.4. Unit tests must be added to test all of that.

Some of the other PRs that might be a useful reference are AcademySoftwareFoundation#1404, AcademySoftwareFoundation#1814, and AcademySoftwareFoundation#896.

include/OpenColorIO/OpenColorTypes.h Outdated Show resolved Hide resolved
@cozdas cozdas force-pushed the cozdas/main/Lut-Free-Built-Ins branch from 33cdc83 to 11d7e17 Compare July 26, 2024 21:04
doug-walker
doug-walker requested changes Aug 12, 2024
View reviewed changes
Copy link

@doug-walker doug-walker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update tests/cpu/Config_tests.cpp, fixed_function_serialization, around line 4795 to validate Yaml serialization. Then add a check (there are other examples in that test) that an exception will be thrown on validation if the version is less than 2.4.

Please update tests/cpu/fileformats/FileFormatCTF_tests.cpp, ff_load_save_ctf, around line 3841 to validate CTF serialization.

I'm realizing that we have not been as rigorous on CTF versioning as we have on config versioning. Given that we are adding a lot for 2.4 we should bump the CTF version. This could be done in a follow-up.

src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
include/OpenColorIO/OpenColorTypes.h Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/transforms/builtins/Displays.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/transforms/builtins/Displays.cpp Outdated Show resolved Hide resolved
tests/gpu/FixedFunctionOp_test.cpp Outdated Show resolved Hide resolved
@cozdas
- WIP Fixed-function for PQ<->Linear function. Doesn't have the GPU i…
f19c6a0 
…mplementation yet and the rest may need tweaks.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
more progress on FIXED_FUNCTION_PQ_TO_LINEAR
fc2c976 
- Python: Added docs
- Python: added the builtin function style
- Simplified the CPU code, it's still a prototype anyhow. Will optimize later.
- Style->OpData now takes the direction into account.
- Preliminary GPU implementation. This still clamps negative values.
- Added GPU tests.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- Fixed a range bug in the GPU test framework
92bc96c 
- Added symmetry to the PQ curve both in CPU and GPU implementations
- Added version check for the FIXED_FUNCTION_PQ_TO_LINEAR transform.
- Adjusting the GPU test threshold as GPU and CPU (32f) are showing mismatch more than the distance each have to the ground truth.
- Adding fixed function OP tests
- Temporary change to the PQ curve CPU code for quickly switching between single and double precision floating point for error analysis.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- Adding CTF tests for PQ_TO_LINEAR FixedFunction.
a2e5459 
Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas cozdas force-pushed the cozdas/main/Lut-Free-Built-Ins branch from 17f0783 to a2e5459 Compare August 13, 2024 02:40
@cozdas
- extending fixed_function_serialization test for PQ_TO_LINEAR fixedf…
4ff6383 
…unction

- minor fixes

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas cozdas marked this pull request as draft August 20, 2024 23:36
@cozdas
- GetFixedFunctionCPURenderer() now takes a parameter for fastpower f…
5711ad9 
…unction

- implemented SSE versions of the PQ curve in both directions with and without fastPower
- restored the old LUT-based PQ curve implementation that's used in the ST2084 Display builtin transform
- added custom range options to gpu unit tests
- changed the PQ curve test ranges to [-0.1, 1.1] in forward and [-0.1, 1001.1] in reverse directions

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- minor cleanup
70058e8 
Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- OglApp::redisplay() function now can return the rendering time in n…
ea417dd 
…anoseconds, this is optional, so the existing clients don't need to change.

- HLG curve in CIE_XYZ_D65_to_REC2100_HLG_1000nit_Functor is pulled into its own function to be reused later on.
- HLG curve is no longer clipping at 0, it mirrors instead. clipping at 1 is currently untouched, may need to remove that too.
- fine-tuned the unit test thresholds for PQ curve so that they pass in all build/use situations.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- hopefully fixing the Windows builds that turn off SSE2 support but …
4da39ac 
…gcc and clang will keep on failing as I realized that those compilers don't have the _mm_pow_ps() SVML library implementation as msvc does. Will need to change the logic a bit.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- more attempts to make all builds happy
9ea4553 
Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- re-worked the logic a bit more so that non-windows systems will eit…
f655010 
…her use the fast-SSE or scalar implementation, while Windows will be able to use the precise-SSE implementation too if needed. Fingers crossed.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
doug-walker
doug-walker requested changes Aug 24, 2024
View reviewed changes
src/OpenColorIO/transforms/builtins/Displays.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/transforms/builtins/Displays.cpp Outdated Show resolved Hide resolved
src/apps/ocioconvert/main.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
src/OpenColorIO/ops/fixedfunction/FixedFunctionOpCPU.cpp Outdated Show resolved Hide resolved
tests/cpu/ops/fixedfunction/FixedFunctionOpCPU_tests.cpp Outdated Show resolved Hide resolved
@cozdas
- Turns out mac OS does not implement the GL_TIME_ELAPSED query targe…
1de60e9 
…t which should be available since GL version 3.3 according to Khronos. Joy of cross-platform development :) Therefore removing the GPU render timing code from this PR. This is probably why GPU performance was not implemented in ocioperf already.
@cozdas
- Making gcc happy about the unused parameter. Probably it's a good i…
5ded1dd 
…dea to try to make all the compiler warning levels same/similar, some platforms being more picky about the same code than the others does not make much sense from the project's point of view, just frustration.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- One more step to make all compilers happy.
4b0d573 
Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- Another pass to make clang happy.
693f9b3 
Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- increasing the PQ_TO_LINEAR test threshold as ARM cpu seems to devi…
c7c2ee1 
…ate more.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas
- removed the run-time checks for sse2 instruction set availability p…
f5f59c7 
…er code review discussion

- BT_2100 HGL clamp at 1 is removed.
- Fixed function tests can now ask for fast math on or off explicitly.
- Fixed function PQ curve tests are now performed both with and without fast math. With fast math on, the thresholds are increased to accommodate the accumulated error during roundtrip.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@cozdas cozdas marked this pull request as ready for review August 27, 2024 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doug-walker doug-walker doug-walker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cozdas @doug-walker