three.js
three.js copied to clipboard
SpotLightShadow.cameraAutoUpdate / Light.shadowAutoUpdate
Preventing the Autoupdating of the SpotLight.shadow.camera was previously performed through the hack of overwriting the SpotLightShadow with a LightShadow.
Instead, this PR proposes a new property ~SpotLightShadow.cameraAutoUpdate~Light.shadowAutoUpdate that makes optional the fitting of the shadow camera frustum to the square cone defined by light.angle/light.distance.
(this PR is the item 2 of #14658, extracted here for an easier review)
== Edit ==
Instead of overwriting the shadow, an alternative hack to get the same result would be to set this.shadow.isSpotLightShadow = false. As that was the sole use of isSpotLightShadow, this PR is essentially moving and renaming Light.shadow.isSpotLightShadow to Light.shadowAutoUpdate.
I definitely support this.
I also support removal of the need for this pattern:
light.shadow = new THREE.LightShadow( ... );
Forcing the shadow frustum to auto-size to the cone can have the side-effect of causing pixelated shadows.
2 more commits :
- I found 2 more uses of the
light.shadow = new THREE.LightShadow( ... );pattern in the examples. - I moved the
cameraAutoUpdatecheck to the calling site, as suggested in the review.
I am not sure about 2da8234 though, at it breaks http://localhost:8000/examples/webgl_lights_spotlight.html due to https://github.com/mrdoob/three.js/blob/2cbd28958be86cb5f34ee5532a3ee18a23467b94/examples/webgl_lights_spotlight.html#L142 which is now ungarded (it is required to have the spotlight and camera helpers in sync)
@WestLangley what do you think of this new commit 64b191c ?
I moved SpotLightShadow.cameraAutoUpdate to Light.shadowAutoUpdate, updated the docs, and added the (conditional) light shadow update to the SpotLight helper. I also remove the now unused isSpotLightShadow.
Actually, I think some others need to chime in regarding both the feature and the implementation details.
You are not "auto-updating" the shadow camera frustum, you are "auto-sizing" it. At least that is how I see it.
The naming shadowAutoUpdate refers to what it does for all lights (it guards a call to shadow.update, similarly to how matrixAutoUpdate works for Object3D.matrix) rather than the previous naming cameraAutoUpdate or autoSizeShadowFrustum which refer to what it does for Spotlights only.
It is thus more generic and makes SpotLightShadow.isSpotLightShadow unnecessary. Furthermore, it can be used to add custom shadow update behavior for any Light.
Up !
There is no new functionality here. The goal is to provide a better API for the hack of overwriting the SpotLightShadow with a LightShadow to prevent the lightshadow update.
The alternatives are :
A- renaming LightShadow.isSpotLightShadow to LightShadow.autoUpdate
B- moving LightShadow.isSpotLightShadow to Light.shadowAutoUpdate
C- use a more specific naming like autoSizeShadowFrustum or cameraAutoUpdate, but that opens the door for users to use the LightShadow.update function as a generic custom callback, guarded by a flag with a specific naming.
(Complementarily the autoUpdate approach, a needsUpdate approach could be implemented as well for a 1-time updating dirty flag.)
This PR implements B without the needsUpdate approach. What API would you prefer @WestLangley @mrdoob ?