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

WebGLRenderer: add reverse-z via EXT_clip_control #29445

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

CodyJasonBennett
Copy link
Contributor

@CodyJasonBennett CodyJasonBennett commented Sep 19, 2024

Related issue: #22017 (comment)

Description

Implements a reversed depth buffer with a [0, 1] projection matrix via EXT_clip_control, which is a strict improvement over logarithmic depth where supported. A reverse depth buffer exploits the high precision range of [0, ~0.8] and inverts depth to better the distribution at distance. This is not only significantly faster than logarithmic depth (where use of gl_FragDepth disables early-z optimizations and MSAA coverage) but more accurate. See the below articles, which visualize this effect.

https://tomhultonharrop.com/mathematics/graphics/2023/08/06/reverse-z.html
https://developer.nvidia.com/blog/visualizing-depth-precision

As both a [0, 1] and reversed projection matrix would require drastic changes to sensitive code like frustum planes or raycasting, I've hidden both conversions when setting the projectionMatrix uniform in WebGLRenderer. It's possible this can be moved to a shader chunk, but any inline clip-space math must account for this mode, and we'll need to add a shader define accordingly.

The same is true for logarithmic depth, e.g., pmndrs/drei#1737. I've debated adding methods to packing chunks for these.

Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 686.03
169.81
686.87
170.11
+841 B
+300 B
WebGPU 835
223.93
835
223.93
+0 B
+0 B
WebGPU Nodes 834.5
223.81
834.5
223.81
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 462.34
111.55
463.18
111.86
+845 B
+309 B
WebGPU 531.77
143.36
531.77
143.36
+0 B
+0 B
WebGPU Nodes 488.43
133.22
488.43
133.22
+0 B
+0 B

Comment on lines +2034 to +2047
if ( capabilities.reverseDepthBuffer ) {

toReversedProjectionMatrix( camera.projectionMatrix );

}

p_uniforms.setValue( _gl, 'projectionMatrix', camera.projectionMatrix );

if ( capabilities.reverseDepthBuffer ) {

toForwardProjectionMatrix( camera.projectionMatrix );

}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I call uniformMatrix4f here somehow? Should be fast and clean.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind explaining in more detail what you want to do?

@Mugen87 Mugen87 added this to the r169 milestone Sep 19, 2024
@@ -57,6 +57,58 @@ import { WebGLUniformsGroups } from './webgl/WebGLUniformsGroups.js';
import { createCanvasElement, probeAsync, warnOnce } from '../utils.js';
import { ColorManagement } from '../math/ColorManagement.js';

function toReversedProjectionMatrix( projectionMatrix ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we put these functions into src/utils.js?


if ( capabilities.reverseDepthBuffer ) {

toForwardProjectionMatrix( camera.projectionMatrix );
Copy link
Collaborator

@Mugen87 Mugen87 Sep 19, 2024

Choose a reason for hiding this comment

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

Instead of recomputing the projection matrix, couldn't you store the previous projection matrix in a temporary variable and then copy it back? Something like:

camera.projectionMatrix.copy( currentProjectionMatrix );

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

Successfully merging this pull request may close these issues.

2 participants