three.js
three.js copied to clipboard
WebGLRenderer: Rename outputEncoding to outputColorSpace.
Related issue: #23614
Description
Renames outputEncoding
to outputColorSpace
as suggested in #23614 (section 1.3).
I'm certainly in favor of the change β my only reservation is about which of these three changes can be done independently and which should be batched into a single release:
- renderer.outputEncoding -> outputColorSpace
- renderer.outputXXX default -> sRGB
- texture.encoding -> colorSpace
Changing the name might be as good a time as any to change the default.
How about using r141
as the target release? A soon as this PR is merged at the end of next week, I'll make the next one and rename Texture.encoding
to Texture.colorSpace
. After that, we change the default value of Texture.colorSpace
and WebGLRenderer.outputColorSpace
and update the examples/docs/manual accordingly.
(From https://github.com/mrdoob/three.js/pull/23937#discussion_r856938059) I think that not only this property should be renamed, but its value should be changed to LinearSRGBColorSpace/SRGBColorSpace instead of the LinearEncoding/sRGBEncoding (and the encodings constants can be set equal to the color spaces for the backwards compatibility).
Yes, the constants should be changed too. But it's sufficient to note this at #23614. Please let's keep the discussion focused on what the PR is about. Renaming WebGLRenderer.outputEncoding
.
@Mugen87 I can't tell if you're saying we should wait to replace the constants until a different release or a different PR. I think a different PR is fine but not a different release. Note that the SRGBColorSpace
and LinearSRGBColorSpace
constants already exist. If you still prefer to wait that's fine with me. π
I'll just not here as I have in other issues that naming this field something like canvasColorSpace
could be more clear.
I can't tell if you're saying we should wait to replace the constants until a different release or a different PR.
Different PR. Otherwise the PRs are going to be too complex. I'd like to implement the things you have mentioned in https://github.com/mrdoob/three.js/pull/23936#issuecomment-1107513416 in a single release. Ideally r141
.
I'll just not here as I have in other issues that naming this field something like canvasColorSpace could be more clear.
I think it's best if @mrdoob makes a final decision here π . The options are:
- The current implemented
WebGLRenderer.outputColorSpace
(the original suggestion of #23614) - The simplified version
WebGLRenderer.colorSpace
. However, it was highlighted in https://github.com/mrdoob/three.js/issues/23614#issuecomment-1107467806 that this version is confusing since the name does not distinct between working and output color space. - @gkjohnson suggested
WebGLRenderer.canvasColorSpace
. This name would highlightWebGLRenderer.canvasColorSpace
only affects the rendering to the canvas/default framebuffer.
After some more thinking I guess I vote for WebGLRenderer.canvasColorSpace
. It seems indeed the most accurate/descriptive name. I'm making this vote under the assumption that we never evaluate WebGLRenderer.canvasColorSpace
when rendering to render targets (see https://github.com/mrdoob/three.js/issues/23614#issuecomment-1107638945).
Either outputColorSpace
or canvasColorSpace
are OK with me. The thing that confused me about the original name was that it is ignored if writing to canvas comes at the end of a post-processing chain. I wish we could fix that but I suppose it's out of scope here, and it's the behavior not the name that should ideally be changed for that issue.
I would consider parallelism among renderers though. Does canvasColorSpace
still make sense for WebGPURenderer? With WebXR APIs? I've been hoping to add an outputColorSpace
option to SVGRenderer at some point βΒ the only renderer that could actually support a wider-gamut Display P3 output to display today β and obviously canvasColorSpace
does not make sense there.
Could then it be named displayColorSpace
?
that it is ignored if writing to canvas comes at the end of a post-processing chain
WebGLRenderer.outputEncoding
triggers the color space conversion in GLSL shaders. That only works for built-in materials but not for custom shader passes. That's the reasons why you need an additional post processing pass for color space conversion.
I've tried to eliminate this inconsistency in #23019 because it also introduces a transparency issue. The idea was to remove the inline tone mapping and color space conversion and always apply them with an additional post processing pass. However, this approach introduces a noticeable performance degradation. So at least with WebGLRenderer
, we have to stick to the current approach.
Does canvasColorSpace still make sense for WebGPURenderer?
Yes. But ideally it is not necessary anymore to perform the final color space conversion inline, see https://github.com/mrdoob/three.js/issues/23019#issuecomment-1011221811.
With WebXR APIs?
WebXRManager
evaluates WebGLRenderer.outputEncoding
for defining the right color space for its projection layers/framebuffers. But I think the name canvasColorSpace
is okay in this context.
I've been hoping to add an outputColorSpace option to SVGRenderer at some point
Well yes, in this case outputColorSpace
would be better since it's the more generic term.
Could then it be named displayColorSpace?
See https://github.com/mrdoob/three.js/issues/23614#issuecomment-1107467806. If we decide against canvasColorSpace
, outputColorSpace
seems the best choice.
The only part of "outputEncoding" I would change is "Encoding". Output is a common enough term for what it does, I think we can clear up any confusion around render targets in other ways.
Display would probably be too specific, as we also use the renderer with OffscreenCanvas, or to bake IBL, or to save a scene screenshot to disk.
Okay, then I change my vote back to outputColorSpace
π .
So at least with WebGLRenderer, we have to stick to the current approach.
I think the design in pmndrs/postprocessing
is perhaps a good alternative. But we can keep that discussion in #23937 or elsewhere.
This PR looks good to me for r141. ππ»
Well, I'm gonna be pesky about naming stuff, hope you folks don't mind it.
Turns out, color-space is a loaded term. However it does have an established meaning, which is a combination of both a standard gamut and a set of transfer functions. ( and a white-point of calibration, if you wanna be really pesky about it )
But that's not what we are really changing when we set an output encode for the final render, be it in built-in shaders or post-processing step. If I'm not mistaken, THREE.js hasn't adopted any support in regards to operating in anything other than standard sRGB gamut. ( can't use Adobe RGB, DCI-P3 ... )
So, if we are not changing gamuts. The only other thing we can be doing is changing if the data is encoded as linear or perceptual data. This is sometimes referred to as EOTF ( Electro-Optical Transfer Function ). Which is indeed sometimes referred to as gamma or simply sRGB, but in practice is just a single set of linear <-> perceptual functions possible.
I don't expect we to name it outputEOTF
as there is no value in naming it something the large comunity won't recognize. But the word 'encode' does better represent what we are doing. Either we are rendering in linear ( energy intensity ) which can be later operated on by other various post-processing steps or perhaps exported to a file ( HDR, EXR ), or perceptually encoded which is meant to be the encoded in the way our eyes interpret light. or as I like to say "human encoded light".
Perhaps we can keep outputEncoding
? π
If I'm not mistaken, THREE.js hasn't adopted any support in regards to operating in anything other than standard sRGB gamut.
Has not yet, but AFAIK aim to in the future.
If I'm not mistaken, THREE.js hasn't adopted any support in regards to operating in anything other than standard sRGB gamut.
Has not yet, but AFAIK aim to in the future.
My point exactly, when we do. Is gonna be easier to both adopt and understand, if we name things more carefully now. Even if we are operating in different color-space, we are still gonna need to ask if we want the render in linear or perceptual data. So it's better if we don't confuse people into thinking those and color spaces are the same thing.
@romainguy have I learned anything from your talks?
Thanks for the comments @sciecode!
The summary in https://github.com/mrdoob/three.js/issues/23614 covers where we are (tentatively) headed in the longer term, which is meant to be self-consistent in the end. We can see where WebGL, WebGPU, and other Web APIs are going in various in-progress specifications...
- https://github.com/w3c/ColorWeb-CG/blob/master/hdr_html_canvas_element.md
- https://github.com/WICG/canvas-color-space
- https://www.w3.org/TR/css-color-4/#predefined
In all of these, the API provides pre-defined color spaces like "srgb", "srgb-linear", and "display-p3", but the web does not expose the transfer function as a distinct user-configurable option. I believe users understand "sRGBEncoding" as synonymous with "the sRGB color space" in three.js today. I wish three.js terminology were more precise β whether we mean color space or transfer function β because "encoding" is ambiguous.
When Display P3 (supported on many mobile displays now) lands, it will have the same transfer function as sRGB. I am proposing...
// this
renderer.outputColorSpace = THREE.DisplayP3ColorSpace;
// not this
renderer.outputEncoding = THREE.sRGBEncoding;
renderer.outputPrimaries = THREE.DisplayP3Primaries;
I've also tried to document these terms better in https://threejs.org/docs/#manual/en/introduction/Color-management, but we're in a weird middle-state at the moment where both "LinearSRGBColorSpace" and "LinearEncoding" enums are in use at the same time.
So it's better if we don't confuse people into thinking those and color spaces are the same thing
In all of these, the API provides pre-defined color spaces like "srgb", "srgb-linear", and "display-p3"
Well, spec makers definitely aren't making it any easier for us, are they?
I'll take a look at the proprosed plan and comment there if it makes sense.
But theoretically, just defining just a color space would not be suficient. That's why they need to create things like "srgb" and "srgb-linear", because the official API also doesn't understand nomenclature. Thanks for letting me know @donmccurdy, will reply after I'm done reading the proposed plan.
It may not be too late to influence the designs for WebGL and WebGPU, if we feel the need. But personally β knowing that a color space is a choice of {primaries, white point, transfer function} β I am OK with selecting color space as a unit, rather than exposing the individual properties. If I look at an OpenColorIO config those are mostly defined in terms of named color spaces and transforms to/from a reference space β and this is sufficient for professional VFX software, Blender, Maya, and Unreal Engine.
It may not be too late to influence the designs for WebGL and WebGPU
I might give it a try perhaps, as I don't see a problem in having a single "setter" for defining the combination for a color-space and the encoding, but making it perhaps clearer of both the color-space and encoding in every option seems more prudent to me.
As I do believe this API is fundamental for shaping the future of color management in the web and guiding the first principles of color management for many developers.
We could also make our (new) enums more specific, while keeping the .colorSpace
terminology. For example:
// Transfer Function, Primaries, White Point
renderer.outputColorSpace = THREE.SRGB_Rec709_D65; // sRGB
renderer.outputColorSpace = THREE.Linear_Rec709_D65; // Linear-sRGB
renderer.outputColorSpace = THREE.SRGB_DisplayP3_D65; // Display-P3
I worry it will scare people, but it's certainly precise. I'd also considered proposing that we just use the color space identifier strings defined by CSS Color Module Level 4...
renderer.outputColorSpace = 'srgb';
renderer.outputColorSpace = 'srgb-linear';
The current THREE.SRGBColorSpace and THREE.LinearSRGBColorSpace enums do use those as values. There is some benefit to fewer parameters than more, as these go into getters/setters like color.setHex. But those APIs are new enough (and gated behind THREE.ColorManagement.legacyMode = false) that they can still be changed.
I agree, specifying every thing is counter productive in terms of usability. I think a standard name_id-encode
would be sufficient in any case. I would just love if both three and official API used:
renderer.outputColorSpaceEncoding = 'p3-perceptual' // 'p3-linear', 'srgb-perceptual'
or
renderer.outputColorSpace = 'displayP3' // 'sRGB', 'AdobeRGB'
renderer.outputEncoding = 'linear' // 'perceptual'
Just for clarity sake, in the end you are still setting a single property as a tuple or two properties as singles.
Apparently they chose to omit the encoding in perceptual
case, which is not horrible. Just is what it is.
Given what you just mentioned, I dont oppose the naming. Will still review the plan in its entirety when I have the time π
edit: maybe just renderer.colorSpaceEncoding
, I don't know π
We could also make our (new) enums more specific, while keeping the
.colorSpace
terminology. For example:// Transfer Function, Primaries, White Point renderer.outputColorSpace = THREE.SRGB_Rec709_D65; // sRGB renderer.outputColorSpace = THREE.Linear_Rec709_D65; // Linear-sRGB renderer.outputColorSpace = THREE.SRGB_DisplayP3_D65; // Display-P3
I very much like this proposal :) I think this is better than "perceptual" since perceptual says nothing about which encoding is being used. And while I understand it might scare off people, it's also how we may avoid perpetuating the ongoing confusion about sRGB, Linear, etc.
I worry it will scare people, but it's certainly precise.
Maybe we can use aliases? Like THREE.SRGB = THREE.SRGB_Rec709_D65
?
I think this is better than "perceptual" since perceptual says nothing about which encoding is being used. And while I understand it might scare off people, it's also how we may avoid perpetuating the ongoing confusion about sRGB, Linear, etc.
Agreed, I would like to avoid the term "perceptual" in the enums for that reason. We'll likely need to be able to distinguish between multiple different perceptual transfer functions, when HDR canvas APIs become available. For example: sRGB, PQ, and HLG.
I'm satisfied with either of the alternatives in https://github.com/mrdoob/three.js/pull/23936#issuecomment-1133259680. @WestLangley do you have a preference between these, or other ideas?
Maybe we can use aliases? Like THREE.SRGB = THREE.SRGB_Rec709_D65?
I'd probably rather not ship these aliases "out of the box", unless we need them temporarily for backward-compatibility... part of the point is to get used to seeing a fully-specified color space, so that introducing color spaces for upcoming web APIs will be less disruptive. In that world there is more than one color space using the "sRGB" and "linear" transfer functions, and their differences are important.
I could be OK with exposing an API on THREE.ColorManagement to register new color spaces, at some point. In that case the user could register an alias themselves, if they prefer. There are other use cases for this, but it doesn't feel like a priority quite yet.
@WestLangley do you have a preference between these, or other ideas?
Color spaces are well-defined, so I do not see the benefit of listing the transfer function, primaries, and white point in the constant name.
My suggestion would be to continue with the following pattern:
THREE.SRGBColorSpace
THREE.LinearSRGBColorSpace // sRGB color space with a linear transfer function
// possible future
THREE.Rec2100PQColorSpace // for HDR monitor support
THREE.ACEScgColorSpace // a wider-gamut working color space...
Color spaces are well-defined, so I do not see the benefit of listing the transfer function, primaries, and white point in the constant name.
@WestLangley with Rec2100PQColorSpace
and Rec2100HLGColorSpace
we're already 2/3rds of the way there... π
But yes, there's also a real advantage to using names that closely parallel the names found in the web's own (upcoming) color APIs, if not using the web's string enum values directly. These would be good pairings with "rec2100-hlg"
and "rec2100-pq"
in the web APIs. This works for me.
Closing, see https://github.com/mrdoob/three.js/issues/23614#issuecomment-1467889921.