-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
WebGPURenderer: Only use HalfFloatType
for the framebuffer target if necessary.
#30392
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
a9240fe
to
2b6771e
Compare
bb3206e
to
2b6771e
Compare
// to improve performance we only want to use HalfFloatType if necessary | ||
// HalfFloatType is required when a) tone mapping is used or b) the backend specifically requests it | ||
|
||
const type = ( useToneMapping || this.backend.needsHalfFloatFrameBufferTarget() ) ? HalfFloatType : UnsignedByteType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note: When using UnsignedByteType
, it is in general recommended to use a sRGB storage format for the framebuffer for better storage precision (see https://stackoverflow.com/a/47098507/5250847). However, these framebuffer formats are buggy in WebGL and do not consistently work across all platforms.
I'm not sure about this, nodes like |
I've checked the demos using the viewport textures and did not see any visual regressions so far. The precision of a 32bit color buffer should be okay as long as the colors do not exceed the I understand the considerations about quality but the performance implications of always using 64 bit color buffers is something to think about as well. Mobile devices tend to be quite bandwidth limited especially when rendering with full native resolution. If we always render into a 64bit color buffer, we will face performance issues on "weaker" mobile devices and mobile XR since these render in very high resolutions. This is also an issue in context of post processing. The WebXR integration into In any event, I'll move the PR to the
Maybe it would be good to figure out a live example that demonstrates potential precision issues. |
Maybe this is not noticeable because most examples use tone mapping or post-processing or do not have much HDR contrast, but if you do this test on The version using |
It seems like a very relevant point, I'm ok with the PR if it improves performance. Mainly because we can use it the previous way with a parameter. |
@donmccurdy Regarding the visual regression in #30392 (comment): What do you think about using a 32 bit color buffer when no tone mapping is configured and Can you imagine other use cases where this approach produces more severe visual errors? TBH, I did not expect the difference is so visible like in the modified version of I wonder if we should introduce a separate renderer flag instead of using 32 bit automatically 🤔 . The XR demos could set this flag to |
Personally I don't feel very comfortable with trying to automatically guess what precision is required in the framebuffer target, based on the presence or absence of tone mapping. A lit rendering workflow should use tone mapping to produce a picture — whether RGB values in the buffer happen to exceed [0,1] does not change this. I'd be OK with exposing an option to manage the type of that internal buffer, and/or with providing an option to skip the intermediate buffer and render directly to the WebXR render target. Tone mapping is an essential part of a lit rendering workflow, so I think it's important that tone mapping be possible on mobile and XR devices with WebGPU — even if that requires the same unfortunate limitations we have had in WebGLRenderer where alpha blending is in sRGB space. |
Closing. A flag that controls the precision of color buffers sounds indeed more appropriate. The default could be 64 bit but it could be set to 32 bit in scenarios where memory and bandwidth are limited. The flag could then be evaluated in the renderer and |
Related issue: #30387 (comment)
Description
This was found during investigating performance behavior in WebXR.
Right now,
WebGPURenderer
always creates a RGBA16F framebuffer for the beauty pass. UsingHalfFloatType
is not always required though. If we switch toUnsignedByteType
whenever possible, we can improve renderer performance especially on mobile GPUs.The idea is to identify HDR workflows by checking
toneMapping
and introduce a new method that allows backends to forceHalfFloatType
if they require it (e.g. whenoutputType
is set toHalfFloatType
).As a reminder: The internal framebuffer target is only relevant when rendering directly to screen since it is the input for the internal tone mapping and output color space conversion pass. It is not used when rendering into a render target e.g. when doing post processing.