immersive-web / anchors

https://immersive-web.github.io/anchors/
Other
50 stars 20 forks source link

first pass at persistent anchors #75

Closed cabanier closed 1 year ago

cabanier commented 2 years ago

/agenda https://cabanier.github.io/anchors/


Preview | Diff

cabanier commented 2 years ago

More work needs to be done to describe how the UUIDs are stored by the UA.

AdaRoseCannon commented 2 years ago

Thanks @cabanier I am looking forward to this

bialpio commented 2 years ago

More work needs to be done to describe how the UUIDs are stored by the UA.

Took a quick pass now. A couple of questions:

cabanier commented 2 years ago

More work needs to be done to describe how the UUIDs are stored by the UA.

Took a quick pass now. A couple of questions:

  • Are you planning on expanding this PR w/ details on how storing the UUIDs will be done, or will that be a separate PR?

I was planning on extending this PR; otherwise the spec would be in an incomplete state. Maybe that doesn't matter?

  • Do we want to add additional feature descriptor for persistent anchors, or will the requestPersistentHandle() just fail in case the UA does not support this capability?

I would prefer if we keep the same descriptor and just fail if the UA doesn't support persistence.

bialpio commented 2 years ago

More work needs to be done to describe how the UUIDs are stored by the UA.

Took a quick pass now. A couple of questions:

  • Are you planning on expanding this PR w/ details on how storing the UUIDs will be done, or will that be a separate PR?

I was planning on extending this PR; otherwise the spec would be in an incomplete state. Maybe that doesn't matter?

Extending the PR sounds good (and it'll be easier to see all the changes needed for persistent anchors).

  • Do we want to add additional feature descriptor for persistent anchors, or will the requestPersistentHandle() just fail in case the UA does not support this capability?

I would prefer if we keep the same descriptor and just fail if the UA doesn't support persistence.

Sounds good to me!

thetuvix commented 2 years ago

Have a conflict for today's call. However, I took a quick look through this PR and the general approach looks good to me! It should be straightforward to implement this on top of the XR_MSFT_spatial_anchor_persistence OpenXR extension.

The major design point worth talking through is whether it's more "webby" for this API to return the persistent anchor keys upon the request, or to accept the desired key from the app. Microsoft's underlying XR_MSFT_spatial_anchor_persistence OpenXR extension accepts a key from the app, though I don't feel strongly about that approach - it seems OK to require the app to tie the UA-generated key to some meaning in its own storage.

The other thing I would be sure to clarify when you write up prose is that no promises are made that this key will be usable on any other device - this API is about local-device anchor persistence rather than cross-device anchor sharing. For privacy reasons, we may actually want the line in the spec that mentions that the keys returned must be unique per-domain to also mention that they also must be unique per-device.

cabanier commented 2 years ago

The other thing I would be sure to clarify when you write up prose is that no promises are made that this key will be usable on any other device - this API is about local-device anchor persistence rather than cross-device anchor sharing. For privacy reasons, we may actually want the line in the spec that mentions that the keys returned must be unique per-domain to also mention that they also must be unique per-device.

Good point! I'll add that to the spec.

cabanier commented 2 years ago

/tpac discuss persistent anchor proposal

bialpio commented 1 year ago

Looks good to me, w/ minor nitpicks that can be addressed later.