CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 401
Spec: Reference space privacy considerations #762
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
Conversation
d3dfe95
to
3b3d687
Compare
@johnpallett: Can't add you as a reviewer directly, but could you take a look at 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.
Thanks @toji comments inline.
I have a couple of general questions. The text here largely focuses on accepting or rejecting features during session creation. The original explainer spoke more about promise rejection during session creation and when creating a reference space.
-
Are we comfortable that this approach gives sufficient direction to implementers? For example, the original explainer directed the implementer to reject the reference space request promise if the requirements for a reference space could not be met;
-
Are implicit features covered (particularly local reference spaces?)
Thanks for the feedback, John! Left some responses and made some tweaks.
This has been my primary source of angst around this text. (And #761, to a slightly lesser degree.) It comes down to a couple of things, I think:
If you have any thoughts on how to ease that tension, I'm happy to hear it! |
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.
[EDIT: I realized my comment was already addressed. You can ignore this!]
4184f6a
to
bccf59e
Compare
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.
A few nits, but mostly my concern is around making those restrictions more algorithmic. For example forcing native origins to be the same isn't likely to be an intrinsic property of XR systems and is something that the UA will need to manage themselves. Similarly, limiting pose data to a specific distance from the native origin is not likely to be a feature of the underlying XR systems. These sorts of things seem like they can actually be done algorithmically as part of the requestReferenceSpace() algorithm flows.
e7c2f4b
to
bcf50a0
Compare
Moved the reference space requirement bullet points around to the appropriate contexts and/or made them part of an algorthim. This feels like it "dilutes" the privacy-centric portion of the doc, because now some privacy driven limits are scattered throughout the doc, but it is probably easier for implementers this way and ultimately that's what counts. |
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.
More comments/questions!
If {{XRReferenceSpaceType/"local"}} is included in an {{XRSession}}'s [=requested features=] the user agent MUST ensure [=user intent=] to allow {{XRReferenceSpaceType/"local"}} tracking is well understood, either via [=explicit consent=] or [=implicit consent=], or the feature MUST be rejected. | ||
|
||
Note: {{XRReferenceSpaceType/"local"}} is always included in the [=requested features=] of {{XRSessionMode/"immersive-vr"}} and {{XRSessionMode/"immersive-ar"}} sessions as a [=default feature=], as as such {{XRSessionMode/"immersive-vr"}} or {{XRSessionMode/"immersive-ar"}} sessions always need to obtain [=explicit consent=] or [=implicit consent=] to use {{XRReferenceSpaceType/"local"}} reference spaces. | ||
|
||
<section class="unstable"> | ||
Gaze Tracking {#gazetracking-security} |
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.
Are we going to have a separate PR to remove the privacy sections that we've replaced with all this new text?
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.
Yeah, a separate PR sounds best for 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.
Before merging, can you file that? Are we intending to do it before VR complete (looks skeptically at the calendar)
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.
#776. And while it would be nice, I don't think this is necessary for VR complete because it should mostly entail removing redundant information.
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.
Ok, let's just make sure we didn't preemptively remove the 'unstable' banner from around it before we cut the next working draft.
index.bs
Outdated
|
||
Note: It is suggested that points of the [=native bounds geometry=] be [=limiting|limited=] to 15 meters from the [=XRSpace/native origin=] in all directions. | ||
|
||
Each point in the [=native bounds geometry=] MUST also be [=rounding|rounded=] sufficiently to prevent fingerprinting. For user's safety, rounded points values MUST NOT fall outside the bounds reported by the platform. |
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.
@johnpallett It just occurred to me... is rounding a sufficient mitigation here or should we also be applying fuzzing?
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.
ping @johnpallett. If you think this is a concern, please file an issue!
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.
No blocking concerns here. Thanks @NellWaliczek
Thanks for the review! Made a partial update to hit some of the simpler changes. I'll get the remaining change on or before Monday. |
8931c95
to
b28126f
Compare
Okay, everything should be addressed one way or another now. PTAL! |
If {{XRReferenceSpaceType/"local"}} is included in an {{XRSession}}'s [=requested features=] the user agent MUST ensure [=user intent=] to allow {{XRReferenceSpaceType/"local"}} tracking is well understood, either via [=explicit consent=] or [=implicit consent=], or the feature MUST be rejected. | ||
|
||
Note: {{XRReferenceSpaceType/"local"}} is always included in the [=requested features=] of {{XRSessionMode/"immersive-vr"}} and {{XRSessionMode/"immersive-ar"}} sessions as a [=default feature=], as as such {{XRSessionMode/"immersive-vr"}} or {{XRSessionMode/"immersive-ar"}} sessions always need to obtain [=explicit consent=] or [=implicit consent=] to use {{XRReferenceSpaceType/"local"}} reference spaces. | ||
|
||
<section class="unstable"> | ||
Gaze Tracking {#gazetracking-security} |
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.
Before merging, can you file that? Are we intending to do it before VR complete (looks skeptically at the calendar)
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.
Looking really good! A few more small nits.
Describes the mitigations and checks that must be used when supporting the various reference spaces. Pulls heavily from text originally in privacy-security-explainer.md.
Co-Authored-By: Nell Waliczek <nhw@amazon.com>
82f79b5
to
2440f62
Compare
Nits addressed. Thanks! |
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.
Only one nit. Approving!
Describes the mitigations and checks that must be used when supporting
the various reference spaces. Pulls heavily from text originally in
privacy-security-explainer.md.
Posted as a draft because I'm not sure if I'm happy with the format yet.