Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PointLight/SpotLight: Change default decay to 2 #23897

Merged
merged 5 commits into from
Nov 12, 2022
Merged

PointLight/SpotLight: Change default decay to 2 #23897

merged 5 commits into from
Nov 12, 2022

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Apr 14, 2022

Related issue: #23614 (comment)

Description

If we aim to make the lighting physically correct we have to switch the default decay to 2.
This PR is an attempt to do the change and see if there are any side effects.

/cc @bhouston @WestLangley @donmccurdy @Mugen87 @sunag

@mrdoob mrdoob added this to the r140 milestone Apr 14, 2022
@donmccurdy
Copy link
Collaborator

donmccurdy commented Apr 15, 2022

tl;dr — this change looks good to me!


Summarizing legacy vs. physical attenuation:

  • legacy: light attenuates from 100% to 0% linearly (decay=1) or by inverse square (decay=2) with the ratio of object distance / light.distance, reaching 0% at light.distance meters. Because the range of the light is specified, and attenuation is distributed over that entire range, attenuation is not realistically based on physical distance. If either decay=0 or distance=0, light does not attenuate. Distance is 0 by default, so light does not attenuate by default.
  • physical: light attenuates from 100% to 0% linearly (decay=1) or by inverse square law (decay=2) with the object distance, never reaching exactly 0% unless light.distance is specified. When light.distance is specified, it smoothly but quickly diminishes intensity to zero at that distance1, without affecting attenuation over the rest of the light's range. If decay=0, light does not attenuate.

1 This is not physically correct, but is a useful tool to limit the otherwise infinite range of the light to something manageable for rendering performance in a scene with many lights. Unlike in the legacy mode, light.distance in physical mode has very minimal effect on the physical-correctness of attenuation within the range of the light.


In the legacy mode, attenuation is disabled by default and so the change to from decay=1 to decay=2 does not affect things much. Even with a distance, the light still has the same range, but at 50% of that range the light will now be at 25% strength rather than 50% strength.

In physical mode, attenuation is enabled by default, and the change in decay will more directly affect the perceived range of the light. This is not a dramatic difference but it is visible.

Given that most users are probably still using the legacy mode, this seems like as good a time as any to make the change. I've created a spreadsheet, below, to illustrate the changes. I approximated the quadratic dropoff at light.distance in physical mode as a hard cutoff for simplicity.

light attenuation

Source: Punctual light attenuation in three.js

NOTE: I've omitted it from the table, but increasing intensity by a factor of Math.PI is generally useful when switching from legacy to physical mode for the first time.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Apr 15, 2022

One other idea. This is obviously not physically realistic, but — if our ultimate goal is to delete the legacy mode entirely, the path of least migration resistance would be to reduce the default decay to 0, instead of increasing it to 2. 😐

@mrdoob mrdoob modified the milestones: r140, r141 Apr 30, 2022
@mrdoob mrdoob modified the milestones: r141, r142 May 26, 2022
@mrdoob mrdoob modified the milestones: r142, r143 Jun 29, 2022
@mrdoob mrdoob modified the milestones: r143, r144 Jul 28, 2022
@mrdoob mrdoob modified the milestones: r144, r145 Aug 31, 2022
@donmccurdy
Copy link
Collaborator

Let me retract my last comment. I'd prefer to see this PR merged and decay=2 as the default. 👍

@bhouston
Copy link
Contributor

If one was making three.js now I would definitely have decay always default to 2 and enable physical lighting mode. Over at Threekit, this is what we do. It is only because of legacy that this isn't the default in Three.js in my opinion.

@WestLangley
Copy link
Collaborator

In physical lighting mode, the decay should be hardwired to 2, otherwise it is not physically-correct.

How to handle artist-friendly mode (the current default), is up for debate.

@bhouston
Copy link
Contributor

The original "physical lighting mode" didn't actually force people to be physical correct, rather it just changed the defaults and equations to be physically correct unless you changed things no? I think we just want the default to be physically correct while still allowing the user to move away to more stylized choices if they really want to. Thus I advocate changing all default behaviour to be physically correct and removing that mode.

@mrdoob mrdoob modified the milestones: r145, r146 Sep 29, 2022
@donmccurdy
Copy link
Collaborator

I like the approach @bhouston describes.

Setting decay=2 as the default is a good change that can be made now, with or without further changes.

If/when we want to simplify further, we'll set physicallyCorrectLights=true as the default, and later remove the property entirely.

After that, if users want lights that decay in a non-realistic way, they can do so by increasing or decreasing .decay, with the understanding that .decay=2 (default) is the physically-correct value. I hope that's a reasonable accommodation that allows some flexibility to deal with artistic goals, performance constraints, etc.

@mrdoob mrdoob modified the milestones: r146, r147 Oct 27, 2022
@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 9, 2022

Although this PR will affect user code, the migration task is simple. Users can restore the lighting by setting decay back to 1.

I vote to merge the PR for r147. I'll update docs, migration guide and check the examples. If some of them are too dark after this change, we can update the light's intensity, color or decay.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 12, 2022

After chatting with @mrdoob this will land in r147 🎉 .

@Mugen87 Mugen87 merged commit df4a3a9 into dev Nov 12, 2022
This was referenced Nov 12, 2022
@mrdoob mrdoob deleted the decay branch November 15, 2022 05:53
@0b5vr 0b5vr mentioned this pull request Dec 7, 2022
12 tasks
0b5vr added a commit to three-types/three-ts-types that referenced this pull request Dec 19, 2022
Repository owner deleted a comment from olyphiri Dec 23, 2022
Repository owner deleted a comment from olyphiri Dec 23, 2022
Repository owner deleted a comment from olyphiri Dec 23, 2022
Repository owner deleted a comment from olyphiri Dec 23, 2022
Repository owner deleted a comment from olyphiri Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants