fix raycaster not grabbing all entities when objects is not set

[ngokevin]

Description:

found by @dsinni. r? @donmccurdy

[donmccurdy]

By default raycasting is recursive, so querySelectorAll('*') will check nested entities multiple times. What issue does this fix? The current behavior does not look obviously wrong to me.

[dsinni]

@donmccurdy: See #3838.

animation startEvents are not working when raycaster objects are not defined. I haven't yet tested further to see if it was limited to animation, though,

[donmccurdy]

Hm ok. Maybe I've misread and each mesh is only raycast against once, then? If so flattenObject3DMaps could probably use a bit clearer documentation and tests.

[dsinni]

Okay, just tested this further and it appears that it's not limited to animation and likely is a general raycasting issue.

I usually wrap the scene in a "stage" entity, save for the camera, which stays outside of it. This allows me to translate the scene if need be.

I'm just noticing that simply having a parent wrapper is causing the issue, which was not the case previously.

You can see it demonstrated here: https://glitch.com/~imported-wallaby

If you try clicking any of the entities, nothing happens. There should be an animation for each, excluding the plane, and the console should log the event.

If you comment out the #stage wrapper entity, it works as expected.

[arpu]

maybe looks like more this problem #3818 @donmccurdy reopen the bugreport?

[dmarcos]

As @donmccurdy I don't understand the difference between this.el.sceneEl.querySelectorAll('*') and this.el.sceneEl.children. They should both iterate over all entities in the scene, don't they?

[donmccurdy]

Given this scene:

<a-scene>
  <a-entity id="room1" gltf-model="room1.glb">
    <a-box/>
    <a-box/>
    <a-box/>
  </a-entity>
  <a-entity id="room2" gltf-model="room2.glb">
    <a-box/>
    <a-box/>
    <a-box/>
  </a-entity>
</a-scene>

scene.children will give a list of the outer "room" wrappers, whereas querySelectorAll('*') will list those wrappers and each of their children:

[ room1, room2, box1, box2, ... ]

If we passed room1.object3D directly to the raycaster in recursive mode, it would be hit-testing each box multiple times. I think the flattenObject3DMaps implementation avoids that. If this is the tree structure:

- room1.object3D
  - mesh
  - box1.object3D
  - box2.object3D
  - ...

... it should only add the mesh, which has no children, to the objects list. But that is a bit hard to confirm from the current documentation and tests, and should probably be more explicit.

[arpu]

dont know if this helps here but the sample from @dsinni works with aframe A-Frame Version: 0.8.2 (Date 2018-06-28, Commit #94c6b33)
https://guttural-sagittarius.glitch.me/ ( i can see the console events)

[donmccurdy]

Yeah to be specific — I'm pretty sure this PR is correct and fixes the issue, but we should add a unit test ensuring we don't raycast the same mesh twice when it's nested under another entity.

[dmarcos]

It sounds reasonable to have a test to ensure there are no redundant checks.

[dmarcos]

Can we add a test to check that there are no redundant raycasts when using querySelector(*)?

[ngokevin]

added test. the behavior was described internally at: https://github.com/aframevr/aframe/blob/master/src/components/raycaster.js#L390 and i updated doc.