-
-
Notifications
You must be signed in to change notification settings - Fork 35.9k
LightsNode: Fix castShadow
regression.
#31106
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
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
return hashArray( hashData ); | ||
const cacheKey = hashArray( _hashData ); | ||
|
||
_hashData.length = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have any source that .length=0
is better than other alternatives?
I didn't do an extensive beachmark, but I had done some tests between creating a new array and using .length=0
and creating a new array worked better. It seems that both force the GC, using null values after use was what worked best for small arrays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to find the original issue/PR where we discussed this but without success so far. It's many years ago but at that time, setting a zero length was the fastest way to clear an array. And it does no create a new array object like the previous approach which invalidates the previous object.
I'm surprised to hear that .length=0
triggers the GC. The array itself is just empty and the reference to it is not set to null
or cleared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can revisit the array management at any point again. Merging for now so the toggling issue is fixed on dev.
* LightsNode: Fix `castShadow` regression. * Update AnalyticLightNode.js
* LightsNode: Fix `castShadow` regression. * Update AnalyticLightNode.js
Related issue: https://discourse.threejs.org/t/webgpu-shadow-tests-and-findings/82561
Description
Toggling
light.castShadow
withWebGPURenderer
has currently no effect.The bit that checks the light state per frame is
LightsNode.customCacheKey()
. In this method we have to implement all checks that detect state changes of lights.