Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spec text for required and optional features #749

Merged
merged 9 commits into from
Jul 3, 2019
Merged

Spec text for required and optional features #749

merged 9 commits into from
Jul 3, 2019

Conversation

toji
Copy link
Member

@toji toji commented Jul 2, 2019

This is the corresponding spec text for the functionality described in
PR #739. There's some hand waving going on around user consent and such
because those will be better defined in a follow-up PR. The primary goal
of this PR is to establish the algorithims that drive how the feature
requests are processed.

@toji toji added this to the June 2019 milestone Jul 2, 2019
@toji toji requested a review from NellWaliczek July 2, 2019 16:55
Copy link
Member

@NellWaliczek NellWaliczek left a comment

Choose a reason for hiding this comment

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

Looking good. A bit of feedback, but it seems pretty close :)

index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@toji
Copy link
Member Author

toji commented Jul 2, 2019

Okay, did another pass on this to address all of @NellWaliczek's feedback and in general try to adhere better to the verbiage established in #739.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
@NellWaliczek
Copy link
Member

Oh, bother. I just noticed that line spatial-tracking-explainer.md line 329 should have been updated to say "local" not "local-floor". Do you mind fixing that up in this PR while you're at it?

@toji
Copy link
Member Author

toji commented Jul 2, 2019

Done. :) Any further thoughts? I know this is a tricky one to get the verbiage right on.

Copy link
Member

@NellWaliczek NellWaliczek left a comment

Choose a reason for hiding this comment

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

One last nit, but otherwise looks good to me. I'd like @johnpallett or @avadacatavra to double check the consent parts, but other than that, ship it!

explainer.md Outdated Show resolved Hide resolved
@toji
Copy link
Member Author

toji commented Jul 2, 2019

Thanks! Took care of the last nit, so now I'm just patiently awaitng feedback from @johnpallett or @avadacatavra. 👀

Copy link
Contributor

@johnpallett johnpallett left a comment

Choose a reason for hiding this comment

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

Found some typos, otherwise LGTM

explainer.md Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved

The feature lists accept any string. However, in order to be a considered a valid <dfn>feature name</dfn>, the string must be a value from the following enums:

- {{XRReferenceSpaceType}}

Choose a reason for hiding this comment

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

I like the overall shape, but have a couple question about the list of features. I apologize if these are discussed or answered in the explainer already, but I didn't see an answer here.

Devices that support local reference spaces MUST support local-floor reference spaces, through emulation if necessary, and vice versa. <- If I require local-floor, am I guaranteed real floor height or emulated is sufficient to satisfy this? Do I need to require local-floor for requestReferenceSpace to succeed later with emulated floor-level?

It seems there isn't any way to require features like "6dof" or "6dof controllers". Is this intentional? I see demand for this (some apps/games may not work well in 3dof). Sites can get a somewhat reasonable approximation by requiring "bounded-floor", but this may exclude hardware that supports 6dof but not bounds.

Its possible that for some features (ie bounds), hardware capability/calibration may not be known until after the session is started from the browser's perspective. Is the required features optimistic, and the session succeeds even though bounds may never be available, or should the browser fail the requestSession call?

Copy link
Member

@NellWaliczek NellWaliczek Jul 3, 2019

Choose a reason for hiding this comment

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

Devices that support local reference spaces MUST support local-floor reference spaces, through emulation if necessary, and vice versa. <- If I require local-floor, am I guaranteed real floor height or emulated is sufficient to satisfy this? Do I need to require local-floor for requestReferenceSpace to succeed later with emulated floor-level?

It seems there isn't any way to require features like "6dof" or "6dof controllers". Is this intentional? I see demand for this (some apps/games may not work well in 3dof). Sites can get a somewhat reasonable approximation by requiring "bounded-floor", but this may exclude hardware that supports 6dof but not bounds.

The decision not to allow "require 6DOF" is actually core to the spatial tracking design. "Local" and "local-floor" experiences are the lowest common denominator and should work for either 3dof or 6dof. Real height is not guaranteed, but developers shouldn't be differentiating it anyway because people come in lots of heights and the UAs may be adding overrides for accessibility purposes. If an experience requires 6dof, then it should request "bounded-floor" by its very nature. Controller requirements are described in #755.

Its possible that for some features (ie bounds), hardware capability/calibration may not be known until after the session is started from the browser's perspective. Is the required features optimistic, and the session succeeds even though bounds may never be available, or should the browser fail the requestSession call?

Perhaps I'm misunderstanding, but the exact bounds aren't necessary to do a session check. The capability to create a bounded reference space is what's being checked.

(ok, seriously.. woods time now)

Choose a reason for hiding this comment

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

Understood about local and local-floor being lowest common denominator that should generally work with 3dof. Also agree with the accessibility concerns. It sounds like we're ok with requiring bounded-floor ~= requiring 6dof, which is probably fine for VR headsets. As AR is added we may want to revisit this. Thanks for the link to the input issue.

For the last point, there are several degrees of capability - the runtime may support bounds for some hardware, the hardware may support bounds, there may be data (calibration?) that actually specify bounds, the bounds may be located relative to the user. Should we be clear which we mean? There may be times where we don't know which degree of capability we have without starting the runtime - OpenXR doesn't seem to specify a way to inquire about bounds without starting a session for example.

More specifically - if bounded-floor is required, should requestReferenceSpace('bounded-floor') necessarily succeed? I think my comment/question is at most a clarification, so feel free to land.

Have fun in the woods.

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to land this now prior to heading into the US holiday weekend, but to try and answer your question:

Requirements are always going to be a best effort endeavor for certain things, especially if connectivity or calibration of the functionality in question is fluid. In a fair number of cases I'd imagine that the user agent will be running on known hardware (like a Quest) and can confidently answer capability questions without any API interaction. In some cases it may actually be appropriate to spin up a platform-level session equivalent to do some basic checks as long as you can delay visual transitions. (I think OpenXR probably allows for this.) It may also be appropriate for the UA/platform to intervene at the point of session creation to ask the user to do some setup. For example:

This VR experience requires you to configure your room boundaries before launching. Do you wish to do so now, or exit this experience?

I would also say in some cases it's probably acceptable to, if you know the platform is capable of a feature but it takes a moment to initialize, return dummy data until you have the real stuff. Such as a bounded-floor reference space returning a ~1m circle of bounds initially and then firing a reset event when the actual data kicks in.

if bounded-floor is required, should requestReferenceSpace('bounded-floor') necessarily succeed?

I'm not 100% confident about this, as I can see the need for some wiggle room in some cases, but in general I would say yes? I also feel like that's a policy that may require different answers for different features as we add new ones in the future. (If we add a theoretical 6dof-controllers feature, it would be impractical to insist that said controllers be continuously available from the very first frame.)

toji and others added 7 commits July 3, 2019 09:35
This is the corresponding spec text for the functionality described in
PR #739. There's some hand waving going on around user consent and such
because those will be better defined in a follow-up PR. The primary goal
of this PR is to establish the algorithims that drive how the feature
requests are processed.
Co-Authored-By: johnpallett <johnpallett@google.com>
@toji toji merged commit a09b874 into master Jul 3, 2019
@toji toji deleted the feature-spec branch July 3, 2019 20:53
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.

4 participants