RoomEnvironment: Set emissiveIntensity instead

[WestLangley]

material.color represents the diffuse reflectance of the material. It is a dimensionless quantity in [ 0, 1 $]^3$.

It does not make sense to set material.color to a value outside that range.

This is particularly true when mapping between sRGB and linear-sRGB color space, as the transfer function is only defined on [ 0, 1 ].

Instead, set the emissive property of MeshLambertMaterial, and assign the desired intensity to material.emissiveIntensity.

emissive is dimensionless; emissiveIntensity represents luminance.

By luck, as used in RoomEnvironment, the result is the same, but the current implementation is improper.

[Mugen87]

When thinking about this: I understand at first glance material.color.setScalar( intensity ); looks odd however MeshBasicMaterial.color is essentially MeshBasicMaterial.emissive.

three.js/src/materials/MeshBasicMaterial.js

Line 45 in c5f48e4

this.color = new Color( 0xffffff ); // emissive

Since there are no lights in the scene I would wonder why the original developer has chosen MeshLambertMaterial. TBH, this looks more unexpected than scaling MeshBasicMaterial.color.

[Mugen87]

On the other hand all of this is somewhat subjective. If no other developer object, I'm okay with merging the PR as it is.

[WestLangley]

TBH, I would not characterize this as "somewhat subjective".

Color takes values in [ 0, 1 ]. It represents the diffuse reflectance of the material -- the fraction of incident light that is reflected diffusely. As a fraction, it is unit-less.

It is improper to set it to a value greater than 1.

For self-illuminating materials, we have emissive, a color, and emissiveIntensity, a measure of luminance.

We could add those two properties to MeshBasicMaterial, but I don't think doing so would be worth it.

Instead, MeshLambertMaterial can properly be set to represent an emissive-only material. That is what is done in this PR. Would adding an inline comment like // emissive only be helpful?

[Mugen87]

Would adding an inline comment like // emissive only be helpful?

Maybe the number of this PR so the discussion can easily be found?

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

	Before	After	Diff
WebGL	337.69 78.76	337.69 78.76	+0 B +0 B
WebGPU	557.29 154.31	557.29 154.31	+0 B +0 B
WebGPU Nodes	556.21 154.09	556.21 154.09	+0 B +0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

	Before	After	Diff
WebGL	468.91 113.45	468.91 113.45	+0 B +0 B
WebGPU	632.91 171.31	632.91 171.31	+0 B +0 B
WebGPU Nodes	588.05 160.67	588.05 160.67	+0 B +0 B

[donmccurdy]

We could add those two properties to MeshBasicMaterial, but I don't think doing so would be worth it.

I'd prefer to avoid this. Probably >98% of users choose MeshBasicMaterial because they want an unlit or shadeless material; that MeshBasicMaterial happens to also support AO and lightmaps is (IMHO) a conceptual inconsistency kept for backward-compatibility and performance. Perhaps preferably it would be split in two, or lighting could be disabled on lit material types, but I wouldn't add more shading behaviors to MeshBasicMaterial as things are now.

Regardless, the changes look good to me. It would also be OK to use MeshBasicMaterial, but to include an inline comment stating that we intend an emissive model but are emulating the effect with MeshBasicMaterial 'for performance reasons'.