Add feature for secondary views #1083

Manishearth · 2020-06-12T20:40:24Z

Alternate to #1080

This PR adds an additional-views feature that can be used as a hint that the content is expecting additional views.

Related: immersive-web/webxr-ar-module#57

Fixes #1045

index.bs

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

index.bs

cabanier

I still think you should rephrase the intro section that mentions "we expect a significant..."

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Manishearth · 2020-06-15T19:38:38Z

Done

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Manishearth · 2020-06-15T19:50:15Z

I also made that entire part a note. I don't quite like notes that are of this form, but it's fine, it wasn't normative anyway.

index.bs

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Manishearth · 2020-06-16T19:44:56Z

Discussed in the call.

The current distinction being made is "additional views", i.e. views that are not the first mono/stereo views. Instead, this should be shifted to be "secondary views": views that you do not have to render to. This applies to bionic displays and FPO views, but not e.g. CAVE. The expectations of the content are the same. The layers spec can also rely on this concept to handle some of the issues in immersive-web/layers#160 , and add additional expectations of code which opts in to secondary-views + layers.

We were effectively saying this anyway, given the note about CAVE systems, but the underlying spec concept was less tangible/useful.

asajeffrey · 2020-06-16T22:23:13Z

So the idea would be to have an API where each view would have a isPrimary() method or some such to indicate if rendering it the content must render to it if it wants to avoid dropped frames, or just may render to it, otherwise the device will render something, even if it is sub-optimal?

Manishearth · 2020-06-17T00:01:56Z

@asajeffrey No, content has no way to distinguish between primary and secondary views in general, though we can add that if requested. If a view is being surfaced to content, content must render to it.

Instead, content is able to pass in a feature hinting to the UA that it would like to receive any secondary views, promising to handle it correctly.

The changes are going to be similar to what the spec is saying now, with some nuance around the precise definitions of "secondary".

asajeffrey · 2020-06-17T00:03:18Z

Ah, makes sense.

Manishearth · 2020-06-17T16:28:34Z

I've updated to make a clearer primary/secondary distinction as discussed on the call.

cc @cabanier @toji @thetuvix

cabanier

This looks good. Some minor nits.

index.bs

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

toji

LGTM overall. Two non-blocking questions:

toji · 2020-06-17T20:02:43Z

index.bs

+If [=secondary views=] have lower underlying frame rates, the {{XRSession}} MAY choose to do one or more of the following:
+
+ - Lower the overall frame rate of the application while the [=secondary views=] are active.
+ - Surface [=secondary views=] in the {{XRViewerPose/views}} array only for some of the frames. Implementations doing this SHOULD NOT have frames where the [=primary views=] are not present.


Should we list a third option, which is to return secondary views every frame, even if they'll be discarded? Obviously not the optimal thing but I can imagine that on some systems that may be the default behavior.

Obviously not the optimal thing but I can imagine that on some systems that may be the default behavior.

Do you know of a system that does this?

@toji I felt this was implied, and it was with the older language, but I'll make this explicit, thanks.

toji · 2020-06-17T20:06:53Z

index.bs

+</div>
+
+<div class=note>
+While content should be written to assume that there may be any number of views, we expect a significant amount of content to make incorrect assumptions about the {{XRViewerPose/views}} array and thus break when presented with more than two views.


Recognizing that https://github.com/immersive-web/webxr-ar-module/pull/57/files is still in progress, is there a compelling reason to identify which views are primary/secondary in cases other than the first person observer scenario? My gut says yes, but I can't identify a scenario where you'd need to know other than the first person observer.

Some other cases I've heard discussed:

Third-person observer (e.g. similar to Spectator View, although that particular implementation uses app-side state sync to a different device rather than this secondary views mechanism on one device)

360-degree screenshot/video (triggered by user in system UI)

@toji I don't see a good reason to identify bionic (quad) views. I think third-person observer would need to be independently identified.

thetuvix · 2020-06-18T00:33:38Z

index.bs

+
+When "[=secondary views/secondary-views=]" is enabled, the user agent MAY surface any [=secondary views=] the device supports to the {{XRSession}}. The user agent MUST NOT use reprojection to reconstruct [=secondary views=] in such a case, but instead rely on whatever the content decides to render.
+
+Note: We recommend content use {{XRSessionInit/optionalFeatures}} to enable "[=secondary views/secondary-views=]" to ensure maximum compatibility.


One thing we should discuss here is whether to share one big secondary-views opt-in, or have an opt-in per secondary view scenario. For example, an app might harden itself to render for "first-person observer", but if some other system asks it to render "third-person observer", it may now need to render an avatar or other annotations around the user, and it's unlikely the app is already ready to do that. However, opt-ins that are too granular limit where these platform features can be applied.

thetuvix · 2020-06-18T00:37:34Z

index.bs

+If [=secondary views=] have lower underlying frame rates, the {{XRSession}} MAY choose to do one or more of the following:
+
+ - Lower the overall frame rate of the application while the [=secondary views=] are active.
+ - Surface [=secondary views=] in the {{XRViewerPose/views}} array only for some of the frames. Implementations doing this SHOULD NOT have frames where the [=primary views=] are not present.


Toggling on/off per-frame brings up another issue about whether web apps will need to prepare when they are first told to render to a secondary view. If the app has any spin-up/spin-down (e.g. allocate/release an extra intermediate framebuffer), toggling on/off could aggravate that. Even in the steady-on case, an app may need a frame or two to get its affairs in order before actually providing a frame for that view.

I believe we don't have any way to know if the app is submitting pixels for secondary views, right? Once the UA asks the app to fill them in, it has to assume they are filled, and would end up with black pixels if not? OpenXR is one step more explicit here, having apps decide to submit any given view at the end of the frame, which gives the app frames if needed to warm up.

I believe we don't have any way to know if the app is submitting pixels for secondary views, right?

In the case of regular WebXR, yes we don't know.
For WebXR Layers, we will.

Once the UA asks the app to fill them in, it has to assume they are filled, and would end up with black pixels if not?

I believe so.

OpenXR is one step more explicit here, having apps decide to submit any given view at the end of the frame, which gives the app frames if needed to warm up.

Yes, with layers getViewSubImage indicates that that the view should be submitted. We don't have that with plain WebXR.

@thetuvix you are correct, we don't have a way of knowing in WebXR. Folks would end up with black pixels here. That might be okay for a few frames IMO.

Manishearth · 2020-06-18T17:09:38Z

@toji do you think we can land this? This approach was mostly agreed upon during the meeting AIUI.

toji · 2020-06-18T17:37:46Z

Yeah, I think we can. The only possible concern is #1085 determining that a single big opt in isn't appropriate, but I think we have backwards compatible paths available there. (For example, "secondary-views" would be a primary opt-in that auto-selects an appropriate mode, but a hypothetical "secondary-views-quad" would only enable quad-view style displays and "secondary-views-observer" would only enable observer views, etc.)

Manishearth · 2020-06-18T19:41:31Z

Sounds good! The AR module side (immersive-web/webxr-ar-module#57) also needs to be reviewed, fwiw.

Allow support for secondary views to be toggled by a pref Implements immersive-web/webxr#1083 , and moves the constant to a pref to be toggled by the user. For now I'd like to keep it a pref since there are perf implications of always enabling this.

Manishearth requested a review from toji June 12, 2020 20:40

Manishearth commented Jun 12, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

This was referenced Jun 12, 2020

Add first person observer view immersive-web/webxr-ar-module#57

Merged

Add text about extra views #1080

Closed

asajeffrey reviewed Jun 12, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

cabanier reviewed Jun 12, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

Manishearth and others added 5 commits June 12, 2020 16:44

Add feature for additional views

e14e14c

Update index.bs

1827ceb

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Update index.bs

ecc81ab

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

more fixes

7f2525b

Address comments

fe28994

Manishearth force-pushed the additional-views branch from 5cd82aa to fe28994 Compare June 12, 2020 23:59

cabanier reviewed Jun 15, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

cabanier reviewed Jun 15, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

cabanier reviewed Jun 15, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

cabanier reviewed Jun 15, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

cabanier reviewed Jun 15, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

cabanier reviewed Jun 15, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

cabanier reviewed Jun 15, 2020

View reviewed changes

Manishearth and others added 3 commits June 15, 2020 12:34

Update index.bs

90e08ce

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Update index.bs

59010e9

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Ask implementations to not suppress primary views

8e4ba00

Manishearth and others added 3 commits June 15, 2020 12:38

Update index.bs

9f3e901

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Disambiguate feature from concept

6a2337c

Make informative text a note

d7e8e24

cabanier reviewed Jun 15, 2020

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Update index.bs

2bc7f53

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

cabanier mentioned this pull request Jun 16, 2020

Add spec text to handle secondary views immersive-web/layers#167

Closed

additional views -> secondary views

c87bc10

Manishearth changed the title ~~Add feature for additional views~~ Add feature for secondary views Jun 17, 2020

cabanier reviewed Jun 17, 2020

View reviewed changes

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

Manishearth and others added 6 commits June 17, 2020 12:45

Update index.bs

99b50ac

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Update index.bs

418e5b7

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Update index.bs

c121b7b

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Update index.bs

2dffabc

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Update index.bs

d87082f

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

Update index.bs

f0c3ff3

Co-authored-by: Rik Cabanier <cabanier@gmail.com>

toji approved these changes Jun 17, 2020

View reviewed changes

thetuvix reviewed Jun 18, 2020

View reviewed changes

thetuvix mentioned this pull request Jun 18, 2020

One big secondary-views opt-in, or an opt-in per secondary view scenario? #1085

Closed

thetuvix reviewed Jun 18, 2020

View reviewed changes

Mention that views can be silently discarded to maintain frame rates

fa7637e

Manishearth merged commit 9106241 into immersive-web:master Jun 18, 2020

Manishearth deleted the additional-views branch June 18, 2020 19:41

toji mentioned this pull request Jun 18, 2020

Allow caching various objects #1086

Closed

Manishearth mentioned this pull request Jul 28, 2020

Allow support for secondary views to be toggled by a pref servo/webxr#188

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feature for secondary views #1083

Add feature for secondary views #1083

Manishearth commented Jun 12, 2020

cabanier left a comment

Manishearth commented Jun 15, 2020

Manishearth commented Jun 15, 2020

Manishearth commented Jun 16, 2020

asajeffrey commented Jun 16, 2020

Manishearth commented Jun 17, 2020

asajeffrey commented Jun 17, 2020

Manishearth commented Jun 17, 2020

cabanier left a comment

toji left a comment

toji Jun 17, 2020

cabanier Jun 18, 2020

Manishearth Jun 18, 2020

toji Jun 17, 2020

thetuvix Jun 18, 2020

Manishearth Jun 18, 2020

thetuvix Jun 18, 2020

Manishearth Jun 18, 2020

thetuvix Jun 18, 2020

cabanier Jun 18, 2020

Manishearth Jun 18, 2020

Manishearth commented Jun 18, 2020

toji commented Jun 18, 2020

Manishearth commented Jun 18, 2020


		When "[=secondary views/secondary-views=]" is enabled, the user agent MAY surface any [=secondary views=] the device supports to the {{XRSession}}. The user agent MUST NOT use reprojection to reconstruct [=secondary views=] in such a case, but instead rely on whatever the content decides to render.

		Note: We recommend content use {{XRSessionInit/optionalFeatures}} to enable "[=secondary views/secondary-views=]" to ensure maximum compatibility.

Add feature for secondary views #1083

Add feature for secondary views #1083

Conversation

Manishearth commented Jun 12, 2020

cabanier left a comment

Choose a reason for hiding this comment

Manishearth commented Jun 15, 2020

Manishearth commented Jun 15, 2020

Manishearth commented Jun 16, 2020

asajeffrey commented Jun 16, 2020

Manishearth commented Jun 17, 2020

asajeffrey commented Jun 17, 2020

Manishearth commented Jun 17, 2020

cabanier left a comment

Choose a reason for hiding this comment

toji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Manishearth commented Jun 18, 2020

toji commented Jun 18, 2020

Manishearth commented Jun 18, 2020