Skip to content

Added support for angular and linear velocity - #1182

Merged
Manishearth merged 3 commits into
immersive-web:mainfrom
cabanier:pose-velocity
Mar 14, 2021
Merged

Manishearth merged 3 commits into
immersive-web:mainfrom
cabanier:pose-velocity

Conversation

@cabanier

@cabanier cabanier commented Mar 12, 2021 •

Copy link
Copy Markdown
Member

Closes #619

Also silences a couple of bikeshed warnings.


Preview | Diff

@cabanier

Copy link
Copy Markdown
Member Author

@Manishearth why can I not add reviewers?

@toji toji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after one correction.

Comment thread index.bs Outdated
@toji

toji commented Mar 13, 2021

Copy link
Copy Markdown
Member

GitHub had a hangover this morning, which is why you couldn't add reviewers. I think it's generally better now.

@cabanier
cabanier requested a review from toji March 13, 2021 04:42
@cabanier

Copy link
Copy Markdown
Member Author

GitHub had a hangover this morning, which is why you couldn't add reviewers. I think it's generally better now.

I still can't add reviewers.

@cabanier

Copy link
Copy Markdown
Member Author

I was looking into a possible implementation and there are some spaces where velocity will never be reported.
For instance XRJointSpaces and XRTargetRaySpace with a XRTargetRayMode different than POINTING.

Should we add those exceptions to the spec, or leave it up to the UA?

@toji

toji commented Mar 13, 2021

Copy link
Copy Markdown
Member

I think it's best to leave that up to the UA. For example, I can see that OpenXR has an extension for hand joint velocity and I suspect that the tracking method (ie: optical vs sensors) will make a big difference in what velocities are available. I wouldn't want to prematurely prevent future advances in tracking.

If we're concerned about developers needing to know what spaces can and can't report velocities, it would probably be best to report that as a property of the space (hasLinearVelocity/hasAngularVelocity booleans, maybe?) but I don't know that we can do that consistently and reliably on all platforms, and I believe even spaces that CAN report velocities may have moments where it lapses, such after the first frame or two of tracking (depending on the tracking method). As a result, developers will need to be able to handle null velocities no matter what.

We should probably just leave it as "If the user agent can't populate this, it's allowed to return null."

@toji toji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the angular velocity definition, by the way!

@cabanier

Copy link
Copy Markdown
Member Author

@toji thanks for approving! Can you merge it for me? I'm not able to.

@toji

toji commented Mar 14, 2021

Copy link
Copy Markdown
Member

If you don't mind I'd like to make sure that @Manishearth has had a chance to look before I do? That should be Tues at the latest, since we have our editors sync then. Normally I wouldn't be too concerned but given this is a change to a largely "shipped" spec I feel a smidge of extra caution is warranted.

I doubt there's going to be any concerns, though.

@cabanier

Copy link
Copy Markdown
Member Author

I see. That sounds good! There's no hurry to get it in.

@Manishearth Manishearth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good now.

One note: angular velocity can be obtained from the other attributes; and we tend towards not having APIs that expose information that can be calculated by the author, but I think it's fine in this case.

@Manishearth
Manishearth merged commit 10a4f07 into immersive-web:main Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Linear/Angular Acceleration/Velocity for XRInputSource

3 participants