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

Add XRView to XRDepthInformation #50

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 32 additions & 11 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -185,23 +185,24 @@ The following table summarizes the ways various data formats can be consumed:
<tr>
<td>{{XRDepthDataFormat/"luminance-alpha"}}</td>
<td>[=GLenum/LUMINANCE_ALPHA=]</td>
<td>2 times 8 bit</dt>
<td>2 times 8 bit</td>
<td>Interpret {{XRCPUDepthInformation/data}} as {{Uint16Array}}</td>
<td>Inspect Luminance and Alpha channels to reassemble single value.</td>
</tr>
<tr>
<td>{{XRDepthDataFormat/"float32"}}</td>
<td>[=GLenum/R32F=]</td>
<td>32 bit</dt>
<td>32 bit</td>
<td>Interpret {{XRCPUDepthInformation/data}} as {{Float32Array}}</td>
<td>Inspect Red channel and use the value.</td>
</tr>
<tr>
<td>{{XRDepthDataFormat/"unsigned-short"}}</td>
<td>[=GLenum/R16UI=]</td>
<td>16 bit</dt>
<td>16 bit</td>
<td>Interpret {{XRCPUDepthInformation/data}} as {{Uint16Array}}</td>
<td>Inspect Red channel and use the value.</td>
<td>Inspect Red channel and use the
value.</td>
</tr>
</tbody>
</table>
Expand All @@ -213,13 +214,18 @@ Session configuration {#session-configuration}
dictionary XRDepthStateInit {
required sequence<XRDepthUsage> usagePreference;
required sequence<XRDepthDataFormat> dataFormatPreference;
boolean matchDepthView = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Github won't seem to let me add a comment there, but does https://github.com/immersive-web/depth-sensing/pull/50/files#diff-5e793325cd2bfc452e268a4aa2f02b4024dd9584bd1db3c2595f61f1ecf7b985L649 need to be updated as well? (The first part of "Native Device Concepts" talks about depth MUST be View aligned).

};
</script>

The {{XRDepthStateInit/usagePreference}} is an ordered sequence of {{XRDepthUsage}}s, used to describe the desired depth sensing usage for the session.

The {{XRDepthStateInit/dataFormatPreference}} is an ordered sequence of {{XRDepthDataFormat}}s, used to describe the desired depth sensing data format for the session.

The {{XRDepthStateInit/matchDepthView}} requests that {{XRDepthInformation/view}} of the depth information must be in sync with the {{XRView}}. If this is <code>true</code>, the {{XRSystem}} SHOULD return depth information that reflects the current frame. If this is <code>false</code>, the {{XRSystem}} MAY return depth information that was captured at an earlier point in time.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to allow an earlier point in time, we should probably add a time element to the data?


NOTE: If {{XRDepthStateInit/matchDepthView}} is <code>false</code>, the author should do the reprojection using the [=XRDepthInformation/view=] from {{XRDepthInformation}}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to provide any sample math for this? (I have some in Chromium C++)

Copy link
Member Author

Choose a reason for hiding this comment

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

We could add that as an informative note but I prefer if we updated the sample.

Copy link
Contributor

Choose a reason for hiding this comment

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

Per meeting this week, let's start with the sample.


The {{XRSessionInit}} dictionary is expanded by adding new {{XRSessionInit/depthSensing}} key. The key is optional in {{XRSessionInit}}, but it MUST be provided when [=depth-sensing=] is included in either {{XRSessionInit/requiredFeatures}} or {{XRSessionInit/optionalFeatures}}.

<script type="idl">
Expand Down Expand Up @@ -308,6 +314,10 @@ interface XRDepthInformation {
[SameObject] readonly attribute XRRigidTransform normDepthBufferFromNormView;
readonly attribute float rawValueToMeters;
readonly attribute XRView? view;

readonly attribute boolean isValid;
readonly attribute float depthNear;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these maybe snuck in here from another change? Is it possible to keep them as a separate change (Issue for them is Issue 43, though I thought we'd come to a decision that maybe we didn't need to do this?) Per offline chats with Piotr, if we are going to merge them in we'd want some math on how to use them, and he thinks there may be additional spec text changes needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

In a previous discussion, I had mentioned that I would combine them in 1 PR.
Happy to pull it into separate ones if it's more clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Per meeting this week, let's split so we can merge this, since I think more tweaks may be needed for that one.

readonly attribute float depthFar;
};
</script>

Expand All @@ -321,9 +331,11 @@ Note: if the applications intend to use the resulting depth buffer for texturing

The {{XRDepthInformation/rawValueToMeters}} attribute contains the scale factor by which the raw depth values from a [=XRDepthInformation/depth buffer=] must be multiplied in order to get the depth in meters.

The optional {{XRDepthInformation/view}} attribute contains the {{XRView}} that was active when the {{XRSystem}} calculated the {{XRDepthInformation}}. This attribute MAY be used by experiences to better align with the real world. If the {{XRDepthInformation/view}} is not provided, the user MUST assume it is the same as the one from the current {{XRFrame}}'s {{XRViewerPose}}.
The {{XRDepthInformation/depthNear}}, {{XRDepthInformation/depthFar}} and optional {{XRDepthInformation/view}} attribute contain the near and far field and the {{XRView}} respectively that were active when the {{XRSystem}} calculated the {{XRDepthInformation}}. This attribute MAY be used by experiences to better align with the real world. If the {{XRDepthInformation/view}} is not provided, the user MUST assume it is the same as the one from the current {{XRFrame}}'s {{XRViewerPose}}.

The {{XRDepthInformation/isValid}} attribute is set to <code>true</code> if the {{XRSystem}} was able to generate depth information for the current frame. It is set to <code>false</code> otherwise.
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we had the ability to simply just not return it otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

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

I found some pages that didn't handle this correctly. It takes our system multiple frames after starting or resuming a session to provide depth depth and this breaks content since it didn't check.

Copy link
Contributor

Choose a reason for hiding this comment

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

Per meeting this week, let's file bugs on those pages that break. We do currently return null in some situations today, and the risk is that sites that are following the current spec would start using invalid data without knowing it.


Each {{XRDepthInformation}} has an associated <dfn for=XRDepthInformation>view</dfn> that stores {{XRView}} from which the depth information instance was created.
Each {{XRDepthInformation}} has an associated <dfn for=XRDepthInformation>view</dfn> that reflects the {{XRView}} from which the depth information instance was created.

Each {{XRDepthInformation}} has an associated <dfn for=XRDepthInformation>depth buffer</dfn> that contains depth buffer data. Different {{XRDepthInformation}}s may store objects of different concrete types in the depth buffer.

Expand Down Expand Up @@ -503,18 +515,28 @@ When {{XRWebGLBinding/getDepthInformation(view)}} method is invoked on a {{XRWeb

In order to <dfn>create a WebGL depth information instance</dfn> given {{XRFrame}} |frame| and {{XRView}} |view|, the user agent MUST run the following steps:
1. Let |result| be a new instance of {{XRWebGLDepthInformation}}.
1. Let |time| be |frame|'s [=XRFrame/time=].
1. Initialize |time| as follows:
<dl class="switch">
<dt> If the {{XRSession}} was created with {{XRDepthStateInit/matchDepthView}} set to <code>true</code>:
<dd> Let |time| be |frame|'s [=XRFrame/time=].
<dt> Otherwise
<dd> Let |time| be time that the |device| captured the depth information.
</dl>
1. Let |session| be |frame|'s {{XRFrame/session}}.
1. Let |device| be the |session|'s [=XRSession/XR device=].
1. Let |nativeDepthInformation| be a result of querying |device|'s [=native depth sensing=] for the depth information valid as of |time|, for specified |view|, taking into account |session|'s {{XRSession/depthUsage}} and {{XRSession/depthDataFormat}}.
1. If |nativeDepthInformation| is <code>null</code>, return <code>null</code> and abort these steps.
1. If the depth buffer present in |nativeDepthInformation| meets user agent's criteria to [=block access=] to the depth data, return <code>null</code> and abort these steps.
1. Initialize |result|'s {{XRDepthInformation/isValid}} to <code>false</code>.
1. If |nativeDepthInformation| is <code>null</code>, return |result| and abort these steps.
1. If the depth buffer present in |nativeDepthInformation| meets user agent's criteria to [=block access=] to the depth data, return |result| and abort these steps.
1. If the depth buffer present in |nativeDepthInformation| meets user agent's criteria to [=limit the amount of information=] available in depth buffer, adjust the depth buffer accordingly.
1. Set |result|'s {{XRDepthInformation/isValid}} to <code>true</code>.
1. Initialize |result|'s {{XRDepthInformation/width}} to the width of the depth buffer returned in |nativeDepthInformation|.
1. Initialize |result|'s {{XRDepthInformation/height}} to the height of the depth buffer returned in |nativeDepthInformation|.
1. Initialize |result|'s {{XRDepthInformation/normDepthBufferFromNormView}} to a new {{XRRigidTransform}}, based on |nativeDepthInformation|'s [=depth coordinates transformation matrix=].
1. Initialize |result|'s {{XRWebGLDepthInformation/texture}} to an [=opaque texture=] containing the depth buffer returned in |nativeDepthInformation|.
1. Initialize |result|'s [=XRDepthInformation/view=] to |view|.
1. Initialize |result|'s {{XRDepthInformation/view}} to the {{XRView}} captured at |time|.
1. Initialize |result|'s {{XRDepthInformation/depthNear}} to the |device|'s [=native depth sensing=] near clip plane at |time|.
1. Initialize |result|'s {{XRDepthInformation/depthFar}} to the |device|'s [=native depth sensing=] far clip plane at |time|.
1. Initialize |result|'s {{XRWebGLDepthInformation/textureType}} as follows:
<dl class="switch">
<dt> If the |result|'s {{XRWebGLDepthInformation/texture}} was created with a textureType of [=XRTextureType/texture-array=]:
Expand All @@ -530,7 +552,6 @@ In order to <dfn>create a WebGL depth information instance</dfn> given {{XRFrame
<dd> Initialize |result|'s {{XRWebGLDepthInformation/imageIndex}} to <code>1</code>.
<dt> Otherwise
<dd> Initialize |result|'s {{XRWebGLDepthInformation/imageIndex}} to <code>0</code>.
</dl>
</dl>
1. Return |result|.

Expand Down
Loading