Skip to content

Conversation

@mrxz
Copy link
Contributor

@mrxz mrxz commented Jun 20, 2025

Description:
The gltf-model component would unconditionally change the model in the gltfLoad callback. However, it's possible for the src value to have changed in the meantime. In case the second model finishes loading before the first one, the component would incorrectly end up showing the first model (once it finishes loading as well).

This PR adds a check to ensure that the src hasn't changed (similar to what is done for textures).

Changes proposed:

  • Check if src changed while loading model to avoid showing the wrong model
  • Always remove previous model, even when setting to an empty value
  • Add unit tests covering the src change and removal logic.

Copy link
Contributor

@vincentfretin vincentfretin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! And the added tests are testing indeed those cases.

@kfarr I think I remember seeing that race condition happening on 3dstreet with the model preview on hover when switching fast enough between cards when we used the same entity, and we currently remove and create a new entity because of that. That was one of the reason anyway.

@dmarcos
Copy link
Member

dmarcos commented Jun 25, 2025

Thank you!

@dmarcos dmarcos merged commit 7e0e070 into aframevr:master Jun 30, 2025
3 checks passed
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.

3 participants