three.js
three.js copied to clipboard
Lights: Add shadowLimit property to PointLightShadow and SpotLightShadow
Fixed #27290
Description
Setting the distance of PointLight/SpotLight affect the PointLightShadow/SpotLightShadow's camera.far when the distance is set to 0. If I set the distance to 5 and then later to 0, the camera.far of the Shadow is still set to 5. This change introduces a shadowLimit property to the Shadow class that will be used to set the value of camera.far when the corresponding light object is 0
📦 Bundle size
Full ESM build, minified and gzipped.
Filesize dev |
Filesize PR | Diff |
|---|---|---|
| 669 kB (166.1 kB) | 669.1 kB (166.1 kB) | +64 B |
🌳 Bundle size after tree-shaking
Minimal build including a renderer, camera, empty scene, and dependencies.
Filesize dev |
Filesize PR | Diff |
|---|---|---|
| 449.9 kB (109 kB) | 449.9 kB (109 kB) | +0 B |
I hesitated a bit with approving the PR since we have always stated users can directly modify the shadow camera's frustum. However, when we started to evaluate the distance (a light property) in the light shadow classes, things started to get confusing. Given the current state, I think this solution is okay.
The alternative would be to completely remove the distance evaluation from the light shadow classes. So devs always have to configure the far property of the shadow camera and distance only affects the light, not shadows anymore. I like the resulting simplicity of this approach but I'm unsure about the consequences. Using point and spot lights with their (physically correct) default distance value won't be affected by this change.
Understandable. I'll leave that decision to you and the rest of the maintainers of this project. Totally fine with scrapping this PR if we end up going down another route
see https://github.com/mrdoob/three.js/pull/27722#issuecomment-1953915716
I can document the new property when we agree on this design. After trying out different approaches, it's the best solution for https://github.com/mrdoob/three.js/issues/27290 , imo.
Hmm... I'll look into this one tomorrow.
I am inclined to think this approach gets away from the original intent of making the lights "just work".
See the discussion here.
@WestLangley What alternative approach would you suggest? Something like https://github.com/mrdoob/three.js/issues/27290#issuecomment-1839697678?