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

PR: Implement clamping support to chromaticity inspector. #2023

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

Conversation

KelSolaar
Copy link
Contributor

I added some new buttons to clamp the chromaticity inspector chromaticities by default in range [0, 1]. This helps giving some control over the transformation chain and alleviate some problems pointed out in #2010.

No Clamp

image

Note how the negative values exceed the boundaries of sRGB.

Clamped

image

@doug-walker
Copy link
Collaborator

@KelSolaar , I'm looking at the second screenshot you posted but I don't see the new button. This has the potential for confusion if ocioview is doing something different than applications that just apply the config "as is". My advice would be:

  1. Make the UI clearly indicate when a clamp is being applied.
  2. Have the default be "no clamp".

@KelSolaar
Copy link
Contributor Author

Hello @doug-walker,

The buttons are here, clamping negative and positive values:
image

This only affects the chromaticity inspector and is enabled by default, like with DCC typically, if we want something application wide for the Display Transforms, we will need to discuss about it.

Note that DCC do not tell you when they clamp, they do it, here we can toggle the behaviour for inspection.

Cheers,

Thomas

@doug-walker
Copy link
Collaborator

doug-walker commented Aug 24, 2024

Ah, thanks for pointing out the buttons @KelSolaar ! It was not obvious to me what those did (I was thinking "previous" and "next" rather than clamp). Would it be clearer to change them to "< 0" and "> 1" rather than just "<" and ">"?

Regarding the default, yes, if the DCC needs to send the values to a display eventually they will become integers and lose the extended range values. However, not all DCCs will insert an explicit clamp. Furthermore, not all DisplayViewTransform output will go to a display, for example, it could go into a file format which may or may not clamp. Even the integer video file formats often allow some amount of extended range. That was my thinking for making the default "no clamp".

That's my opinion, but I will defer to you and Michael on all of this.

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.

2 participants