Repository navigation
metal: size multisampled depth planes by their own swizzle blocks (3D map white wash) - #514
Merged
Merged
Conversation
A multisampled depth or stencil plane was taken to span DepthSliceBytes(extent) * samples: the single-sample 128x128 (or 256x128, 256x256) block padding times the sample count. A 64 KiB swizzle block holds 64 KiB / (bytes per texel * samples) pixels, so a 4x 1920x1080 D32 plane pads to 64x64 blocks: 33.4 MB, not 35.4 MB. Since depth surfaces are retired when another depth target is bound over their planes (#484), the overstated extent reached a single-sample depth target the title allocates right after its 4x scene depth. Binding that target retired the 4x scene depth surface, so later passes that sample scene depth (fog of war, water, atmosphere) read stale guest memory and washed the 3D map out in white and pale blue. DepthPlaneBytes(extent, bytes per texel, samples) gives the plane size with the sample count's block dimensions (equal to DepthSliceBytes for a single sample) and replaces DepthSliceBytes * samples in the depth surface overlap, retirement and mapping checks. Test: metal_utility_tests (DepthResource.mm, multisampledDepthPlane) checks the plane sizes and that a single-sample target 0x1FF0000 bytes after a 4x 1920x1080 one retires neither; it fails without this change ("Binding a depth target after a multisampled one retired it"). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This was referenced Oct 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The 3D map showed a white / pale-blue wash over terrain (unexplored hexes, water, fog). It is a regression from #484 (Metal depth, MSAA and fill-kernel correctness).
Root cause
A multisampled depth or stencil plane was sized as
DepthSliceBytes(extent) * samples, which is the single-sample 128x128 block padding multiplied by the sample count. A 64 KiB swizzle block holds64 KiB / (bytes per texel * samples)pixels, so a 4x 1920x1080 D32 plane pads to 64x64 blocks and takes 33.4 MB, not 35.4 MB.Since #484 retires depth surfaces when another depth target is bound over their planes, the overstated extent reached a single-sample depth target that the title allocates 0x1FF0000 bytes after its 4x scene depth. Binding that target retired the 4x scene-depth surface. Every later pass that samples scene depth (fog of war, water, atmosphere) then read stale guest memory and washed the map out.
Fix
DepthPlaneBytes(extent, bytesPerTexel, samples)sizes a plane with its sample count's block dimensions. For a single sample it equalsDepthSliceBytes. It replacesDepthSliceBytes * samplesin the depth-surface overlap, retirement and mapping checks.Evidence
metal_utility_testsmultisampledDepthPlanefails without the change ("Binding a depth target after a multisampled one retired it") and passes with it.Not changed
MSAA color targets still use
bytes * samples. Their layout depends on the swizzle mode, and nothing showed them over-retiring.Verification after rebase onto main (incl. #507, sync 11)
metal_utility_tests(guest-replay config): pass.replay-ref.pngin 290k px, all in the 3D character, so--expect-pngfails. The reference was dumped on a build with this bug. The pre-Upstream PR batch 1 U8b: Metal depth, MSAA and fill-kernel correctness #484 build gives the same 290k-px difference from the game frame and the same 50-480 px repeat flicker. Both capture references need refreshing after merge.Review findings
🤖 Generated with Claude Code
https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj