WebGPURenderer: Avoid hardcoding color spaces. #31519
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
| * The current output color space of the renderer. When a render target is set, | ||
| * the output color space is always `LinearSRGBColorSpace`. | ||
| * The current color space of the renderer. When not producing screen output, | ||
| * the color space is always the working color space. | ||
| * | ||
| * @type {string} | ||
| */ | ||
| get currentColorSpace() { |
There was a problem hiding this comment.
Both the “current color space” and “current output color space” phrasing feel unclear to me, since we're describing output but not necessarily .outputColorSpace — maybe this would be clearer named something like targetColorSpace or dstColorSpace? For example:
/**
* Target color space of the current render stage. Target is the working color
* space when a render target is assigned, and the output color space otherwise.
*/
get targetColorSpace() { ... }Importantly, the only possible results are the working and output color spaces. I didn't mean to imply "render target" with the "target" keyword here, so maybe that's confusing too. 😕
(No changes needed in this PR – just a drive-by thought!)
There was a problem hiding this comment.
/cc @WestLangley just FYI!
There was a problem hiding this comment.
The properties should describe what color space and tone mapping the renderer is going to apply during the render pass. It is not necessarily what is defined in outputColorSpace and toneMapping. So maybe the term "target" is more clear?
Related issue: #29573 (comment)
Description
The PR makes sure that
LinearSRGBColorSpaceisn't hardcoded as the working color space inWebGPURenderer. As pointed out in #29573 (comment), we should useColorManagement.workingColorSpaceinstead.