WebXR: Added model caching to XRHandModelFactory.#33252
Conversation
| loader.load( `${handedness}.glb`, gltf => { | ||
|
|
||
| const object = gltf.scene.children[ 0 ]; | ||
| const object = gltf.scene.children[ 0 ].clone(); |
There was a problem hiding this comment.
I don't think this code works. The glb is supposed to hold a skinned mesh and those can't be cloned via clone(). You must use SkeletonUtils.clone().
There was a problem hiding this comment.
You know...even if you are using AI to code things you are supposed to test what you are doing. This bug is quite obvious for developers who actually understand three.js.
There was a problem hiding this comment.
I'm sorry this is my first issue to work on so to have a review I have used AI , but I have tested this on my own I'll make sure that this won't repeat again, sorry for the inconvenience
| * @param {?Function} [onLoad=null] - A callback that is executed when a controller model has been loaded. | ||
| */ | ||
| constructor( handModel, controller, path, handedness, loader = null, onLoad = null ) { | ||
| constructor( handModel, controller, path, handedness, loader = null, onLoad = null, customCache = null ) { |
There was a problem hiding this comment.
Please document the new parameter.
|
@mrdoob How about adding an AI/LLM Usage Guideline to the Contribution guide? Gemini comes up with something like this: AI/LLM Usage GuidelinesYou are responsible for the code you submit.While AI tools can provide powerful assistance, we strictly discourage "copy-paste" contributions. PRs are frequently rejected because the submitter did not understand the code the AI generated.
If a PR is found to be low-quality or "hallucinated" code, it will be closed immediately. Please value our time by thoroughly reviewing and testing your contributions. Then we have something we can link to instead of explaining these things over and over again. |
|
From my perspective a suggestion that if you know the code and want to review from ai ,I want to have some guidelines so it will help contributor's like me to review the code easily and get the suggestions. |
|
The first thing you should keep in mind is: Don't contribute to projects you are not familiar with. At the end, you are causing more troubles with your "contribution" than producing a benefit. The real contributors and maintainers need to spend a lot of time to review your PR and explain the bugs you introduce. At the end, we are faster when we implement issues like #33242 ourselves. |
Completely agreed 💯 |
Clean up.
|
I'll fix the open issues and merge. |
Fixed #33242
Description
Implemented model caching for XRHandMeshModel to improve performance and prevent redundant network requests.
Previously, XRHandMeshModel lacked the asset caching found in XRControllerModelFactory, causing the hand model (
${handedness}.glb) to be re-fetched every time a hand connected or re-entered view. This PR introduces a shared_assetCachein XRHandModelFactory and updates XRHandMeshModel to:.clone()on cached models to ensure independent instances for multiple hand models.These changes unify the WebXR model factories and optimize hand-tracking experiences by reducing fetch overhead.