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: Explicitly specify precision for all sampler types #27482

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

StrandedKitty
Copy link
Contributor

@StrandedKitty StrandedKitty commented Jan 1, 2024

Description

Most of the code written using Three.js (and Three.js itself in the case of shadow mapping) doesn't use depth textures to store depth values — RGBA8 textures are used instead by storing depth in an encoded format. However, there are cases when you want to render scene depth into a depth texture directly and subsequently sample depth values from this texture.

It seems like on some Android devices if you sample a depth texture without specifing a precision qualifier in the shader code then the precision ends up to be very low. It leads to noticable artifacts which can be described as banding.

This PR adds precision qualifiers for all possible sampler types to make sure that the precision value specified when creating a WebGLRenderer gets applied to all samplers.

WebGL1 sampler types are listed here: https://www.khronos.org/files/webgl/webgl-reference-card-1_0.pdf
And WebGL2 sampler types are listed in this card: https://www.khronos.org/files/webgl20-reference-guide.pdf

See screenshots below from webgl_depth_texture example taken on Google Pixel 7a. Note much smoother depth rendering on the second screenshot.

Before (no precision highp sampler2D;) After (with precision highp sampler2D;)
Screenshot_20240101-011539 Screenshot_20240101-011506

Copy link

github-actions bot commented Jan 1, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
671 kB (166.8 kB) 671.7 kB (166.9 kB) +690 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
451.9 kB (109.8 kB) 452.6 kB (109.9 kB) +690 B

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 2, 2024

I agree it makes sense to do this. In the past, it was the task of the developer to ensure enough precision but this can easily be missed (see #27320). The resulting errors are also hard to debug.

@Mugen87 Mugen87 added this to the r161 milestone Jan 2, 2024
@Mugen87 Mugen87 merged commit 589eeb1 into mrdoob:dev Jan 5, 2024
12 checks passed
AdaRoseCannon pushed a commit to AdaRoseCannon/three.js that referenced this pull request Jan 15, 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

Successfully merging this pull request may close these issues.

2 participants