Skip to content
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

Added session feature handling to the explainer. #433

Closed
wants to merge 1 commit into from
Closed

Conversation

toji
Copy link
Member

@toji toji commented Nov 13, 2018

Addresses #423 and #424.

A couple of key points that aren't explicitly stated here:

  • Checking with @bfgeek, it sounds like passing the "desired" feature list as string and mapping them to the feature enum is a reasonable way of avoiding errors on unrecognized enums. Hence the difference in the IDL between the desired and required sets.
  • Since the desired behavior for requiring frames of reference is "or" logic, which is different than every other expected feature, this PR leaves the existing requiredFrameOfReferenceTypes dictionary key alone.

Finally, this PR is obviously using a "filler feature" to demonstrate the API, since we don't have any other real features decided on yet. I would like to establish this as the expected method for handling these features now, but effectively mark it as "unimplemented" until we have features that will actually make use of it. (I'd need to update the PR to show that prior to landing.)

@toji
Copy link
Member Author

toji commented Nov 16, 2018

Updated this PR so that it also fixes #417 while I'm at it, since it's closely related.

// TODO: Make the example feature not theoretical.
navigator.xr.requestSession({
mode: 'immersive-vr',
desiredFeatures: ['feature-name']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been suggesting "geoAligned" as a feature. Perhaps that would be a reasonable example? It matches the pattern almost perfectly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

desiredFeatures: ['geoAligned']

}).then(onSessionStarted);

function onDrawFrame(timestamp, xrFrame) {
if (xrFrame.featureName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above:

if (xrFrame.geoAligned) {


UAs should ideally combine any consent requests resulting from both the `desiredFeatures` and `requiredFeatures` arrays into a single request so that the user isn't, for example, shown a long series of modal consent dialogs when a session is requests that makes use of many features.

Finally, there are some occasions where developers may want to defer a feature request until a more contextually appropriate time. For example **(FIXME: Is there a better short-term example?)**, an `immersive-ar` application may not need access to the camera stream until the point that the user clicks a "take a photo" button, at which point a request for camera access will be less concerning for the user. For these types of situations, new features can be requested after session creation with the `requestAdditionalFeatures` method. `requestAdditionalFeatures` can only be called in response to a user activation event, and accepts an array of strings that are processed identically to the `desiredFeatures` session creation option. It returns a promise that resolves when the user has consented to or denied use of any supported features. Individual features must still be checked to see if they are available after the promise resolves.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there a list of "candidate features" anywhere? Could choose from that.

@blairmacintyre
Copy link
Contributor

Overall, I like this.

@klausw
Copy link
Contributor

klausw commented Dec 13, 2018

@blairmacintyre and @toji - we have a potential candidate feature in #374 "More general layer quality hints". In short, an app should be able to opt in to foveated renderering, but for this to work on mobile GPUs it would also have to agree to abide by restrictions that it's not allowed to rebind the output framebuffer multiple times. So maybe something like desiredFeatures: ['foveatedRendering', 'singleBindFramebuffer']?

To avoid derailing this bug, can we continue discussion on this specific feature on #374 ? I added additional details there, I just wanted to mention it here as a possible candidate.

@AdaRoseCannon AdaRoseCannon added the agenda Request discussion in the next telecon/FTF label Jan 9, 2019
@toji toji added agenda Request discussion in the next telecon/FTF and removed agenda Request discussion in the next telecon/FTF labels Mar 4, 2019
@toji toji closed this Apr 29, 2019
@toji toji deleted the features branch April 29, 2019 22:47
@toji
Copy link
Member Author

toji commented Apr 29, 2019

Closing because when we do revisit this issue it's likely to not have the exact solution proposed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants