ColorManagement: renamed .legacyMode to .enabled

[WestLangley]

The current API is as follows:

THREE.ColorManagement.legacyMode = false; // false means enabled

This PR changes the API to:

THREE.ColorManagement.enabled = true;

[donmccurdy]

@gkjohnson and I had discussed this as well...

The semantic issue is that color management (as a concept, not THREE.ColorManagement) includes more than just handling input colors, which is all the flag currently does. "Disabling color management" does not disable renderer output encoding, for example.

Maybe that is not confusing though? If you think of it not as a toggle on all color management, but as a toggle on the things that THREE.ColorManagement does.

I am happy with the change. 👍

[donmccurdy]

We should accept .legacyMode=false as a fallback for the next ?? releases though. This option is enabled by default in R3F, and would otherwise cause some real churn.

/cc @drcmda FYI

[CodyJasonBennett]

No worries. We can work around it fine there.

[gkjohnson]

The semantic issue is that color management (as a concept, not THREE.ColorManagement) includes more than just handling input colors, which is all the flag currently does. "Disabling color management" does not disable renderer output encoding, for example.

If everyone wants to change this I won't strongly fight it but I think it's worth making sure the whole picture is known. Currently setting "legacyMode = true" (what is being suggested to be changed to "ColorManagement.enabled = false") is not truly unmanaged color - it has just been mistakenly referred to as such. So to that end I would consider it incorrect to change the current nomenclature to "enabled".

At some point what we're calling "legacy mode" should be removed from the project. And it should be decided if we really plan to support actual unmanaged color workflows which would basically just mean ignoring all "encoding" fields throughout the renderer. I still don't see a lot of value in that. And with the recent change to use SRGB conversion on texture upload with the WebGL API the question of performance implication is less relevant, I think.

[drcmda]

the most important for us is that color management is switched on. like @donmccurdy mentioned, we have this on for a longer while and it works great! but since it's opt in atm there are unavoidable bugs that are only fixed if it's the default.

the amount of churn is still a burden, since we have to take four three different three breaking changes into account:

• old three w/o color management, needs .convertSRGBToLinear hacks
• new three with CM.legacy = false
• newer three with CM.legacyMode = false
• even never three with CM.enabled = true

but seeing the = true, and in hopes this won't change again, perhaps it's worth it. though i wish it had semver, what a struggle.

---

if in the end, for whatever reason, it is not changed to "true" then please let us not change the name, because that would create churn for nothing.

[LeviPesin]

new three with CM.legacy = false

What version is that?

[donmccurdy]

we have this on for a longer while and it works great! but since it's opt in atm there are unavoidable bugs that are only fixed if it's the default.

To clarify for others, "unavoidable bugs" refers to the fact that R3F cannot always execute CM.legacyMode = false before its users have begun constructor colors. Consider:

// root.js
import { ... } from './a.js';
import { ... } from './b.js';

// a.js
import { Color } from 'three';
const DEFAULT_COLOR = new THREE.Color(0x404040); // CM.legacyMode is still true

// b.js
import { ... } from '@react-three/fiber'; // sets CM.legacyMode to false

So the CM.legacyMode property works for a user who sets it at the root of their application, but a library like R3F can only "opt in" on behalf of its users with these limitations, and sometimes users get bitten. Here's an example shared in three.js Slack earlier today:

https://codesandbox.io/s/red-grass-60xi6p

---

@drcmda I think that @WestLangley is hoping to improve the API, this does not have to be a breaking change for R3F. As proposed above, we could continue accepting legacyMode=false as an undocumented alias until R3F (eventually) increases its minimum three.js version to include the update.

None of our official examples use CM.legacyMode = false today. I suspect it will be a hard sell to turn it on by default until that happens, and we have a chance to identify problems. Many of the changes we'd like to make to color-related APIs are interlinked (see #23614) and to the extent that we can unlink them — e.g. changing a property name by itself — without adding breaking changes, this will go faster.

---

At some point what we're calling "legacy mode" should be removed from the project.

By "legacy mode" we are talking about ignoring the colorSpace arguments to THREE.Color constructors and setters, correct? So that legacyMode=false means hexadecimal and CSS-style inputs are understood to be sRGB rather than Linear-sRGB?

I'd be glad to see legacyMode=false (or similar) enabled by default, but I worry that I may have stepped on toes getting THREE.ColorManagement added in the first place. So I am hoping to wait and let others try it out and guide what happens to it next. 😇

And it should be decided if we really plan to support actual unmanaged color workflows which would basically just mean ignoring all "encoding" fields throughout the renderer. I still don't see a lot of value in that.

Nope, let's not do that! 🙅🏻‍♂️

I'm still unsure of what to recommended for unlit work, but ignoring .encoding fields does not feel like the right solution.

[WestLangley]

OK. Waited until after the release to add the milestone. :-)

[LeviPesin]

I think it is better to keep the name as it is? Per @gkjohnson's reasons.

[WestLangley]

Closing, then...

[donmccurdy]

Would you feel more comfortable using ColorManagement.enabled = true in examples, if it were renamed as proposed here? I still feel that THREE.ColorManagement is a good direction, e.g. as it simplifies things like #24362 (comment). I do not care very much what it is named, and would be happy to support any change you prefer.

[WestLangley]

Yes, I would prefer merging this PR.

[donmccurdy]

Ok! Continued in #24940.