webxr icon indicating copy to clipboard operation
webxr copied to clipboard

Expose a way to query a session about the supported features - need to reconsider?

Open bialpio opened this issue 3 years ago • 12 comments

I do not seem to be able to reopen the original issue (#952), so filing a new one now that we have independent sets of enabled features on an XR device & on an XRSession.

If my read of the spec is correct, the Permissions API will only return permission status for the device features (which may be persistent across sessions and do not really say anything about the state of the session), so the apps have no way of observing the contents of XRSession's set of enabled features.

My preferred way to solve this would be extending the XRSession to expose a set of enabled features. Spec-wise, it seems it could be achieved by turning the not-IDL-exposed XRSession/set of enabled features into an IDL-exposed attribute. XRSession.enabledFeatures of type set<DOMString> (or set<any>, although I'm not sure why DOMString wouldn't be sufficient here).

Related issues: immersive-web/real-world-geometry#30 immersive-web/anchors#64

bialpio avatar Jun 17 '21 23:06 bialpio

+1 on that

cabanier avatar Jun 17 '21 23:06 cabanier

Yeah, I agree, and when I was adding that field it was with the hope that we might expose it at some point.

Manishearth avatar Jun 18 '21 00:06 Manishearth

Strawperson proposal:

enum XRFeature {
  "local",
  "local-floor",
  ...
};

interface XRFeatureSet {
  // Alternatively, this could be a DOMString, or any.
  readonly setlike<XRFeature>;
};

partial interface XRSession {
  // Probably should not be [SameObject] if we want to allow enabling features post-session-creation.
  // Alternatively, FrozenArray (which would make it impossible to make it a [SameObject] IIUC).
  readonly attribute XRFeatureSet enabledFeatures;
};

I don't think there's a big difference between an enum vs DOMString (maybe for editorial purposes, but we already have a "feature descriptor" that we keep extending in modules). IMO it should not be any (I'm not sure why we're accepting anys as required / optional features if we also spec they'll be ToString()'ed), but I don't have full context on this so I may be missing something.

Naming TBD, I went with XRFeature above but it may very well be XRFeatureName or XRFeatureDescriptor. This only matters if we want to go with the enum route. I also went with enabledFeatures for the attribute name, but this may be confusing if we ever have a feature that needs to be configured post-session-creation (so the feature is granted, but isn't yet enabled / active / etc) so it's probably better to call it grantedFeatures.

As for the attribute type, I think we should be returning something that's setlike as the main use case for the apps would be to check whether a specific feature was enabled, and this boils down to enabledFeatures.has("feature") check with a setlike.

As for semantics, I'd propose that all enabled features are returned (including the required ones). I think this would make the app logic simpler ("to check whether a feature is enabled, I check if it's in XRSession.enabledFeatures" vs "to check whether a feature is enabled, if it was optional, I check if it's in XRSession.enabledFeatures, otherwise it's enabled because I wouldn't have gotten a session"). As a bonus, the spec text becomes simpler since we'd just need to expose the session's set of granted features to the app.

LMK if this looks reasonable, I can issue a PR if so.

bialpio avatar Jun 23 '21 21:06 bialpio

Should we just return a set directly? I guess it could get more complex for cases of non-stringy descriptors.

Manishearth avatar Jun 23 '21 22:06 Manishearth

Another thing to consider: can a feature become available/unavailable during session life? E.g. physical disconnect of hardware, or permissions change, etc?

Maksims avatar Jun 25 '21 17:06 Maksims

Should we just return a set directly? I guess it could get more complex for cases of non-stringy descriptors.

I'm not sure if this is doable in Web IDL - what type to put in Web IDL to signify that the result is a Set?.. Set is defined in ECMAScript spec so it may not be possible to do so through Web IDL (there's no Web IDL type that maps to ECMAScript's Set), IIUC setlikes are the way to go here. :frowning_face: FWIW, I've had some internal discussion about it and was told that devs are used to FrozenArrays & that introducing a new interface that's empty save for setlike is not a great idea. I'm not sure if I agree with that given the use case we have here (IIUC, FrozenArray would make the "is feature enabled" check linear).

As for the non-stringy descriptors - do you have some context on them? I see we have specced that feature descriptor can be any, I imagine so that we could have features that could have more complex initialization, but so far for those we went with feature descriptors that are DOMStrings & extended the XRSessionInit dictionary if a feature needed additional parameters (see for example DOM Overlays and Depth Sensing, which extend the dict with additional entries that are required by the UA if a feature that needs them is requested [this is not enforced at the Web IDL level]). Is the flexibility of passing anys still needed given this approach?

can a feature become available/unavailable during session life?

Currently, I believe it's not possible, but I've seen discussions that we may want to be able to allow that, e.g. because an app could ask for more features after the session was created. If we ever allow it, we'll need to be certain to spec it so that UAs can still disallow it - I know there are some AR session features can only be enabled at session creation in our implementation.

bialpio avatar Jun 25 '21 22:06 bialpio

/agenda

toji avatar Aug 10 '21 18:08 toji

Did we discuss this issue 2 weeks ago? If so does it still need discussing?

AdaRoseCannon avatar Aug 24 '21 14:08 AdaRoseCannon

We discussed it but we didn't ask anyone to work on it so nothing happened :-)

cabanier avatar Aug 24 '21 14:08 cabanier

I've got some time and was looking at writing up the spec change for this. From looking here and at the minutes; it's not clear how/if people felt strongly about:

  1. FrozenArray vs. Setlike (I think I lean towards FrozenArray given that it sounds like that has faster lookup times and blocks us applying [SameObject], which is probably the right thing to give developers flexibility to enable/disable items mid session)
  2. enum vs. DOMString vs. any It's a non-normative note that features that can be ".ToString()" will be; and we have somewhat established a pattern that feature descriptors are strings and initialization structs/otherwise extends the XRSessionInit dictionary. I shy away from an enum just a bit, because I think it adds some extra overhead in adding a new feature descriptor that we haven't done in the past, and we'd probably need to go update every spec that defines it. It also wouldn't make sense to convert required/optional features to take an enum, as there would then be rejections for unknown optional features. For these reasons, I'm leaning towards DOMString and potentially also updating the required/optional feature args from any and to DOMString. If we feel strongly that required/optional feature args should remain as any, I'd like to add some text requiring that feature descriptors have some form of "ToString()" or otherwise have a string representation as well as their more complex representation.

Does anyone have some thoughts here before I take a stab at a draft? I don't know that these needs to be discussed in a meeting since it seems we had general agreement about doing this in the past; but if I don't get any responses (or anyone would prefer to discuss it there), I'll add it to the agenda.

alcooper91 avatar Aug 16 '22 17:08 alcooper91

It sounds like you already spent a good time thinking this through. It'd be great if you could create a draft proposal that we can discuss in a meeting.

cabanier avatar Aug 16 '22 18:08 cabanier

/agenda to discuss the PR above

We can remove it from the agenda if we feel okay to merge it before then.

alcooper91 avatar Aug 16 '22 20:08 alcooper91

Closing since PR #1296 fixed this

alcooper91 avatar Oct 20 '22 20:10 alcooper91