-
Notifications
You must be signed in to change notification settings - Fork 380
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
Explicitly spec out when requestReferenceSpace() can reject queries #651
Conversation
not sure if this is what you were looking for. I can also change the language to be stronger/weaker on this. (I suspect we'll have to tweak this a bit as our permissions model comes in) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good initial pass, but we probably want to iterate on how we present these requirements a bit.
I think the first new line, about rejecting bounded/unbounded for inline, is perfect as-is. After that I feel like we might want something more along the lines of "In condition if the requirements for the type are not met, return null", and then point back at the type definitions somehow for individualized requirements. (I'm not sure what the right way to indicate that kind of "one-to-many" relationship in spec text is.)
For example: The bounded
reference space should only be supported if the XR device can report a bounded area and accurately align with the floor. (And Nell and I have talked about limiting it to only supporting play areas above a certain size as well, but we don't have an exact recommendation for that yet.) That doesn't feel like criteria that belongs in this algorithm, though. I'd rather have it appear in the bounded
ref space definition somehow, and have the algorithm refer back to it.
index.bs
Outdated
1. If |type| is {{bounded-floor}} or {{unbounded}}, and |session|'s [=XRSession/mode=] is not {{XRSessionMode/inline}} the user agent may choose to return <code>null</code> if such experiences are not supported by the underlying [=/XR device=] | ||
1. If |type| is {{local-floor}} or {{local}}, and |session|'s [=XRSession/mode=] is {{XRSessionMode/inline}} the user agent may choose to return <code>null</code> if such experiences are not supported by the underlying [=/XR device=] in non-immersive sessions | ||
|
||
NOTE: All other combinations of {{XRReferenceSpaceType}}s and [=XRSession/mode=]s are guaranteed by this spec, provided the [=XRSession/mode=] itself is supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't imagine placing a note in the middle of an algorithm like this formats nicely? It should definitely be moved outside the algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes like this seem to work fine, they get indented. A couple specs do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well color me surprised! Today I learned something new.
Happened to be able to catch @bfgeek and ask him about this. (Presented as "I want my spec algorithm to have a virtual function, is that a thing?") He wasn't aware of any other specs that did something similar, and recommended a "manual V-Table dispatch" (because we are oh so painfully geeky) like so:
Then, near the reference space definitions, define the support tests if they're simple enough or link off to a new algorithm if they're complex:
And then in a contextually relevant spot:
(Obviously I'm being pretty hand-wavy with all of that.) Sorry that this is turning out to be a bigger issue than it first appeared. |
Thanks, I don't mind being more explicit in spec text :) I wasn't quite comfortable with the compact text in the current PR either but I didn't know enough about the various criteria for support to write more. This seems good! |
Redid it. I'm not quite sure what the criteria for inline local and unbounded spaces should be, but I took a guess. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! A couple more tweaks, but I think we're close now. We'll definitely be coming back to these later and adding criteria around user consent and such once some of the privacy policies are landed, but this will give us a much better framework in which to define those restrictions. Thanks!
index.bs
Outdated
1. If |type| is {{local}} or {{local-floor}}, and |session| is an [=immersive session=], return <code>true</code>. | ||
1. If |type| is {{local}} or {{local-floor}}, and the [=/XR device=] supports reporting orientation data, return <code>true</code>. | ||
1. If |type| is {{bounded-floor}} and |session| is an [=immersive session=], return if [=bounded reference spaces are supported=] by the [=/XR device=]. | ||
1. If |type| is {{unbounded}}, |session| is an [=immersive session=], and the [=/XR device=] supports reporting position and orientation data, return <code>true</code>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm don't think this is an adequate requirement for unbounded
, but I'm not sure how to describe it better. @thetuvix, do you have any thoughts about an appropriate support test here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of moving this PR forward, I propose we change this to something along the lines of "supports stable tracking near the user over an unlimited distance, return true
."
If there's a more accurate way to describe the requirements for an unbounded
space then we can fix it up later, but that should be good enough for now and prevent implementers from saying "Oh, by this definition my Vive totally supports unbounded
!"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Awesome, thanks! |
@@ -877,6 +892,15 @@ Each {{XRBoundedReferenceSpace}} has a <dfn for="XRBoundedReferenceSpace">native | |||
|
|||
The <dfn attribute for="XRBoundedReferenceSpace">boundsGeometry</dfn> attribute is an array of {{DOMPointReadOnly}}s such that each entry is equal to the entry in the {{XRBoundedReferenceSpace}}'s [=XRBoundedReferenceSpace/native bounds geometry=] premultiplied by the {{XRRigidTransform/inverse}} of the [=XRSpace/origin offset=]. In other words, it provides the same border in {{XRBoundedReferenceSpace}} coordinates relative to the [=XRSpace/effective origin=]. | |||
|
|||
|
|||
<div class="algorithm" data-algorithm="bounded-space-supported"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thetuvix are you on board with this language? If not, please feel free to propose an alternative when you update your tracking loss PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a reasonable approach. If the device can produce a floor-based coordinate system, but doesn't know boundaries, it should only support local-floor
.
If the device somehow knows boundaries without knowing the floor, I suppose the UA could estimate the floor as with local-floor
, but I don't know of any such devices today, so that would be more theoretical. We can always relax this later.
|
||
1. If |type| is {{inline}}, return <code>true</code>. | ||
1. If |type| is {{local}} or {{local-floor}}, and |session| is an [=immersive session=], return <code>true</code>. | ||
1. If |type| is {{local}} or {{local-floor}}, and the [=/XR device=] supports reporting orientation data, return <code>true</code>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To confirm, apps should be able to get a local
reference space even before the session becomes immersive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sessions do not "become" immersive, they are a single mode for their lifetime. Inline sessions should be able to create local
reference spaces, assuming the UA has properly handled the privacy implications (ongoing discussion).
fixes #564