react-three-fiber icon indicating copy to clipboard operation
react-three-fiber copied to clipboard

RFC: `workflow` canvas prop

Open gsimone opened this issue 2 years ago • 10 comments

from discord

Don: [pedantic stuff] three.js always renders with sRGB primaries and white point (i.e. color gamut) but lighting calculations explicitly do not use the sRGB transfer function (sometimes called a gamma curve). Usually what people mean when referring to named color spaces are:

  • sRGB / THREE.sRGBEncoding: sRGB transfer function, primaries, and white point
  • Linear-sRGB / THREE.LinearEncoding: sRGB primaries and white point, but Linear transfer function For that reason flag names like linear={true} and flat={true} are probably adding some confusion... I love short property names as much as the next JavaScript programmer, but color seems to be an area where precise wording pays off. Alternative to the full technical terms would be to have an API in terms of 'what the user wants to do', like workflow={ 'lit' | 'unlit' | 'legacy' | ... } and internally set opinionated defaults for each workflow. [/pedantic stuff]

Cody: I'm not sure if people think beyond which encoding/tonemapping they're using.

gsimone avatar Jun 01 '22 12:06 gsimone

My general concern with short prop names is how they describe themselves -- linear, flat, legacy, etc. don't inform me that I'm working with color. I think this makes for a confusing API, despite the above semantical concerns. I like how Don's proposal clarifies their relation to each other in more familiar terms, although I think it's important for options to be documented as to how they would be implemented in three. For existing projects, the only reference point users have would be which encoding, tonemapping, or colorspace they're using.

CodyJasonBennett avatar Jun 01 '22 13:06 CodyJasonBennett

if we want the workflow-oriented approach, these could be some opinionated defaults:

  • lit: color inputs assumed sRGB, converted to Linear-sRGB, scene rendering in Linear-sRGB, output to sRGB with a (probably filmic) tone map. examples: most PBR or lit non-photorealistic e.g. pixar scenes.
    • ColorManagement.legacyMode = false
    • texture.encoding = sRGBEncoding (for color textures only)
    • renderer.outputEncoding = sRGBEncoding
    • renderer.toneMapping = FilmicToneMapping
  • unlit: color inputs assumed sRGB, no lighting so no need to convert anything to Linear-sRGB, skip output encoding and tone map. examples: 2D art with sprites, text, illustrated styles.
    • Some debate about semantically 'best' way to do this moving forward. The defaults below give the correct result, but for implicit and confusing reasons.
    • ColorManagement.legacyMode = true
    • texture.encoding = LinearEncoding (always)
    • renderer.outputEncoding = LinearEncoding
    • renderer.toneMapping = NoToneMapping
  • legacy: if we need to keep current behavior available for backward compatibility
    • ...

Users will reasonably want to change tone mapping and change texture encoding in certain cases. The other settings could perhaps be hidden behind the 'workflow' flag or something like it.

Probably it's also worth spelling out what more explicit versions of the API might look like, for example:

<Canvas
  inputColorSpace={'srgb'}
  workingColorSpace={'srgb-linear'}
  outputColorSpace={'srgb'}
  toneMapping={'filmic'} />

Predefined color space names above borrowed from CSS Color Module Level 4.

donmccurdy avatar Jun 01 '22 14:06 donmccurdy

Maybe having the props split up like this for advanced users:

<Canvas
  colorspace={{ input: 'srgb', working: 'srgb-linear', output: 'srgb' }}
  tonemapping={'filmic'} />

Then if I am doing a postprocessing workflow, assuming 'lit' is the default, I can just do:

<Canvas tonemapping={false} />

I'm wondering how useful wide reaching presets are here. Seems to me most users use the 'lit' preset for most of their content and then some run into texture encoding issues, and then rarely others are trying to import an entire prelit scene. In those cases I'm thinking it would be better to have a short guide for how to use the colorspace and tonemapping options to get the result they want.

@donmccurdy The part I'm curious about in your presets is texture.encoding. That seems to me the biggest magic setting, touching all my textures. How does three handle it?

krispya avatar Jun 01 '22 15:06 krispya

three.js cannot guess the right texture.encoding settings — it's a property on the Texture class and no usage context is really available there. Suggested settings are provided in the color management docs, but even here there's some variation — .map is usually an albedo / diffuse texture and would be [0,1] sRGB, but using [0,∞] Linear-sRGB OpenEXR data for .map with a MeshBasicMaterial is a totally fair (though far more advanced) thing to do. In either case three.js must specify the encoding when uploading the texture to the GPU, and WebGL handles the conversion to Linear-sRGB if required.

I'm thinking it would be better to have a short guide for how to use the colorspace and tonemapping options to get the result they want...

I'm certainly not opposed to exposing more granular options if they're clearly and precisely named. I would caution that these "short guides" can get quickly get more complicated as we try to cover lit vs. unlit workflows, and the various post-processing implementations. That complexity cannot totally be avoided — we do need more of these guides — but (maybe) presets are a way to help some users avoid the pain.

donmccurdy avatar Jun 01 '22 16:06 donmccurdy

Side note — I'm not totally confident on the best 'unlit' settings. Maybe those users are totally fine going with the 'lit' workflow. The actual performance overhead for the (possibly unnecessary) sRGB → Linear-sRGB → sRGB round trip is probably not much. And effects like blending and color grading do still apply to 2D and unlit stuff, but I have pretty limited understanding of the tradeoffs of doing those operations in sRGB or Linear-sRGB space.

donmccurdy avatar Jun 01 '22 16:06 donmccurdy

I'm certainly not opposed to exposing more granular options if they're clearly and precisely named. I would caution that these "short guides" can get quickly get more complicated as we try to cover lit vs. unlit workflows, and the various post-processing implementations.

My intuition is since these workflows are so complicated trying to cover them with presets might also cause more pain compared to having users read some information and ask questions on the Discord. On the other hand, it is nice to tell a user when doing some diagnosis to try putting flat or linear on their Canvas and see if it just solves their issue. But also this doesn't help them learn what the actual problem is if they run into an issue that can't be solved with those presets.

krispya avatar Jun 03 '22 05:06 krispya

I believe this has become redundant since the introduction of THREE.ColorManagement which has made this much more stable in R3F.

CodyJasonBennett avatar Jun 12 '23 13:06 CodyJasonBennett

three.js r157 added (early, experimental) support for wide gamut color. We have a lot more to do on that side, but I think I feel comfortable suggesting that R3F will — eventually — need some changes to take full advantage of that work:

  1. <Canvas linear /> may be confusing in this workflow. I'd suggest, instead:
<Canvas colorSpace={ LinearSRGBColorSpace | SRGBColorSpace | DisplayP3ColorSpace } />
  • ⚠️ LinearSRGBColorSpace: Equivalent to current linear option, use sparingly
  • ⚠️ LinearDisplayP3ColorSpace: Like current linear but wider gamut, use sparingly
  • ✅ SRGBColorSpace: Current default
  • ✅ DisplayP3ColorSpace: Wide-gamut display
  1. I anticipate more tone-mapping options coming to three.js, and would suggest <Canvas toneMapping={ X } /> to handle that.

Threlte ships both (1) and (2) now.

Wide-gamut color will also require setting THREE.ColorManagement.workingColorSpace = THREE.LinearDisplayP3ColorSpace, but that's probably outside of what R3F needs to think about.

Finally, textures need to be tagged with their color space correctly, e.g. texture.colorSpace = SRGBColorSpace or texture.colorSpace = DisplayP3ColorSpace. I've been unable to find a safe way to do that automatically in three.js, unfortunately.

donmccurdy avatar Oct 25 '23 17:10 donmccurdy

Definitely a fan of such configuration, and find linear and flat more confusing than not for those who are familiar with these workflows.

WRT assigning texture color space, I saw https://github.com/mrdoob/three.js/pull/27042 recently which somewhat worked around this.

CodyJasonBennett avatar Oct 26 '23 05:10 CodyJasonBennett

I support this change as well.

krispya avatar Oct 30 '23 17:10 krispya