[raycaster] Use MutationObserver and object3dset/remove to refresh. #3070
Conversation
|
Thanks I'll look into it post 0.7.0 release |
|
Added /cc @machenmusik |
ngokevin
left a comment
ngokevin
left a comment
There was a problem hiding this comment.
The approach looks good to me to continue with tests, will want to mention in perf best practices section to watch out for this potentially
src/components/raycaster.js
Outdated
| this.el.sceneEl.addEventListener('child-attached', this.refreshOnceChildLoaded); | ||
| this.el.sceneEl.addEventListener('child-detached', this.refreshObjects); | ||
| if (!this.data.watch) { return; } | ||
| this.observer.observe(this.el.sceneEl, { |
There was a problem hiding this comment.
Could store/reuse this object as a constant
src/components/raycaster.js
Outdated
| this.updateOriginDirection(); | ||
| this.refreshObjects = bind(this.refreshObjects, this); | ||
| this.refreshOnceChildLoaded = bind(this.refreshOnceChildLoaded, this); | ||
| this.setDirty = bind(this.setDirty, this); |
There was a problem hiding this comment.
I re-ran some jsperf stuff, we can just start using vanilla bind. I don't think there's perf difference
src/components/raycaster.js
Outdated
| showLine: {default: false}, | ||
| useWorldCoordinates: {default: false} | ||
| useWorldCoordinates: {default: false}, | ||
| watch: {default: true} |
src/components/raycaster.js
Outdated
| // Defines selectors that should be 'safe' for the MutationObserver used to | ||
| // refresh the whitelist. Less common selectors, like [position=0 2 0], cannot | ||
| // be detected here. | ||
| var SAFE_SELECTOR_RE = /^[\w\s-.[\]#]+$/; |
There was a problem hiding this comment.
Can explain more what this selector matches, and name maybe like OBSERVER_SELECTOR_RE
src/components/raycaster.js
Outdated
| } | ||
|
|
||
| if (data.watch !== oldData.watch && el.isPlaying) { | ||
| data.watch ? this.play() : this.pause(); |
There was a problem hiding this comment.
Should not call handlers from other handlers, can extract to separate methods
|
Updated based on feedback, and added a couple tests. |
4 participants
This is a proposed fix for #2980 and #3067. It's not possible to perfectly detect when the whitelist may have changed — direct calls to
el.object3D.add(mesh)will be missed, for example — but this covers all of the A-Frame API surface I think.Changes:
object3dsetandobject3dremove. Unlike child-added/removed, these are only triggered after the entity has loaded, so no need for a delay.raycaster.objectsselector is exotic, like[position=0 2 0], which will be missed by our MutationObserver.If this direction looks OK, I'll add tests. It's a bigger change than we should include for 0.7.0, and can wait for 0.7.1 or 0.8.0.