-
Notifications
You must be signed in to change notification settings - Fork 419
add support for XRVisibilityMaskChangeEvent #1412
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1238,16 +1238,19 @@ When this method is invoked, the user agent MUST run the following steps: | |
| 1. [=Populate the pose=] of |session|'s [=XRSession/viewer reference space=] in |referenceSpace| at the time represented by |frame| into |pose|, with `force emulation` set to `true`. | ||
| 1. If |pose| is `null` return `null`. | ||
| 1. Let |xrviews| be an empty [=/list=]. | ||
| 1. Let |offset| be `0`. | ||
| 1. For each [=view/active=] [=view=] |view| in the [=XRSession/list of views=] on {{XRFrame/session}}, perform the following steps: | ||
| 1. Let |xrview| be a new {{XRView}} object in the [=relevant realm=] of |session|. | ||
| 1. Initialize |xrview|'s [=XRView/underlying view=] to |view|. | ||
| 1. Initialize |xrview|'s {{XRView/eye}} to |view|'s [=view/eye=]. | ||
| 1. Initialize |xrview|'s {{XRView/index}} to |offset|. | ||
| 1. Initialize |xrview|'s [=XRView/frame=] to |frame|. | ||
| 1. Initialize |xrview|'s [=XRView/session=] to |session|. | ||
| 1. Initialize |xrview|'s [=view/reference space=] to |referenceSpace|. | ||
| 1. Let |offset| be an [=new=] {{XRRigidTransform}} object equal to the [=view offset=] of |view| in the [=relevant realm=] of |session|. | ||
| 1. Set |xrview|'s {{XRViewGeometry/transform}} property to the result of [=multiply transforms|multiplying=] the {{XRViewerPose}}'s {{XRPose/transform}} by the |offset| transform in the relevant realm of |session| | ||
| 1. [=list/Append=] |xrview| to |xrviews| | ||
| 1. Let |viewtransform| be an [=new=] {{XRRigidTransform}} object equal to the [=view offset=] of |view| in the [=relevant realm=] of |session|. | ||
| 1. Set |xrview|'s {{XRViewGeometry/transform}} property to the result of [=multiply transforms|multiplying=] the {{XRViewerPose}}'s {{XRPose/transform}} by the |viewtransform| transform in the relevant realm of |session|. | ||
| 1. [=list/Append=] |xrview| to |xrviews|. | ||
| 1. Increase |offset| by `1`. | ||
| 1. Set |pose|'s {{XRViewerPose/views}} to |xrviews| | ||
| 1. Return |pose|. | ||
|
|
||
|
|
@@ -1574,6 +1577,7 @@ enum XREye { | |
|
|
||
| [SecureContext, Exposed=Window] interface XRView { | ||
| readonly attribute XREye eye; | ||
| readonly attribute unsigned long index; | ||
| readonly attribute double? recommendedViewportScale; | ||
|
|
||
| undefined requestViewportScale(double? scale); | ||
|
|
@@ -1586,6 +1590,8 @@ The {{XRViewGeometry/transform}} is given in it's [=view/reference space=]. | |
|
|
||
| The <dfn attribute for="XRView">eye</dfn> attribute describes the [=view/eye=] of the underlying [=view=]. This attribute's primary purpose is to ensure that pre-rendered stereo content can present the correct portion of the content to the correct eye. | ||
|
|
||
| The <dfn attribute for="XRView">index</dfn> attribute describes the offet of this {{XRView}} when it is return in the {{XRViewerPose/views}} array by {{XRFrame/getViewerPose()}}. | ||
|
|
||
| The optional <dfn attribute for="XRView">recommendedViewportScale</dfn> attribute contains a UA-recommended viewport scale value that the application can use for a {{XRView/requestViewportScale()}} call to configure dynamic viewport scaling. It is `null` if the system does not implement a heuristic or method for determining a recommended scale. If not null, the value MUST be a numeric value greater than 0.0 and less than or equal to 1.0, and MUST be [=quantization|quantized=] to avoid providing detailed performance or GPU utilization data. | ||
|
|
||
| Note: It is suggested to quantize the recommended viewport scale by rounding it to the nearest value from a short list of possible scale values, and using hysteresis to avoid instant changes when close to a boundary value. (This also helps avoid rapidly oscillating scale values which can be visually distracting or uncomfortable.) | ||
|
|
@@ -2589,6 +2595,51 @@ The optional <dfn attribute for="XRReferenceSpaceEvent">transform</dfn> attribut | |
|
|
||
| NOTE: situations where {{XRReferenceSpaceEvent/referenceSpace}} or {{XRReferenceSpaceEventInit/referenceSpace}} can be when the headset was doffed and donned between 2 seperate locations. In such cases, if the experience relies on world-locked content, it should warn the user and reset the scene. | ||
|
|
||
| XRVisibilityMaskChangeEvent {#xrvisibilitymaskchangeevent-interface} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed in the other thread, we must have code in `requestSession() (step 5.4.11) that guarantees that this event is fired on session creation. Otherwise there is no way to get the initial mask. Also, if we aren't going to have an attribute for this I would like docs to prominently state that this is the behavior everywhere.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mask is not known at session creation (at least on Quest) so we can't fire that event.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps what is needed then is at least a note to indicate that the events will always fire after the session request is resolved, so users that want to ensure that they get the event should always listen for it in the resolve callback.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
| -------------- | ||
|
|
||
| Because the frustum does not intersect precisely with the rectangular display, the entire area of the {{XRLayer}} MAY not be displayed. This event will inform the experience about the region of the {{XRView}} that is displayed to the user. | ||
|
|
||
| The {{XRVisibilityMaskChangeEvent}} event is fired when the user agent wants to inform the experience that the displayed area of the {{XRLayer}} changed. The experience MAY choose to only draw that region which MAY help with performance. | ||
|
|
||
| NOTE: the experience MUST register this event during the promise resolution of {{XRSystem/requestSession}}. Otherwise, the event may fire and the mask will be lost to the experience. | ||
|
|
||
| <pre class="idl"> | ||
| [SecureContext, Exposed=Window] | ||
| interface XRVisibilityMaskChangeEvent : Event { | ||
| constructor(DOMString type, XRVisibilityMaskChangeEventInit eventInitDict); | ||
| [SameObject] readonly attribute XRSession session; | ||
| readonly attribute XREye eye; | ||
| readonly attribute unsigned long index; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the intent that the eye and the index together make a unique key, or is the index alone enough to uniquely identify the view? The added prose above suggested that the index is only unique per eye. If so, I'm not really clear on what the benefit is to doing it that way? Seems like simply using a unique index/ID would be easier for devs using it to cache things like meshes. Alternatively, maybe I'm just reading it wrong and there's a benefit to having the eye explicitly given here that I'm not aware of? (Especially since you wouldn't be able to get the eye otherwise till a ViewPose was queried.)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I made it unique per eye. I can change it to drop that so we match OpenXR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated the PR. Please take another look |
||
| [SameObject] readonly attribute Float32Array vertices; | ||
| [SameObject] readonly attribute Uint32Array indices; | ||
| }; | ||
|
|
||
| dictionary XRVisibilityMaskChangeEventInit : EventInit { | ||
| required XRSession session; | ||
| required XREye eye; | ||
| required unsigned long index; | ||
| required Float32Array vertices; | ||
| required Uint32Array indices; | ||
| }; | ||
| </pre> | ||
|
|
||
| The <dfn attribute for="XRVisibilityMaskChangeEvent">session</dfn> attribute indicates the {{XRSession}} that generated the event. | ||
|
|
||
| The <dfn attribute for="XRVisibilityMaskChangeEvent">eye</dfn> attribute indicates which {{XREye}} the mask applies to. | ||
cabanier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| The <dfn attribute for="XRVisibilityMaskChangeEvent">index</dfn> attribute indicates the offset into the [=XRSession/list of views=] of the {{XRView}} that this mask applies to. | ||
|
|
||
| The <dfn attribute for="XRVisibilityMaskChangeEvent">vertices</dfn> attribute is a [=/list=] of <code>X</code>, <code>Y</code> coordinates. The experience MUST assume that the <code>Z</code> coordinate is <code>-1</code>. Each <code>X</code>, <code>Y</code>, <code>Z</code> coordinate describes a vertex. | ||
| If this array is empty, the whole region of the {{XRView}} SHOULD be drawn. | ||
|
|
||
| The <dfn attribute for="XRVisibilityMaskChangeEvent">indices</dfn> attribute is a [=/list=] of indices that describe the index into the vertex list described by {{XRVisibilityMaskChangeEvent/vertices}}. These indices will describe the region of the eye's {{XRView}} that SHOULD be drawn. | ||
| If this array is empty, the whole region of the {{XRView}} SHOULD be drawn. | ||
|
|
||
| The area MUST be drawn using the {{XRViewGeometry/projectionMatrix}} of the {{XRView}} of {{XRVisibilityMaskChangeEvent/eye}} and a default {{XRRigidTransform}}. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a comment on this proposal, just a general observation: I didn't realize that OpenXR's visibility mask required transformation by the projection matrix! I would have thought that reporting the points in NDC would be easier all around. Wonder why they went that route.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this made it easier to integrate into rendering pipelines? Maybe @thetuvix knows since he's listed as one of the authors of this extension. |
||
|
|
||
| NOTE: this means that the area MUST NOT use the {{XRRigidTransform}} of the current {{XRView}} of {{XRVisibilityMaskChangeEvent/eye}}. | ||
|
|
||
| Event Types {#event-types} | ||
| ----------- | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indexsuggests to me that this would be analogous to the index of theXRViewerPose.views[]array that this view is located at, and even if that wasn't explicit I can see implementations naturally having them line up anyway, giving the impression that they're the same.Feels like if this is guaranteed to always align with the array index then we should let it be implicit. If it's not intended to align with the array index then we should call it something like
viewIdand potentially state that they're intended to be unique across the entire session. Maybe even enforce that they can't have an ID of 0 so that there's no ambiguity about the index alignment.(Also, if what I was wondering about below is true and it's a per-eye index, the name should reflect that.
eyeViewIndexor similar.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having it match the index in the views array sgtm.
I'll update the spec.