Reintroduce light intensity scale factor removed in THREE r165 after WebGLRenderer.useLegacyLights deprecation (fix #5556, #5546)

[dmarcos]

Is this good? @vincentfretin @mrxz

[dmarcos]

Colors of the hello world look same as before after applying the intensity scale factor

[vincentfretin]

Why did you change decay?

[dmarcos]

This is @mrxz suggestion in #5546 (comment)

[vincentfretin]

It shouldn't change the intensity for scene that used renderer="physicallyCorrectLights: true" already.

[dmarcos]

Not sure I understand the comment. Without the factor scene is dark. What should we do?

[vincentfretin]

We have aframe experiences with renderer="physicallyCorrectLights:true" and light intensity that was carefully set, those scenes are not dark, we don't want the intensity to change for those.

[vincentfretin]

We could keep the physicallyCorrectLights property on renderer just to add a condition in the light component. But we should remove the lines

aframe/src/systems/renderer.js

Lines 38 to 40 in 2aca98c

	if (!data.physicallyCorrectLights) {
	renderer.useLegacyLights = !data.physicallyCorrectLights;
	}

and modify the tests.

#5546 for context.

[dmarcos]

We could keep the physicallyCorrectLights property on renderer just to add a condition in the light component. But we should remove the lines

aframe/src/systems/renderer.js

Lines 38 to 40 in 2aca98c

	if (!data.physicallyCorrectLights) {
	renderer.useLegacyLights = !data.physicallyCorrectLights;
	}

and modify the tests.
#5546 for context.

I think should be fine to remove the physicallyCorrectLights property in the renderer?

[mrxz]

As @vincentfretin pointed out, there are already scenes that have physicallyCorrectLights enabled and should not have their intensities scaled. If we get rid of the property on renderer, then there won't be a way to distinguish the two.

Besides that, applying the scaling only in the light component will miss lights created in other ways. Components might directly create lights or a glTF file might include them. Even the reflection component manually sets light intensities, which would need to be updated as well (reflection.js#L20).

Instead of this PR, I think we should either:

1. Follow Three.js and deprecate the "legacy lights" entirely. The default light setup and things like the environment-component can be updated. Document what users need to do to upgrade their scenes.
2. Retain the old behaviour by re-introducing useLegacyLights in super-three. This ensures that everything behaves like before, instead of only the light component.

Personally not a fan of diverging from Three.js, but forcing upgrading users to make changes with no way to opt-out is also far from ideal.

[dmarcos]

Leaning towards deprecation

[dmarcos]

Going back and forth with this:

1. Keeping old behavior is a big depart from mainstream THREE and code is complex to maintain forward.
2. Adapting to new model requires modifying examples and 3rd party components and corresponding examples to recreate same look. And code ends up with random ugly numbers that are hard to understand. Dev experience suffers and unclear how the change benefits most of A-Frame users.

Not saying this change in THREE won't be worth long term but short tem dev experience suffers.

[dmarcos]

I multiplied the light defaults and values in examples by 3.14

8813d3d

Also updated the environment component defaults and published a new version: 1.4.0

Any other outstanding 3rd party components that we should adjust?

[vincentfretin]

@dmarcos can you please also remove the physicallyCorrectLights option from the renderer system?

[dmarcos]

@vincentfretin done. thanks 🫡