MeshDepthMaterial: Compute depth manually rather than using gl_FragCoord.z #18696

Oletus · 2020-02-20T22:16:24Z

Some platforms compute gl_FragCoord at a lower precision which makes the manually computed value better for depth-based postprocessing effects.

The shader precisions that implementations report through the getShaderPrecisionFormat API are not reliable in practice, so the manually computed value is always used rather than trying to enable it conditionally on those platforms which have various shader precisions.

WebGL 2.0 should in principle compute gl_FragCoord at a higher precision, but this may also be affected by driver bugs on some platforms.

Improvement from this patch can be seen for example on iPad with A10 processor / iPadOS 13.3.1, when using depth based postprocessing effects on a scene with a high far/near plane ratio.

mrdoob · 2020-02-21T03:09:56Z

Hmm, would this allow distance based fog? #17316

Oletus · 2020-02-21T08:32:06Z

@mrdoob Looks like the distance-based fog is already using a custom varying rather than relying on builtins, so this has no bearing on that, even if there is some similarity.

WestLangley · 2020-02-21T18:44:52Z

Should the logdepthbuf calculations be modified, too?
Suggested nomenclature: vHighPrecisionZW ?

…ord.z Some platforms compute gl_FragCoord at a lower precision which makes the manually computed value better for depth-based postprocessing effects. The shader precisions that implementations report through the getShaderPrecisionFormat API are not reliable in practice, so the manually computed value is always used rather than trying to enable it conditionally on those platforms which have various shader precisions. WebGL 2.0 should in principle compute gl_FragCoord at a higher precision, but this may also be affected by driver bugs on some platforms. Improvement from this patch can be seen for example on iPad with A10 processor / iPadOS 13.3.1, when using depth based postprocessing effects on a scene with a high far/near plane ratio.

Oletus · 2020-02-24T12:24:49Z

I changed the variable naming. I also removed usage of gl_FragCoord.z from the logdepthbuf shader, though it doesn't make a lot of difference since it was only being used in orthographic projection mode in case EXT_frag_depth is available.

WestLangley · 2020-02-24T23:00:22Z

Should the logdepthbuf calculations be modified, too?

it doesn't make a lot of difference...

Maybe not modify logdepthbuf then? Lacking a compelling reason, I would be inclined to "leave well-enough alone"...

I'll defer to @Oletus and @Mugen87 on this PR.

Oletus · 2020-02-25T14:50:16Z

@Mugen87 do you prefer the patch with the logdepthbuf change, or do you think we'd be better off without it like @WestLangley suggested? I don't think it makes much of a difference in practice, since platforms that support EXT_frag_depth likely also have higher gl_FragCoord.z precision, and the change also only affects orthographic projection.

Mugen87 · 2020-02-25T15:00:52Z

I think the PR is okay as it is.

WestLangley · 2020-02-25T16:00:16Z

To make myself clear...

do you prefer the patch with the logdepthbuf change, or do you think we'd be better off without it like @WestLangley suggested?

Without it, since @Oletus provided no evidence of a problem with the logdepthbuf code, and in fact, provided a reason to leave the code as-is.

I'll defer to @Oletus and @Mugen87 on the rest of this PR.

Oletus · 2020-02-28T14:22:44Z

I removed the logdepthbuf change now, I agree that there's no strong enough reason to include it.

mrdoob · 2020-02-28T20:23:45Z

Thanks!

mrdoob · 2020-02-28T20:27:44Z

@mrdoob Looks like the distance-based fog is already using a custom varying rather than relying on builtins, so this has no bearing on that, even if there is some similarity.

If I remember correctly, the issue was that the fog is in frustum space, instead of based on distance:

three.js/src/renderers/shaders/ShaderChunk/fog_vertex.glsl.js

Line 4 in f57665f

fogDepth = -mvPosition.z;

We tried to change it to based on distance but we ran on precision issues and we had to revert.

#14633 #14736 #14786

Oletus · 2020-02-29T09:26:05Z

Thanks for merging this!

barnabasbartha · 2020-03-08T16:41:43Z

Is there any solution for distance based fog? AFAIK it had issues on some mobile platforms but for me it is enough if it works on desktop. Could we enable it using some feature flag until it gets sorted out?

WestLangley · 2020-03-29T00:04:30Z

/ping @Mugen87

Is there a problem with this PR that needs to be addressed?

Does this PR improve #9092?

Mugen87 · 2020-03-29T10:01:11Z

Does this PR improve #9092?

Unfortunately, I have no iOS device at hand to verify how webgl_materials_channels is rendered now. But yes, the issue mentioned here should be solved by this PR.

Oletus · 2020-03-29T12:18:53Z

I can confirm webgl_materials_channels is fixed on the iPad with A10 at least, I'd think that older iPads/iPhones probably behave the same as well.

pailhead · 2020-05-06T20:13:06Z

@Oletus

Hi!

I have a depth peeling experiment that ran on R98:

https://raw.githack.com/pailhead/three.js/depth-peel-stencil/examples/webgl_materials_depthpeel.html

I've added a block of GLSL to every surface shader that's present in the scene:

if(uLayer != 0 ){
          vec2 screenPos = gl_FragCoord.xy * uScreenSize;      
          float prevDepth = unpackRGBAToDepth(texture2D(uPrevDepthTexture,screenPos));
          if(prevDepth + uDepthOffset - gl_FragCoord.z >= 0. ){
              discard;
          }
}

And with this, i'm able to compare the gl_FragCoord.z depth, with what was encoded in the texture. What i don't understand is that this uDepthOffset is required, even though i set it to 0.0.

I've tried bumping the library, and noticed artifacts that appear when moving to R114, and it seems to be caused by this.

If you could shed some light on how this change impacts my code i'd really appreciate it.

I'm also curious, my demo never ran properly on iOS. I don't remember the discussion about this, but i know i've seen at least one issue where it was discussed. When running on an iphone, it looked like there was a serious precision mismatch between the target and the gl_FragCoord.z but i couldn't figure out more detail.

I always thought the texture is not being encoded propely (the discussion i think was about some storage format on ios), but with what this PR mentions, now i'm thinking that my gl_FragCoord.z could have actually been the culprit.

If i can figure out a way to get rid of the artifacts, the way this PR works is, i could expect iOS to just work?

Oletus · 2020-05-06T20:29:39Z

So after this change you probably shouldn't compare gl_FragCoord.z directly to the RGBA encoded depth, since they have different precision. Instead you should compute a replacement value for gl_FragCoord.z similarly to how the encoded depth is computed in this patch.

Adding a uniform set to zero shouldn't fix things either, but that might be disabling some faulty optimization in a driver.

And yes, I think there was some confusion before - some people thought that encoding to RGBA didn't work properly, even though the root cause of the issue was the poor precision of gl_FragCoord.z on some platforms, like iOS.

pailhead · 2020-05-06T20:40:38Z

Omg, this answered some year long questions. Thank you!

pailhead · 2020-05-06T20:42:14Z

Just to be sure i understand, I should expect the equality check to pass in the shader, depthA == depthB if i adjust both to be computed the same, regardless of one being piped through a texture? Or should i just figure out some small offset ?

pailhead · 2020-05-09T00:54:41Z

@Oletus i just want to send an infinite amount of gratitude your way. Just to confirm, not only that the equality check seems to work, but i can remove the +0.0 because whatever it was doing seems unnecessary now!

EliasHasle · 2020-12-06T14:34:17Z

If I remember correctly, the issue was that the fog is in frustum space, instead of based on distance:

three.js/src/renderers/shaders/ShaderChunk/fog_vertex.glsl.js

Line 4 in f57665f

fogDepth = -mvPosition.z;

We tried to change it to based on distance but we ran on precision issues and we had to revert.
#14633 #14736 #14786

(@mrdoob, @BarthaBRW)

#17592 is supposed to fix all of this, and more.

Oletus force-pushed the high-precision-z branch from dc98843 to 11a92d0 Compare February 24, 2020 12:22

WestLangley requested review from WestLangley and Mugen87 and removed request for WestLangley February 24, 2020 23:00

mrdoob added this to the r115 milestone Feb 25, 2020

Mugen87 approved these changes Feb 25, 2020

View reviewed changes

Oletus force-pushed the high-precision-z branch from 11a92d0 to fb7edb9 Compare February 28, 2020 14:22

mrdoob modified the milestones: r115, r114 Feb 28, 2020

mrdoob merged commit f57665f into mrdoob:dev Feb 28, 2020

Oletus deleted the high-precision-z branch February 29, 2020 09:25

moraxy mentioned this pull request Mar 3, 2020

shader errors in webgl_buffergeometry_instancing_lambert #18801

Closed

5 tasks

Mugen87 mentioned this pull request Mar 29, 2020

Depth pass unnecessarily low precision on Apple iPad Air 2 #9092

Closed

12 tasks

WestLangley mentioned this pull request May 6, 2020

Some kind of a depth buffer regression from 114 #19302

Closed

14 tasks

leonidaspir mentioned this pull request Aug 21, 2021

Screen depth artifacts on mobile (iOS / Android) playcanvas/engine#3197

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MeshDepthMaterial: Compute depth manually rather than using gl_FragCoord.z #18696

MeshDepthMaterial: Compute depth manually rather than using gl_FragCoord.z #18696

Oletus commented Feb 20, 2020

mrdoob commented Feb 21, 2020

Oletus commented Feb 21, 2020

WestLangley commented Feb 21, 2020

Oletus commented Feb 24, 2020

WestLangley commented Feb 24, 2020

Oletus commented Feb 25, 2020

Mugen87 commented Feb 25, 2020

WestLangley commented Feb 25, 2020 •

edited

Loading

Oletus commented Feb 28, 2020

mrdoob commented Feb 28, 2020

mrdoob commented Feb 28, 2020 •

edited

Loading

Oletus commented Feb 29, 2020

barnabasbartha commented Mar 8, 2020

WestLangley commented Mar 29, 2020

Mugen87 commented Mar 29, 2020

Oletus commented Mar 29, 2020

pailhead commented May 6, 2020 •

edited

Loading

Oletus commented May 6, 2020

pailhead commented May 6, 2020

pailhead commented May 6, 2020 •

edited

Loading

pailhead commented May 9, 2020 •

edited

Loading

EliasHasle commented Dec 6, 2020 •

edited

Loading

MeshDepthMaterial: Compute depth manually rather than using gl_FragCoord.z #18696

MeshDepthMaterial: Compute depth manually rather than using gl_FragCoord.z #18696

Conversation

Oletus commented Feb 20, 2020

mrdoob commented Feb 21, 2020

Oletus commented Feb 21, 2020

WestLangley commented Feb 21, 2020

Oletus commented Feb 24, 2020

WestLangley commented Feb 24, 2020

Oletus commented Feb 25, 2020

Mugen87 commented Feb 25, 2020

WestLangley commented Feb 25, 2020 • edited Loading

Oletus commented Feb 28, 2020

mrdoob commented Feb 28, 2020

mrdoob commented Feb 28, 2020 • edited Loading

Oletus commented Feb 29, 2020

barnabasbartha commented Mar 8, 2020

WestLangley commented Mar 29, 2020

Mugen87 commented Mar 29, 2020

Oletus commented Mar 29, 2020

pailhead commented May 6, 2020 • edited Loading

Oletus commented May 6, 2020

pailhead commented May 6, 2020

pailhead commented May 6, 2020 • edited Loading

pailhead commented May 9, 2020 • edited Loading

EliasHasle commented Dec 6, 2020 • edited Loading

WestLangley commented Feb 25, 2020 •

edited

Loading

mrdoob commented Feb 28, 2020 •

edited

Loading

pailhead commented May 6, 2020 •

edited

Loading

pailhead commented May 6, 2020 •

edited

Loading

pailhead commented May 9, 2020 •

edited

Loading

EliasHasle commented Dec 6, 2020 •

edited

Loading