aframe
aframe copied to clipboard
aframevr
Nested raycasters cause false intersections on parent raycasters due to event bubbling
Description:
- A-Frame Version: 1.3
- Platform / Device: NA
- Reproducible Code Snippet or URL:
<a-scene cursor="mouse" raycaster>
<a-entity cursor></a-entity>
</a-scene>
When the cursor on the entity hits an object it does an intersection and events are fired
It then bubbles up the DOM tree and reaches the a-scene which sees an intersection event assumes that it is it's own and fires off it's own intersection events so anything listening for intersection events now thinks the a-scene cursor intersected it when it did not.
The fix is to check for event.currentTarget === event.target in onIntersection in cursor.js
If you are using multiple cursors on a single scene, you will hit other problems as well (as I did, and tried to fix in this PR, that's not been accepted yet: https://github.com/aframevr/aframe/pull/4983)
The way the cursor component uses a closure here means that there is a single global instance of rayCasterConfig.
This leads to the two cursors scribbling over each other.
The code fails to follow the advice on closures here "Be careful that these variables do not hold state because they will be shared between all instances of the component."
Perhaps a subset of my PR 4893 should be pulled out to fix this bug, at least. I had struggled to make the case that support for multiple cursors on a single scene was a mainstream use case.
it's used with setAttribute which I think will assign the properties from the argument onto the data for the raycaster rather than replacing it entirely with new object. I need to check but I would be very surprised if it did.
Yes, sorry, my comment isn't in fact relevant here, although reasons why are a little subtle...
setAttribute() doesn't work as you might expect in this case.
The values aren't copied from the origin and direction vectors - instead the raycaster directly references the supplied vec3s.
This call to setAttribute does not even trigger a call to the raycaster update() method, as the objects haven't changed (the xyz values have changed, but the objects themselves are the same).
As far as I can tell, this setAttribute line in cursor.js is entirely superfluous.
That said, the shared values don't actually cause a problem in the case where one cursor is mouse-based and one is gazed-based, because they are only used for mouse-based cursors.
So you'd need to have 2 x mouse-based cursors that map to different points in the 3D space (e.g. by being projected through 2 diferent canvases) to have a problem.
That's exactly what I saw in this example (worked around using a custom cusror.js), but it's a very specific case, and won't apply any more broadly: https://diarmidmackenzie.github.io/aframe-multi-camera/examples/multi-screen-with-cursor.html
In case it's helpful to anyone, here's a working example showing mouse-based + gaze-based cursors working independently in the same scene. https://dual-cursors.glitch.me/
You can set a breakpoint on the raycaster update() method and observe that it is not actually invoked when using the mouse.
I was a little curious why this happens.
It looks like the root cause is this code
function isObjectOrArray (value) {
return value && (value.constructor === Object || value.constructor === Array) &&
!(value instanceof window.HTMLElement);
}
this function isObjectOrArray is used to determine whether to copy or deep clone an object. It does not classify Vector3s as objects (because value.constructor is set to Vector3, not Object or Array)
This means that when A-Frame copies data to oldData, it just assigns the Vector3 objects by reference, rather than doing a deep copy.
When cursor.js updates the direction and origin values, the values in both data and oldData (which both return to the same global vector3 instances) both change, and when A-Frame compares data and oldData to determine whether it should update the component, it doesn't see any difference between them (even doing a deep comparison).
hasComponentChanged = !utils.deepEqual(this.oldData, this.data);
I don't know whether this is a bug, or intentional. It's certainly counter-intuitive, but maybe has performance benefits by avoiding creating unecessary Vector3 objects.
(sorry for hijacking your issue with this, will ponder whether I should spin this off into a separate issue).
In the example provided:
<a-scene cursor="mouse" raycaster>
<a-entity cursor></a-entity>
</a-scene>
If cursor mouse is active all the other cursors should be inactive? Any use cases for several cursors working simulatenously?
It certainly helps but as @diarmidmackenzie pointed out it doesn’t fix the whole issue for every situation which was a bit large for me to tackle in that PR so I closed it.
A good reason for allowing mouse and other cursors would be sticking cursors onto animating objects to automate cursor behavior or let other participants use cursors remotely
I think it should be the app responsibility to enable/disable cursor/raycaster you don't want. For example, cursor/raycaster on hands disabled initially, and when you enter vr you disable cursor/raycaster defined on a-scene that is used for desktop, and you enable cursor/raycaster on hands. Disabling raycaster that you don't use is best practice for perf to not do unnecessary work. So that's what I'm doing in my app. The issue I had is that I can't really disable the cursor, there is no enabled property on this component that make it inactive, unregistering the listeners, so I really need to remove the component completely currently.
I don't know what all the remaining issues are here, but I believe that fixing #5181 (which I have meaning to be raised for a while, and finally decided to get on with) would address the issues with multiple cursors that I've observed in the past & described above.
I described the issue I had in the comment https://github.com/AdaRoseCannon/aframe-htmlmesh/pull/5#issuecomment-1336139745 and that's probably what @AdaRoseCannon saw as well. I thought at first it was an event bubbling issue but reading your comments again @diarmidmackenzie you're saying the real issue is the shared config that triggers actually a new event from the parent raycaster/cursor, and it's not actually the same event. That may indeed be the case if I remember my debugging session, the event details cursorEl was different for the two mouseenter events, that couldn't make sense if it was event bubbling. I'll review again in details the issue another day with what you wrote in mind.