aframe icon indicating copy to clipboard operation
aframe copied to clipboard

Cursor component: fixes for WebXR mode

Open JL-Vidinoti opened this issue 1 year ago • 4 comments

Description:

This PR fixes a couple of small issues when using the cursor component with rayOrigin: xrselect.

Changes proposed:

  • If the cursor component is attached to an entity other than a-scene, an error occurs.
  • The XR Session events are not unregistered when the cursor attribute is removed.

JL-Vidinoti avatar May 29 '24 07:05 JL-Vidinoti

While I'm not entirely sure about the history of the cursor component, it seems to me that the original intent was for cursors with rayOrigin: xrselect to be defined on <a-scene>. I don't think there is any harm in making it work on any arbitrary entity, but more changes might be needed. Looking through the code it seems that rayOrigin: xrselect actually causes the underlying raycaster to not use world coordinates, meaning incorrect results can happen when the entity is translated or rotated.

As for the event listeners, the change looks good, but addEventListeners should be amended as well. In case the cursor component is added during an XR session (or when calling .pause()/.play()) there won't be an enter-vr event, but the relevant listeners should still be added.

mrxz avatar May 29 '24 10:05 mrxz

I am doing some tests with the Vision Pro. The cursor component works with the mode xrselect using eye gaze (see the new input mode Apple introduced https://webkit.org/blog/15162/introducing-natural-input-for-webxr-in-apple-vision-pro/).

If we use a camera rig like below, it is necessary to add the cursor component to the rig (not to the scene) otherwise the computed eye gaze origin is wrong when moving the rig.

<a-entity id="rig" cursor="rayOrigin: xrselect;">
    <a-entity id="camera" camera look-controls position="0 1.6 0" ></a-entity>
</a-entity>

I see your point with the addEventListener, should I update the PR?

JL-Vidinoti avatar May 29 '24 11:05 JL-Vidinoti

If we use a camera rig like below, it is necessary to add the cursor component to the rig (not to the scene) otherwise the computed eye gaze origin is wrong when moving the rig.

I see, I still think this is something that the cursor component could and should handle internally. The ray's origin and direction retrieved from the WebXR API is relative to the reference space, and it should be translated to world space based on the camera parent's transform. Requiring the user to place the cursor on the rig just opens the possibility of users either forgetting it or placing it on the wrong entity. Not to mention situations where the camera is changed to a different camera/rig.

Put differently, there is no meaningful reason to use a different entity than the rig. And since A-Frame knows the reference space and the relevant transform of the reference space (parent transform of the active camera), there's no reason to require the user to specify/configure it manually.

I see your point with the addEventListener, should I update the PR?

Updating the PR would be nice, it makes sense to have removeEventListener and addEventListener updated at the same time.

mrxz avatar May 29 '24 12:05 mrxz

I updated the handling of the WebXR event listeners such that they are correctly registered/unregistered. Concerning the cursor component, I think that is is up to the user whether he adds it to the scene or any other entity.

JL-Vidinoti avatar Jun 10 '24 06:06 JL-Vidinoti

Thanks for the patience. I had to reload the context in my head. xrselect nomenclature is definitively confusing.

We introduced in https://github.com/aframevr/aframe/pull/5065 to handle generic input events that come from an input we can't track like in the case of taps on the screen while in immersive mode on mobile or gaze in the vision Pro. Origin of those events is uknown until the event fires vs a mouse, non immersive touch events on mobile or a tracked controller that we always know where they are.

I agree, we should calculate the correct coordinates internally and not put the burden on the dev. Is there any quick modification we can do to this PR?

Goal is to ship 1.7.0 next week. Would be awesome to include this. Thanks again

dmarcos avatar Nov 12 '24 05:11 dmarcos

I agree, we should calculate the correct coordinates internally and not put the burden on the dev. Is there any quick modification we can do to this PR?

It should just be a matter of converting the WebXR pose from the reference space into world space. Basically just applying the worldMatrix of the parent of the (active) camera.

mrxz avatar Nov 13 '24 10:11 mrxz

@JL-Vidinoti @mrxz Any of you wanna take this PR and make it calculate the pose in world space vs having the user to configure manually? Thanks

dmarcos avatar Nov 14 '24 02:11 dmarcos

Unfortunately, I don't have the resources to do it in the short term.

JL-Vidinoti avatar Nov 14 '24 06:11 JL-Vidinoti

@dmarcos Created a new PR that implements the logic along with two additional fixes: #5606. Might be a good idea to test on an AVP as well to make sure it works correctly with transient inputs.

mrxz avatar Nov 14 '24 10:11 mrxz

Closing this in favor of https://github.com/aframevr/aframe/pull/5606

Thanks everybody

dmarcos avatar Nov 15 '24 02:11 dmarcos