Skip to content

Same-pass sampling of a depth attachment reads a copy - #901

Open
nobz226 wants to merge 1 commit into
seedds:mainfrom
nobz226:fix/depth-feedback-snapshot
Open

nobz226 wants to merge 1 commit into
seedds:mainfrom
nobz226:fix/depth-feedback-snapshot

Conversation

@nobz226

@nobz226 nobz226 commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Part of #676: the flickering background rectangles. The audio half is #894.

Donkey Kong Country: Tropical Freeze draws rectangles or squares flickering over the background in levels 1-3, 2-1, 2-2 and the world 1 boss.

Two fragment shaders (976bc74475e9443b, dbd64ad1611ed4c9) sample the 1920×1080 Depth32Float buffer attached to their own pass. fb_prepare only handled colour attachments. Turning off lossless compression on that one buffer cleared the picture but cost frame rate, about 50 fps instead of 60 at the boss. DM_NVN_UNWRITTEN_DC=0 did not clear it.

fb_prepare now gives such a draw a copy of the depth buffer from the existing snapshot cache, for a single-level, single-layer 2D view, including a swizzled view of the buffer (Breath of the Wild samples its depth that way). DM_NVN_FB_DEPTH=0 turns it off.

I don't know why compression makes the direct read wrong. The link comes from these tests and the pass dump.

Checked

  • Tests: make -C dm/gfx/tests GAME=0100C1F0051B6000 check_t_feedback: 69 checks, 0 failed. It has no depth case, so this path has no test of its own.
  • Games run: Donkey Kong Country: Tropical Freeze 0100C1F0051B6000, v131072, including the world 1 boss. The player reports it looks and plays right. The worst 300-frame median was 17.4 ms, against about 20.9 ms with compression off on all textures. The log shows the snapshot taken on both shaders.
  • That run was on a host built from a fork branch based on an earlier main, not this exact rebased commit. This commit passed the test above.
  • Breath of the Wild 1.6.0 ran with this path active on an older source (a662f29e plus c692ba05), 30 fps as set, no crash or red line in that session. That shows the view path running; it does not show it was needed there.
  • Other games were not run. Any game whose draws sample their own depth attachment now pays for a copy and an extra pass break.
  • Devices and OS: M2, macOS 27.2.
  • scripts/regress.py: not run, although this touches a path every game uses.

Docs

  • The finding is in docs/reference/rendering.md and the game note, docs_symptoms.py was run, and the switch is in docs/env_switches.tsv
  • No game files, keys or data extracted from a game in the change or the attachments

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings November 1, 2026 22:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@nobz226
nobz226 force-pushed the fix/depth-feedback-snapshot branch from 3656123 to 1998e81 Compare November 1, 2026 22:57
@github-actions github-actions Bot added nvn NVN on Metal and its tests (dm/gfx/nvn, dm/gfx/tests) docs Documentation game: 0100C1F0051B6000 Donkey Kong Country: Tropical Freeze labels Nov 3, 2026
@seedds

seedds commented Nov 3, 2026

Copy link
Copy Markdown
Owner

Thanks. I tried this on current main and it conflicts with main's newer handling of depth readers: t_feedback asserts that "a draw that reads the depth attachment stays in its pass" (and the next one, which writes no depth), and with this change both fail, as does "feedback reuse". Since this PR, main ends the pass only when a later draw writes depth after a reader (g_fb_dread, #988 / #1068), so a reader that writes nothing reads the attachment in place. Could you first check whether Tropical Freeze's rectangles still show on current main? If they do, the case to cover is "in-place depth read with lossless compression", reconciled with those tests (for instance, the copy only when the depth texture is compressed and the pass wrote depth before the reader).

…Country: Tropical Freeze

Its depth reads in place give wrong tiles under Apple's lossless compression
(seedds#676): background rectangles flicker. fb_prepare gives such a draw a copy of
the depth attachment from the snapshot cache. The copy ends the pass, so it is
on by default only in that game; elsewhere a draw that only reads depth stays
in its pass, as main's depth-reader handling (seedds#988, seedds#1068) and t_feedback
expect. DM_NVN_FB_DEPTH=1/0 forces it on or off; t_feedback covers both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nobz226
nobz226 force-pushed the fix/depth-feedback-snapshot branch from 1998e81 to 587542b Compare November 4, 2026 00:18
@nobz226

nobz226 commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, you're right: with the copy on, those t_feedback checks fail, because the copy ends the pass. I've rebased onto current main and made the copy default-on only in Tropical Freeze (DM_NVN_FB_DEPTH=1/0 forces it either way). Other games keep main's in-place depth reads, and t_feedback now passes all its checks plus a new one for the copy path. I'll check whether the rectangles still show on current main without the copy and report back here. I don't see anything on main yet that covers lossless compression on in-place depth reads.

@PucaVaz

PucaVaz commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

@nobz226 a heads-up, since I'd like to build on this. In Ocarina of Time about 72 of 175 render passes a frame end because shaders that read textures through the texture pool count as depth readers (#1385; counters and an opt-in narrowing in draft #1396). The narrowing can't help there: the game registers a view of its own depth buffer in the pool those draws read. A throwaway build that never ends those passes (unsound, measurement only) took frame p50 from 33.7 to 27.4 ms on an M4.

Giving those draws a copy of the depth, the way this PR does for draws that sample the depth attachment, looks like the sound way to get that back: about 4 copies a frame instead of 72 pass ends. Pool reads would need the table entries of the depth's IDs to point at the copy, so the change is larger. I'm starting on it as a follow-up stacked on this PR, with credit. If you're already extending it to pool reads, say so and I'll test yours instead.

Drafted with Claude; reviewed by me.

@PucaVaz

PucaVaz commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

@nobz226 an update on what I said above: I built the pool-table snapshot on top of this PR (the pool draws read a copy of the depth through their tables instead of ending the pass). It's sound, but in Ocarina of Time it gains nothing over lunks's #1430, which removes the same passes without copies (frame p50 −5.0 ms for the snapshot against −5.6 ms for #1430, same run), so I'm not opening it. The numbers are on #1430. This PR stands on its own; thanks for it.

Drafted with Claude; reviewed by me.

@seedds

seedds commented Nov 10, 2026

Copy link
Copy Markdown
Owner

Main now has a general depth rule, fb_depth_rule in nvn_mtl.m (8190693, from #1917 and #1866, and b54968e). It covers two cases, in every game:

  • A draw that writes depth or stencil and samples its depth attachment through a texture slot reads a copy, taken at the first such draw since the game's last barrier and reused until the next one. A swizzled single-level 2D view is copied too.
  • A draw that samples the depth attachment after an earlier draw in its pass wrote depth or stencil, and writes none itself, ends the pass first.

A draw that samples the depth attachment first in its pass, before anything in that pass writes it, still reads it directly. If 976bc74475e9443b and dbd64ad1611ed4c9 are in one of the two cases above, the rectangles should be gone on main. If they are in the third, main doesn't cover them, and that would be worth finding out.

Note also that DM_NVN_FB_DEPTH is now main's switch for this rule (0 turns it off), so this PR's switch of the same name conflicts with it.

Could you run Tropical Freeze on current main at levels 1-3, 2-1, 2-2 and the world 1 boss? Please post:

  • whether the rectangles show;
  • a pass dump (DM_CTL passdump 1) of one frame where they do, so the depth-test/write state of those draws and of the earlier draws in their pass is visible;
  • whether the log has same-pass depth sampling: copy #1 or pass end #1.

I don't have the game on this side to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation game: 0100C1F0051B6000 Donkey Kong Country: Tropical Freeze nvn NVN on Metal and its tests (dm/gfx/nvn, dm/gfx/tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants