Add ignoreDepthValues attribute to the XRWebGLLayer #548
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NellWaliczek
approved these changes
Feb 28, 2019
| @@ -689,6 +701,7 @@ dictionary XRWebGLLayerInit { | |||
| boolean depth = true; | |||
There was a problem hiding this comment.
Per our conversation earlier about naming confusion... Perhaps these become "depthBuffer" or "alphaEnabled"?
There was a problem hiding this comment.
I'm highly reluctant to do so because currently these match, in both verbiage and function, the arguments from WebGL context creation exactly.
This value instructs the UA to prevent the values left in the depth buffer after a frame is rendered for compositing.
469c18e to
ee49bae
Compare
|
Updated this so that it also fixes #555, given that everybody seems to agree on it's interpretation. PTAL. |
|
LGTM! Per #555 (comment), if we also expect some WebXR content to start trying out reversed-Z, I wonder if we should also specify that UAs must allow |
|
Added line to the spec allowing |
klausw
added a commit
to klausw/webxr
that referenced
this pull request
Mar 26, 2019
For screen-based input, it seems overly specific to require that the ray origin must be at the near plane. If the system knows the actual position of the screen in 3D space, i.e. for a zSpace-style display or for smartphone AR, it would be preferable to use the actual mapping of the touch point into virtual world space for this. For example, if you use touch controls to add bubbles or other particle effects to the scene, it would look most natural to have those actually originate at the touched point. As an additional complication, immersive-web#548 changed the core spec to explicitly allow reversed-Z where `depthFar` is less than `depthNear`. If the actual screen plane isn't available, it would be more consistent to use the closer of the two instead of always using `depthNear`. As far as I can tell the spec itself doesn't currently require any particular depth for `XRTargetRayMode` `screen`. See also https://github.com/immersive-web/webxr/issues/489#issuecomment-455291966 - I think it's problematic to assume that the screen plane corresponds to the near plane.
toji
pushed a commit
that referenced
this pull request
Mar 26, 2019
For screen-based input, it seems overly specific to require that the ray origin must be at the near plane. If the system knows the actual position of the screen in 3D space, i.e. for a zSpace-style display or for smartphone AR, it would be preferable to use the actual mapping of the touch point into virtual world space for this. For example, if you use touch controls to add bubbles or other particle effects to the scene, it would look most natural to have those actually originate at the touched point. As an additional complication, #548 changed the core spec to explicitly allow reversed-Z where `depthFar` is less than `depthNear`. If the actual screen plane isn't available, it would be more consistent to use the closer of the two instead of always using `depthNear`. As far as I can tell the spec itself doesn't currently require any particular depth for `XRTargetRayMode` `screen`. See also https://github.com/immersive-web/webxr/issues/489#issuecomment-455291966 - I think it's problematic to assume that the screen plane corresponds to the near plane.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #523.
This value instructs the UA to prevent the values left in the depth buffer after a frame is rendered for compositing. Slightly concerned about potential for confusion regarding the
depthandignoreDepthValuesattributes being on the same dictionary, so clarifying bikeshedding is welcome there if you have suggestions.Given the scope of the feature it felt appropriate to handle both the spec text and explainer text in the same PR. Please take a look!