AcademySoftwareFoundation / OpenPBR

Specification and reference implementation for the OpenPBR Surface shading model
Apache License 2.0
401 stars 17 forks source link

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

Open portsmouth opened 1 month ago

portsmouth commented 1 month ago

Based on the interesting discussion in https://github.com/AcademySoftwareFoundation/OpenPBR/issues/149, it seems worth adding this side note:

image

portsmouth commented 1 month ago

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

KelSolaar commented 1 month ago

It might also be:

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

portsmouth commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 4 weeks ago

@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.