Add RenderTarget depthResolveMode controlling multisampled depth resolve - #9226
Conversation
Public API reportThis PR changes the public API surface (+6 / −1), per the docs' rules (@ignore / @Private / undocumented are excluded). Show API diff-RenderTarget.constructor(options?: { autoResolve: boolean; colorBuffer: Texture; colorBuffers: Texture[]; depth: boolean; depthBuffer: Texture; face: number; mipLevel: number; name: string; origin: string; resolveBuffer: Texture | null; resolveBuffers: Texture | null[]; samples: number; stencil: boolean; transientColor: boolean; transientDepth: boolean })
+RenderTarget.constructor(options?: { autoResolve: boolean; colorBuffer: Texture; colorBuffers: Texture[]; depth: boolean; depthBuffer: Texture; depthResolveMode: string; face: number; mipLevel: number; name: string; origin: string; resolveBuffer: Texture | null; resolveBuffers: Texture | null[]; samples: number; stencil: boolean; transientColor: boolean; transientDepth: boolean })
+RenderTarget.get depthResolveMode(): string
+RenderTarget.set depthResolveMode(value: string)
+const DEPTHRESOLVE_MAX: "max"
+const DEPTHRESOLVE_MIN: "min"
+const DEPTHRESOLVE_SAMPLE0: "sample0"Informational only — this never fails the build. |
Build size reportThis PR changes the size of the minified bundles.
|
mvaligursky
left a comment
There was a problem hiding this comment.
Automated PR review by Codex (GPT-5) at exact head 3ef58df615f34b9d79f8f40d6ffb3adffb043364.
No actionable issues found. I reviewed the public constants and RenderTarget API, the intentional default behavior change, all shader-based resolve call paths (scene depth grab, depth copy, and automatic resolve), user-provided versus internally allocated MSAA depth attachments, WGSL reduction logic, direction of min/max depth semantics, runtime mode switching, shader/pipeline cache keys and cleanup, destination formats, performance, generated types, tests, CI, and current discussion.
Verification performed:
- Full unit suite: 2,429 passing, 2 pending
- Focused render-target/WebGPU resolver tests: 40 passing
- Changed-file ESLint: clean
- Type generation and type tests: passing
- Deployed
ground-fogMSAA depth-grab path: rendered successfully on WebGPU with no relevant shader, pipeline, or validation errors - All required CI and deployment checks: green
Residual test risk: the automated resolver tests verify the preprocessed WGSL for all three modes, but do not numerically render and read back mixed-sample pixels on a real GPU. The PR description reports that numerical WebGPU coverage was performed manually; the deployed default MIN path also compiled and ran successfully during this review.
Adds control over how the samples of a multisampled depth buffer are resolved into a single depth value by the WebGPU shader-based depth resolve - used by the depth grab pass (
sceneDepthMap), a depthRenderTarget#copy, and the automatic resolve into a provideddepthBuffer.Behavior change: the default resolve changes from sample 0 to
DEPTHRESOLVE_MIN. Sample 0 is an arbitrary sample position - at geometry silhouettes it lands on the foreground or background essentially at random per pixel, causing edge noise in depth-consuming effects such as soft particles. MIN (the nearest surface, with a standard depth buffer) is deterministic and conservative, and matches what depth consumers such as Hi-Z occlusion expect.DEPTHRESOLVE_SAMPLE0remains available to restore the previous behavior.Changes:
DEPTHRESOLVE_MIN(default),DEPTHRESOLVE_MAXandDEPTHRESOLVE_SAMPLE0, and a mutableRenderTarget#depthResolveModeproperty with a matching constructor option. WebGPU only - on WebGL2 the depth resolve sample selection is defined by the implementation and the property is ignored.WebgpuResolversupports all three modes, with shaders and pipelines lazily cached per mode and format. Its WGSL source is extracted into a shader chunk (shader-chunks/frag/webgpu-depth-resolve.js), with the mode selected by aDEPTH_RESOLVE_***define handled by the shader preprocessor - the first of the internal device-level renderers to move its shader to the platform chunks.copyRenderTargetwhen copying depth from a render target with a user-provideddepthBufferandsamples> 1 - the multisampled depth of such a target is stored separately from the internally-allocated case, and the copy read a null texture.Verified on WebGPU (Metal): a known-depth scene rendered into an MSAA target and resolved with each mode - at all 100 mixed-sample edge pixels, MIN returned the near surface depth, MAX the far clear value, and SAMPLE0 a value within that range; mode switching at runtime works. The
ground-fogandparticles-snowexamples exercise the new default through the MSAA depth grab.Checklist