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

Add footnote about colors being in-gamut (i.e. components in [0,1]) #208

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

Conversation

portsmouth
Copy link
Contributor

Based on the interesting discussion in #149, it seems worth adding this side note:

image

@portsmouth
Copy link
Contributor Author

@anderslanglands and @KelSolaar Would appreciate your thoughts on whether this change makes sense.

@KelSolaar
Copy link

It might also be:

"... all RGB colours are in-gamut of the working space, i.e., have components in the $[0, +\inf]$ range"

  • "In-gamut" of what: We need to specify the container
  • Why limiting to 1]? Colour emission should not be restricted so we might need a case for albedo et al. and another one for emission.

@portsmouth
Copy link
Contributor Author

portsmouth commented Jun 4, 2024

@KelSolaar

It might also be: "... all RGB colours are in-gamut of the working space, i.e., have components in the $[0, \infty]$ range"

OK, so in your view "in-gamut" (of the working color space of the shader) simply means that the color components are non-negative?

"In-gamut" of what: We need to specify the container

Can you clarify what you mean by container? We say in the spec that "A color3 value is associated with a color space ACEScg by default" and that "The assumed color space of all the color parameters .. by default .. is assumed to be ACEScg".

So "in-gamut" is implied to mean in-gamut of that color space (or ACEScg if unspecified). Is that still under-specified in your view?

Why limiting to 1? Colour emission should not be restricted so we might need a case for albedo et al. and another one for emission.

Yes right, so emission color should not be restricted to [0,1], but it seemed like according to our earlier discussion we had agreed that for all the other colors, which represent albedos or fractional tints, they should be restricted to [0,1] (regardless of the color space). Do you still agree with that?

@KelSolaar
Copy link

OK, so in your view "in-gamut" (of the working color space of the shader) simply means that the color components are non-negative?

Yes, e.g., HDR values are considered in-gamut of their colourspace for example.

Is that still under-specified in your view?

I would certainly be very specific, there is an infinity of gamuts and it does not cost much to add.

Do you still agree with that?

Yes!

@portsmouth
Copy link
Contributor Author

Sorry can you clarify what you mean by “there is an infinity of gamuts”?

We say the color space must be specified (or is ACEScg by default). How can we be more specific that that?

@KelSolaar
Copy link

KelSolaar commented Jun 4, 2024

We are splitting hairs here but "gamut" by definition (Oxford Languages) is "the complete range or scope of something", what I'm saying is that the notes does not explicitly specifies it.

Note that we assume that all RGB colors are in the specified color space's gamut, i.e. have components in the $[0, +inf]$ range.

This is better because the "something" is specified explicitly in the sentence right next to "gamut" and you do not have to reach the end of the sentence to understand the full context.

In theory, the i.e., portion of the sentence could be omitted and the sentence would be explicit by itself, I could only write:

Note that we assume that all RGB colors are in the specified color space's gamut.

@portsmouth portsmouth changed the base branch from main to dev_1.1 June 7, 2024 16:35
@portsmouth portsmouth changed the base branch from dev_1.1 to main June 7, 2024 16:37
@portsmouth
Copy link
Contributor Author

@jstone-lucasfilm Note, don't merge this one yet. There needs to be some changes to the way we describe the color ranges in the spec.

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.

None yet

2 participants