immersive-web / webxr

Repository for the WebXR Device API Specification.
https://immersive-web.github.io/webxr/
Other
3k stars 386 forks source link

How can an author determine if the a feature was granted #1148

Closed cabanier closed 3 years ago

cabanier commented 3 years ago

If an author asks for layers in requestSession, they have no way to determine if it was granted apart from brute-forcing it by requesting a cubelayers. Should we add a way to returns what features were granted?

Maksims commented 3 years ago

This is somewhat an issue for writing code against ungranted APIs. It it requires to make some bespoke checks for each API to see if it was granted based on available functionality. This probably even wont work where platform supports feature, but no permission was granted.

Manishearth commented 3 years ago

Duplicate of https://github.com/immersive-web/webxr/issues/794

This already exists via the permission API integration, you run navigator.permissions.query() with an XRPermissionDescriptor.

I don't think this has been implemented in any browser just yet, but there was general agreement as to this being the right way to do this (and i think Chromium has or planned to internally refactor to use the permissions API anyway).

cc @klausw who might know more about the implementation timeline for Chromium

cabanier commented 3 years ago

Until that feature lands (and it's been in limbo for several years), there is no way to check. @klausw do you know if there is any movement on getting it landed? I know the implementation is pretty much complete in Chromium so this would just be on the spec front.

Manishearth commented 3 years ago

@cabanier I don't think it's been in limbo for several years, it was specced a year ago .

But yes, there is no way to check right now unless you just try to use the APIs.

Not sure what you mean on the spec front: the spec work here is complete, there is so far no implementation work in browsers though.

cabanier commented 3 years ago

@cabanier I don't think it's been in limbo for several years, it was specced a year ago .

No, it was proposed almost 4 years ago.

Not sure what you mean on the spec front: the spec work here is complete, there is so far no implementation work in browsers though.

The implementation is complete in Chromium but behind a flag. During my Magic Leap days about 3 years ago, we had it enabled so we could detect enabled features. If there is no movement we should add a flag to the layers spec so authors aren't forced to create dummy layers.

Manishearth commented 3 years ago

No, it was proposed almost 4 years ago

I mean sure, that's true of most of this spec, I thought you were talking about implementation

The implementation is complete in Chromium but behind a flag.

I did a quick code search and it didn't seem to be?

cabanier commented 3 years ago

The implementation is complete in Chromium but behind a flag.

I did a quick code search and it didn't seem to be?

See https://github.com/chromium/chromium/blob/master/third_party/blink/renderer/modules/permissions/permissions.cc#L53

I just checked and it's already enabled in Chrome so we're good :-)

klausw commented 3 years ago

@cabanier , this doesn't appear implemented yet in Chromium. The code you link to checks for named permissions, but this issue is about getting the status of required/optional features via permission queries which would need to be implemented separately.

if I'm reading it right, Permissions::query calls ParsePermissionDescriptor which has case handling for various permissions, but I don't see any specific handling for the "xr" permission, and no code for requiredFeatures/optionalFeatures attributes from an XRPermissionDescriptor.

The only reference to XRPermissionDescriptor I can find in Chromium source is in third_party/blink/web_tests/external/wpt/interfaces/webxr.idl which is part of web_tests, and separate from the IDL files used as part of the implementation such as third_party/blink/renderer/modules/xr/xr_session_init.idl .

klausw commented 3 years ago

To follow up, it seems that the implementation would need a new xr_permission_descriptor.idl file alongside the existing examples such as camera_device_permission_descriptor.idl, conversion logic in ParsePermissionDescriptor, and glue code to hook this up with the actual required/optionalFeatures logic in the XR code to answer queries about the current session.

@toji , I think you were more involved in the spec discussions around this, do you happen to know if there's an existing Chromium bug to implement this and/or concrete plans?

Manishearth commented 3 years ago

Yeah, to be clear, Chrome implements navigator.permissions.query() just fine, but it doesn't know about XRPermissionDescriptor so you can't query any XR stuff.