RenderTarget: Add resolveDepthBuffer.
#28170
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
src/core/RenderTarget.js
Outdated
| @@ -34,6 +34,7 @@ class RenderTarget extends EventDispatcher { | |||
| minFilter: LinearFilter, | |||
| depthBuffer: true, | |||
| stencilBuffer: false, | |||
| ignoreDepthValues: false, | |||
There was a problem hiding this comment.
Is this an undocumented property that users do not set? Perhaps it should it be "private" in some form.
ignoreDepthValues: false
Also, I am not in fan of double-negatives. Maybe _resolveDepthBuffer, or something similar.
There was a problem hiding this comment.
I'm going to add a documentation. Similar to resolveStencilBuffer this property is for advanced use cases.
I've picked ignoreDepthValues as a name so it matches the same property of the WebXR spec.
https://developer.mozilla.org/en-US/docs/Web/API/XRWebGLLayer/ignoreDepthValues
The property does not just control the MSAA resolve. The ignore also means the respective depth buffers are invalidated (if the browser supports framebuffer invalidation).
There was a problem hiding this comment.
We also need to evaluate this property in WebGPURenderer at some point. Not having a property in RenderTarget means redundant code snippets like the following will be spread in both renderers multiple times.
const renderTargetProperties = properties.get( renderTarget );
const ignoreDepthValues = ( renderTargetProperties.__ignoreDepthValues !== undefined ) ? renderTargetProperties.__ignoreDepthValues : false;There was a problem hiding this comment.
Besides, the property does not just control the MSAA resolve. The ignore also means the respective depth buffers are invalidated (if the browser supports framebuffer invalidation).
On the other hand, the framebuffer invalidation is some sort of clean up that only happens in the MSAA codepath now. Um, I'll add a commit with resolveDepthBuffer for comparison.
There was a problem hiding this comment.
@WestLangley Looking better now?
There was a problem hiding this comment.
@Mugen87 I like that better, thanks.
Let's leave this review section open for now, so others can see your explanations. 👍
Mugen87
changed the title
ignoreDepthValues.resolveDepthBuffer.
Related issue: #28132
Description
This PR fixes a bug in the transmission implementation.
ignoreDepthValueswas a stored in the render target properties so far. When a render target gets resized though, the properties are deleted becausedispose()is internally called so the flag is lost. This actually happens inrenderTransmissionPass()when the transmission render target gets resized.Since
ignoreDepthValuesis also used in the WebXR space, it seems reasonable to add it asresolveDepthBufferto theRenderTargetclass./cc @mrxz