fix: write-back stale-shadow edgecase #1776
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1776
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/wb-diff-stale-shadow"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:
NOP_CLEAR_BUF), copies, clears and updates done on the CPU, textures written back into their memory,WriteTexels. They go throughguest_cpu_write(or, forWriteTexels' 16-byte runs, oneguest_cpu_wroteover the texture).RX_VK_CAPTURE=0, every write of the game's:capture_makereturned before reporting and the collection did not report. The collection now reports every page it finds.gpu_buffer_bindoutside 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.
Looks good. I went through every
host->gpu_writeleft in gpu/vk: each is either insideguest_cpu_writeor is theWriteTexelsrun write that gets oneguest_cpu_wroteover the whole texture afterwards. The collection now reports every page, soRX_VK_CAPTURE=0is covered too. Theshadow_forgetin the direct-bind branch ofgpu_buffer_bindis clamped to the pool, and the lock-order inshadows_forget_writtenis 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.pygreps for the literal string "shadows hear of" on the line of each rawgpu_write, so reword that comment and the test fails. That is fine as a guard, just a bit brittle.