fix: write-back stale-shadow edgecase #1776

Merged
ShroomKing merged 1 commit from fix/wb-diff-stale-shadow into main 2026-11-10 17:16:05 +00:00
ShroomKing commented 2026-11-10 16:26:22 +00:00 (Migrated from github.com)

The write-back diff can leave a wrong value in the game's memory when something writes there without the diff hearing of it. The game reads that memory back for its own decisions (the counters behind its light and room conditions), so a stale value there can turn into wrong lighting.

While i was not able to reproduce the problem i showed in #1109 this should prevent anything like that from happening. @nickyfactz can you rerun your tests?

What changed

Four kinds of write went unreported; each is reported now:

  • The replay's own CPU writes of guest memory: the in-stream clears of 256 bytes and under (NOP_CLEAR_BUF), copies, clears and updates done on the CPU, textures written back into their memory, WriteTexels. They go through guest_cpu_write (or, for WriteTexels' 16-byte runs, one guest_cpu_wrote over the texture).
  • With RX_VK_CAPTURE=0, every write of the game's: capture_make returned before reporting and the collection did not report. The collection now reports every page it finds.
  • With captures, pages written after a submission's capture: also reported by the collection now.
  • The GPU's own writes past the mirror: a written binding given a mirrored pool's guest buffer directly (gpu_buffer_bind outside a recording's uploads) now forgets the shadow's pages, which are read again at the next write-back.

The 600-submit "write-back diff" log line now counts, per source, the pages the reports made the shadows read again ("N in captures, N only in the collection, N only by the replay itself").

The framerate has not been affected by these changes.

The write-back diff can leave a wrong value in the game's memory when something writes there without the diff hearing of it. The game reads that memory back for its own decisions (the counters behind its light and room conditions), so a stale value there can turn into wrong lighting. While i was not able to reproduce the problem i showed in #1109 this should prevent anything like that from happening. @nickyfactz can you rerun your tests? ## What changed Four kinds of write went unreported; each is reported now: - **The replay's own CPU writes of guest memory**: the in-stream clears of 256 bytes and under (`NOP_CLEAR_BUF`), copies, clears and updates done on the CPU, textures written back into their memory, `WriteTexels`. They go through `guest_cpu_write` (or, for `WriteTexels`' 16-byte runs, one `guest_cpu_wrote` over the texture). - **With `RX_VK_CAPTURE=0`, every write of the game's**: `capture_make` returned before reporting and the collection did not report. The collection now reports every page it finds. - **With captures, pages written after a submission's capture**: also reported by the collection now. - **The GPU's own writes past the mirror**: a written binding given a mirrored pool's guest buffer directly (`gpu_buffer_bind` outside a recording's uploads) now forgets the shadow's pages, which are read again at the next write-back. The 600-submit "write-back diff" log line now counts, per source, the pages the reports made the shadows read again ("N in captures, N only in the collection, N only by the replay itself"). The framerate has not been affected by these changes.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 16:41:54 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. I went through every host->gpu_write left in gpu/vk: each is either inside guest_cpu_write or is the WriteTexels run write that gets one guest_cpu_wrote over the whole texture afterwards. The collection now reports every page, so RX_VK_CAPTURE=0 is covered too. The shadow_forget in the direct-bind branch of gpu_buffer_bind is clamped to the pool, and the lock-order in shadows_forget_written is unchanged (pools_mu, then shadow_written_mu). The stale-shadow test models the sequence from #1109 and checks it against whole copies, and it is registered in run_all.

CI: the only failure is nx2/test_nup_stand_in.py (a lambda arity TypeError). The PR doesn't touch that file, so it looks like a main breakage. The new wb_diff tests pass on both OSes.

Nit: test_wb_diff_cpu_writes.py greps for the literal string "shadows hear of" on the line of each raw gpu_write, so reword that comment and the test fails. That is fine as a guard, just a bit brittle.

Looks good. I went through every `host->gpu_write` left in gpu/vk: each is either inside `guest_cpu_write` or is the `WriteTexels` run write that gets one `guest_cpu_wrote` over the whole texture afterwards. The collection now reports every page, so `RX_VK_CAPTURE=0` is covered too. The `shadow_forget` in the direct-bind branch of `gpu_buffer_bind` is clamped to the pool, and the lock-order in `shadows_forget_written` is unchanged (pools_mu, then shadow_written_mu). The stale-shadow test models the sequence from #1109 and checks it against whole copies, and it is registered in run_all. CI: the only failure is `nx2/test_nup_stand_in.py` (a lambda arity TypeError). The PR doesn't touch that file, so it looks like a main breakage. The new wb_diff tests pass on both OSes. Nit: `test_wb_diff_cpu_writes.py` greps for the literal string "shadows hear of" on the line of each raw `gpu_write`, so reword that comment and the test fails. That is fine as a guard, just a bit brittle.
Commenting is not possible because the repository is archived.
No description provided.