Skip to content
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

Don't call raycaster setDirty on each mouse move unnecessarily #5542

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

vincentfretin
Copy link
Contributor

Description:

Before this PR, setAttribute('raycaster', {origin, direction}) is called by the cursor component in onMouseMove triggering raycaster update method and this.setDirty() so actually calling raycaster.refreshObjects() on every tick with the default raycaster refresh interval that is 0, or every 100ms minimum if you set raycaster interval to 100 for example, even if nothing changed in the scene.

Some notes:

  • cursor this.el.setAttribute('raycaster', rayCasterConfig) was introduced in 2017-07-18 22:28 in a4b91bc
  • setDirty was introduced in 2017-09-19 20:26 in a463d64 it was calling refreshObjects directly before that.

Changes proposed:

  • In raycaster update, call this.setDirty() only if objects property changed

…and not on every mouse move (setAttribute('raycaster', {origin, direction}) is called by the cursor component in onMouseMove triggering refreshObjects on each tick with the default raycaster refresh interval that is 0)
@vincentfretin
Copy link
Contributor Author

That issue is actually new in aframe 1.6.0. It wasn't an issue in 1.5.0.
So the this.el.setAttribute('raycaster', rayCasterConfig) in cursor component is now triggering the raycaster update function in 1.6.0. It wasn't the case in 1.5.0 @mrxz

@vincentfretin
Copy link
Contributor Author

You can test on the http://localhost:8080/showcase/ui/ example, put a console.log in raycaster refreshObjects function.

@mrxz
Copy link
Contributor

mrxz commented Jun 15, 2024

Prior to 1.6.0 the handling of instances of classes like Vector3 when updating components had bugs (#3681, #5181). Passing in the same reference in subsequent .setAttribute calls would not trigger any update, as no changes would be detected, even if the values were actually changed.

Looking at the cursor, it does call .setAttribute with newly computed values, with the expectation that the raycaster gets updated. So update now being called with 1.6.0 is IMO the correct behaviour. That the raycaster always marks itself dirty regardless of what changed, seem to be a different bug that now surfaces, for which this PR seems like a good fix.

@dmarcos
Copy link
Member

dmarcos commented Jun 18, 2024

Thanks so much!

@dmarcos dmarcos merged commit 998e4fc into aframevr:master Jun 18, 2024
1 check passed
@vincentfretin vincentfretin deleted the raycaster-set-dirty branch June 19, 2024 08:06
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