Allow caching various objects #1086
Conversation
|
I do not believe this will cause problems. I think the first three commits should definitely be landed, and the XRPose ones might be something we can experiment with. |
index.bs
Outdated
| @@ -1350,6 +1376,42 @@ To <dfn for="XRView">obtain the projection matrix</dfn> for a given {{XRView}} | | |||
| </div> | |||
|
|
|||
|
|
|||
| Primary and Secondary Views {#primary-and-secondary-views} | |||
There was a problem hiding this comment.
Hmm, something very weird happened here, but I fixed it
| 1. Initialize |viewport| as follows: | ||
| <dl class="switch"> | ||
| <dt>If {{XRWebGLLayer/getViewport()}} was called previously with a |view| corresponding to the same underlying [=view=], the user agent MAY:</dt> | ||
| <dd>Let |viewport| be the same {{XRViewport}} object returned by the earlier call to {{XRWebGLLayer/getViewport()}}</dd> |
There was a problem hiding this comment.
This applies to all of these, just using this one as an example. I know what's intended here, but it reads to me a bit like: "If the last time you called getViewport() you passed in the same view you're allowed to return the previously returned viewport, but if you cycle through a list of views you have to return a new viewport every time." (Does that make sense?)
I'm not sure exactly what wording would resolve the ambiguity, but I'd lean towards something along the lines of (paraphrasing):
"If view has been passed to a getViewport() call previously then the user agent MAY return the same viewport that was returned the last time getViewport() was called with |view|"
There was a problem hiding this comment.
FWIW, viewports kind of straddle the line between XRPose and XRView for me in terms of whether I think it's appropriate to re-use them or not. The broad expectation is that they shouldn't change much frame-to-frame, so comparing viewports between frames doesn't seem terribly useful, but that may not be true for all hardware.
Also, I already worry that developers may see viewports as static (which they will be on some hardware!) so they start grabbing them only once and re-using for the entire app life cycle. Object re-use may inadvertently encourage that behavior if we allow the viewport objects to be updated in place. There may be some middle ground in saying that you can return the same viewport as long as the values are unchanged, but if they update you have to return a new one, but that feels like a pretty subtle difference that won't actually deter the behavior listed above. (It WOULD give us a nice clear dividing line on what can/can't be re-used, though. Anything that returns actual values doesn't update in place, with opaque objects or those with only getter functions being free for re-use?)
| @@ -1045,12 +1051,24 @@ When this method is invoked, the user agent MUST run the following steps: | |||
| 1. Let |frame| be [=this=]. | |||
| 1. Let |session| be |frame|'s {{XRFrame/session}} object. | |||
| 1. If |frame|'s [=animationFrame=] boolean is <code>false</code>, throw an {{InvalidStateError}} and abort these steps. | |||
| 1. Let |pose| be a [=new=] {{XRViewerPose}} object in the [=relevant realm=] of |session|. | |||
| 1. Initialize |pose| as follows: | |||
There was a problem hiding this comment.
I don't think this is a reasonable candidate for re-use. Given the data the pose contains it's perfectly reasonable for developers to try and save poses from a previous frame for comparisons over time. (Calculating velocity, for example.) Yes, they could make an explicit copy in that case, but at that point we're forcing developers into some of the C-like patterns we were instructed to avoid in the WebVR days.
| 1. [=Populate the pose=] of |session|'s [=XRSession/viewer reference space=] in |referenceSpace| at the time represented by |frame| into |pose|, with <code>force emulation</code> set to <code>true</code>. | ||
| 1. If |pose| is <code>null</code> return <code>null</code>. | ||
| 1. Let |xrviews| be an empty [=/list=]. | ||
| 1. For each [=view=] |view| in the [=XRSession/viewer reference space/list of views=] on the[=XRSession/viewer reference space=] of {{XRFrame/session}}, perform the following steps: | ||
| 1. Let |xrview| be a new {{XRView}} object in the [=relevant realm=] of |session|. | ||
| 1. For each [=view=] |view| in the [=XRSession/viewer reference space/list of views=] on the [=XRSession/viewer reference space=] of {{XRFrame/session}}, perform the following steps: |
There was a problem hiding this comment.
Given that they're opaque keys used to query data from elsewhere XRViews strike me as another good candidate for re-use, modulo the same general concerns about bad developer assumptions mentioned above.
| @@ -938,7 +938,13 @@ NOTE: The <a href="https://immersive-web.github.io/layers">WebXR layers module</ | |||
| When an {{XRSession}} |session| receives updated [=viewer=] state for timestamp |frameTime| from the [=XRSession/XR device=], it runs an <dfn>XR animation frame</dfn>, which MUST run the following steps regardless of if the [=list of animation frame callbacks=] is empty or not: | |||
|
|
|||
| 1. Let |now| be the [=current high resolution time=]. | |||
| 1. Let |frame| be a [=new=] {{XRFrame}} with [=XRFrame/time=] |frameTime| and {{XRFrame/session}} |session| in the [=relevant realm=] of |session|. | |||
| 1. Initialize |frame| as follows: | |||
There was a problem hiding this comment.
So the big concern I have here is that some developer notices this behavior (for any of these objects), finds that they can pull off some "clever" trick because of it, and then their app breaks on some other UA they didn't test with because that UA chose not to apply this type of optimization.
The way I see it, it could manifest in one of two ways:
- Dev decides to hold on to first
XRFramethey receive and do all their calls off that. - Dev attaches some additional data to the
XRFrameobject and depends on the UA to preserve it for them for the lifetime of the app.
There's no way to prevent this if we start allowing object re-use. The best we can do is discourage it in the spec and call out bad behavior if we see it crop up in libraries. It's not necessarily a blocking issue, but one we need to consider carefully before allowing.
(Though, for the record, I think XRFrame is one of the better candidates for this type of re-use if we agree to it.)
|
I actually copied the verbiage from @cabanier's work on layers 😄 Let's discuss this tomorrow and perhaps agenda if necessary. |
|
Closing in favor of #1088 |
Fixes #1010
This allows the session to cache:
The restriction of cache frames being animation frames is there to make it possible to "hold on" to input poses during select events.
cc @asajeffrey @MortimerGoro @cabanier