supermedium / aframe-react

:atom: Build virtual reality experiences with A-Frame and React.
https://ngokevin.github.io/aframe-react-boilerplate/
MIT License
1.42k stars 151 forks source link

Raycaster events not triggered on second render #89

Open RSpace opened 7 years ago

RSpace commented 7 years ago

I just updated my A-Frame app's aframe-react version from 3.5.0 to 4.2.4. I believe I have made all the changes required from the upgrade (events, vector properties, _ref), although the lack of a changelog makes it hard to say exactly.

I have everything working as it did before I updated aframe-react, except for one thing: I have a menu that can be toggled on and off. The menu is initially visible, and initially raycaster events such as click and mouseover work fine. However, when I toggle the menu off and on again the raycaster events are no longer triggered in 4.2.4. If I roll back to 3.5.0 - all other code and dependencies being equal - it works again.

I don't toggle the visibility of the menu being setting the visible property. Instead I pass a prop to the React component rendering the menu telling it whether it should render or not. If the prop is true, the component returns an <Entity>, if it is false, it returns null. This setup is due to a number factors, related to how I use Redux to manage state and use of animations.

For performance reasons I have the configured the raycaster to only intersect with specific elements:

         <Entity
            primitive="a-cursor"
            raycaster='objects: .interactable'
          />

If I remove raycaster='objects: .interactable', the menu works every time, but I get worse performance.

I have reproduced the issue here: https://www.webpackbin.com/bins/-KkkMiVwnvG9MVsuq0us

The box toggles a sphere (a stand-in for the menu in my actual code). The first time the sphere is shown, it changes color when clicked. Once it has been hidden and is shown again, it no longer changes color.

From debugging, it seems like the Three.js raycaster simply doesn't match the sphere on subsequent renders. I just can't figure out how any of changes made in aframe-react affects this. It doesn't seem to be related to the changes event system, since the problem is the same if I manually attach event handlers to the element. Rather, it seems like the intersected elements are somehow cached, or like the className is not properly included on subsequent renders.

But I have nothing solid, just this example reproducing the issue above. Any help would be appreciated.

ngokevin commented 7 years ago

Doing <Entity id="sphere" visible={this.props.show}/> is better than detaching/attaching the entity. That makes it work and saves some performance.

I'll check why the raycaster objects aren't refreshing.

RSpace commented 7 years ago

Thanks for checking, @ngokevin

In my example it is simple to avoid detaching/attaching entities and instead just toggle the visibility. However, my actual Aframe/React app is a lot more complex, and since React is built to conditionally render or not render components based on state, it would be very hard to apply this pattern all the way through the app.

ip commented 7 years ago

I think I found the problem, will send a pull request in a moment

RSpace commented 7 years ago

I just noticed that the raycaster has a refreshObjects method. Should this be called whenever React re-renders?

ngokevin commented 7 years ago

The raycaster should refresh itself when entities are attached/detached from the scene. But calling it manually is a workaround.

ngokevin commented 7 years ago

Perhaps try @ip's PR to see if it fixes.

RSpace commented 7 years ago

It does not fix it for me, unfortunately.

ip commented 7 years ago

I've just discovered this feature of limiting raycast by specific objects, and I experience exactly the same behavior. I have a menu with a list of items, and when I click an item, the list is changed. The behavior is that the only clickable items are first N items, where N is a number of items from previous render. So yes, seems like some kind of cache related issue.

ip commented 7 years ago

@RSpace does the usage of this feature impove the FPS noticeably for your case?

RSpace commented 7 years ago

@ip I'm not sure. I am currently tracking down performance issues that might be related to having to remove the limitation on the raycasting, but it's too early to say whether that's the culprit.

RSpace commented 7 years ago

Update: This appears not to be an issue with aframe-react, but with aframe itself.

With 0.5.0, scoped raycasting using objects works and there I experience no performance problem.

With newest aframe master, scoped raycasting using objects does not work, and I experience performance issues in VR whenever I use raycasting with input controllers.

However, I noticed @ngokevin is working on https://github.com/aframevr/aframe/pull/2678 so I tried pulling aframe from https://github.com/ngokevin/aframe/tree/lasercontrols. I switched my input controller setup from:

<a-entity auto-detect-controllers='hand: left' controller-cursor='' />
<a-entity auto-detect-controllers='hand: right' controller-cursor='' />

(I was using aframe-auto-detect-controllers-component and aframe-controller-cursor-component) to:

<a-entity id="leftHand" laser-controls="hand: left" raycaster="objects: .interactable" />
<a-entity id="rightHand" laser-controls="hand: right" raycaster="objects: .interactable" />

and with that I have functional scoped raycasting, improved laser controls and no performance issues.

ip commented 7 years ago

I researched this further and found the problem in aframe-react with child-attached and child-detached events. They're dispatched while (real) DOM is in the process of update. This breaks the contract for existing code - querying elements inside child-attached event handler.

There are two existing tests related to these events and they were broken (the usage of setTimeout()). I adjusted them to listen for child-attached/detached events and they demonstrate the issue now: https://github.com/ip/aframe-react/commit/1bbe98fa7bec213046ce638f648900d94dbebaef

ip commented 7 years ago

The solution that worked for me was to queue the events of these types and emit them later using setTimeout(). At first I tried to monkey-patch the ANode.prototype but it seems to be immutable, perhaps because it's a custom HTML element. So I edited the A-Frame itself. It broke the A-Frame tests and didn't fix the aframe-react ones from the previous post, but it fixed the issue in my app.

Here is the fix: https://github.com/ip/aframe/tree/child_attached_delay

It fixed the problems with two components: A-Frame native raycaster (with enabled object filtering) and a fork of third-party mouse-component (the fork adds object filtering as well).

ngokevin commented 7 years ago

@ip RSpace mentioned laser-controls (in A-Frame master) now works, do you see any improvements?

ip commented 7 years ago

@ngokevin I just tried the latest version of A-Frame from master (commit #e9ba54ebcf7 at the moment) and it doesn't fix.

RSpace commented 7 years ago

With the latest A-Frame master, filtering the raycaster using objects: .interactable works as expected for me, both for laser controls and cursor.