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 depthNear and depthFar to XRDepthInformation #43

Open
cabanier opened this issue Oct 11, 2023 · 8 comments
Open

Add depthNear and depthFar to XRDepthInformation #43

cabanier opened this issue Oct 11, 2023 · 8 comments

Comments

@cabanier
Copy link
Member

I noticed that the IDL for XRDepthInformation in Chromium has depthNear and depthFar but they are not defined in the spec. @bialpio, should we document those?
OpenXR depth sensing also exposes them so it seems they're needed.

/agenda add depthNear and depthFar to XRDepthInformation

@probot-label probot-label bot added the agenda label Oct 11, 2023
@cabanier
Copy link
Member Author

oops. Sorry, I added those in our repo :-)
Let me follow up if they're needed.

@alcooper91
Copy link

Correlating what piotr says, I'm only seeing depthNear and depthFar defined for XrRenderState(Init), which is specced: https://immersive-web.github.io/webxr/#xrrenderstate-interface, in case you maybe had some inheritance chain somewhere.

@cabanier
Copy link
Member Author

cabanier commented Oct 26, 2023

@bialpio @alcooper91 we need to communicate the near and far clip plane that were used for the depth texture to the session. Otherwise we can't line them up perfectly.
The OpenXR API even provides the fov but I'm unsure if we need to communicate that.

@Yonet Yonet removed the agenda label Oct 31, 2023
@cabanier
Copy link
Member Author

/agenda add depthNear and depthFar to XRDepthInformation

We need to convey the same clipping planes that were used for computation of the depth texture

@probot-label probot-label bot added the agenda label Nov 28, 2023
@bialpio
Copy link
Contributor

bialpio commented Dec 12, 2023

Is it just to convey the clipping planes, or is it going to also need to change our existing text around what the values are? Here is what we currently say:

  1. For on-CPU data: "The data is stored in row-major format, without padding, with each entry corresponding to distance from the view's near plane to the users' environment, in unspecified units." - link.
  2. For on-GPU data: "Each texel corresponds to distance from the view's near plane to the users' environment, in unspecified units." - link.

If it's just to convey the clipping planes used, then I'm not too worried. Otherwise (i.e. if you actually put values into the depth buffer that represent the distance from a near plane that's different than XRView's), I think it's going to be a breaking change.

@cabanier
Copy link
Member Author

cabanier commented Dec 12, 2023

Is it just to convey the clipping planes, or is it going to also need to change our existing text around what the values are?

It is just to convey the clipping planes. We want to matches the projection matrices that are used in the immersive scene with the ones that were used to calculate the depth.

@cabanier
Copy link
Member Author

This is not resolved yet. I will publish a PR by TPAC and hopefully I can get the engineer on the line

/tpac add depth near/far + a pose

@probot-label probot-label bot added the TPAC label Sep 10, 2024
@Yonet Yonet removed the TPAC label Sep 24, 2024
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

No branches or pull requests

5 participants