Keep the CPU's newer pages when a remade pool's mirror inherits a retired one #1127

2026-11-09T13:31:10Z

duck-paradise commented

2026-11-09 13:31:10 +00:00

(Migrated from github.com)

Summary

A pool remade larger gets a new mirror: mirror_make copies guest memory into it whole, then copies each retired pool's mirror over its whole part, so that what the GPU wrote there carries over. A page the CPU wrote after the retired mirror's last sync is newer in guest memory. The old mirror's copy of that page went over the fresh one, and the new mirror then counted it as synced, so it stayed stale until the game wrote that page again.

OoT writes its light probes once, when an area comes in. After the pool holding them was remade larger (vk: pool 0x1089000000..0x1091000000: mirror made with the GPU's contents of 1 retired pool(s) in the log), the fog's compute pass (27ee9883e2a20323, its read-only c0+0x880 buffer) read the old area's probes for the rest of the session. On screen that is froxel-shaped blocks over the fog: white glow, black blocks over the sky, or magenta/red/green blocks, together with abnormally dark shadows. It shows after 20-60 minutes of play. A save reload does not clear it, a restart does. This looks like part of #993 and like #1120 (9070 XT, after 2 h+).

Change

gpu/vk/mirror_inherit.h: the rule. Only the pages not stamped since the retired mirror's last sync (page_gen <= r.synced_gen, clean 2 MB regions in one step) are copied from the retired mirror, merged into runs. The other pages keep guest memory's copy, which the full copy just put there. What the GPU wrote before that sync still carries over, as before.
gpu/vk/vk_memory.cpp: mirror_make copies through the rule and logs, 40 times, how much it kept from guest memory instead. RX_VK_INHERIT_ALL=1 restores the old whole-part copy.
tests/test_mirror_inherit.py + .cpp: unit test of the rule: clean parts, a newer page splitting a run, pages at the sync's generation, unaligned parts, clean regions. It also checks the source still goes through it.

How it was tested

Hardware: RX 9070 XT (16 GB), Windows 11, AMD driver 0x8001a0. Local build on top of v2.0.8-rc.1 plus my fork's AMD changes; this PR carries only the fix, rebased on main.
Healthy frame: dumped with RX_VK_DUMP_FILE and RX_VK_DUMP_SSBO=27ee9883e2a20323,1e2fcee33a0c7209. The probe buffer's GPU copy matched guest memory word for word (0 of 4,194,304 words differ).
Broken frame (black blocks over the sky): 3,608,923 of 4,194,304 words (86%) differed. The light list next to it matched. The log had the pool remade with an inherited mirror shortly before.
Before the fix: the blocks or glow came back in every session over about 30 minutes (5 sessions; Hyrule Field, Lon Lon Ranch, Death Mountain, Dodongo's Cavern).
With the fix:
- One session so far, about an hour across the same areas, with no glow, blocks or dark shadows.
- The new log line fired 21 times, for example pool 0x117c000000..0x117e000000: 2436 KB the CPU wrote since retired pool 0x117d000000's last sync kept from guest memory, not its mirror.
- No fps change I could see.
- I'll add to this PR if it turns up again over longer play.
Tests: python -I -B tests/test_mirror_inherit.py passes (g++ 10). It fails on main without the change.

Refs #993, #1120

🤖 Generated with Claude Code

## Summary A pool remade larger gets a new mirror: `mirror_make` copies guest memory into it whole, then copies each retired pool's mirror over its whole part, so that what the GPU wrote there carries over. A page the CPU wrote **after** the retired mirror's last sync is newer in guest memory. The old mirror's copy of that page went over the fresh one, and the new mirror then counted it as synced, so it stayed stale until the game wrote that page again. OoT writes its light probes once, when an area comes in. After the pool holding them was remade larger (`vk: pool 0x1089000000..0x1091000000: mirror made with the GPU's contents of 1 retired pool(s)` in the log), the fog's compute pass (`27ee9883e2a20323`, its read-only `c0+0x880` buffer) read the old area's probes for the rest of the session. On screen that is froxel-shaped blocks over the fog: white glow, black blocks over the sky, or magenta/red/green blocks, together with abnormally dark shadows. It shows after 20-60 minutes of play. A save reload does not clear it, a restart does. This looks like part of #993 and like #1120 (9070 XT, after 2 h+). ## Change - **`gpu/vk/mirror_inherit.h`:** the rule. Only the pages **not stamped since the retired mirror's last sync** (`page_gen <= r.synced_gen`, clean 2 MB regions in one step) are copied from the retired mirror, merged into runs. The other pages keep guest memory's copy, which the full copy just put there. What the GPU wrote before that sync still carries over, as before. - **`gpu/vk/vk_memory.cpp`:** `mirror_make` copies through the rule and logs, 40 times, how much it kept from guest memory instead. `RX_VK_INHERIT_ALL=1` restores the old whole-part copy. - **`tests/test_mirror_inherit.py` + `.cpp`:** unit test of the rule: clean parts, a newer page splitting a run, pages at the sync's generation, unaligned parts, clean regions. It also checks the source still goes through it. ## How it was tested - **Hardware:** RX 9070 XT (16 GB), Windows 11, AMD driver `0x8001a0`. Local build on top of v2.0.8-rc.1 plus my fork's AMD changes; this PR carries only the fix, rebased on `main`. - **Healthy frame:** dumped with `RX_VK_DUMP_FILE` and `RX_VK_DUMP_SSBO=27ee9883e2a20323,1e2fcee33a0c7209`. The probe buffer's GPU copy matched guest memory word for word (0 of 4,194,304 words differ). - **Broken frame (black blocks over the sky):** 3,608,923 of 4,194,304 words (86%) differed. The light list next to it matched. The log had the pool remade with an inherited mirror shortly before. - **Before the fix:** the blocks or glow came back in every session over about 30 minutes (5 sessions; Hyrule Field, Lon Lon Ranch, Death Mountain, Dodongo's Cavern). - **With the fix:** - One session so far, about an hour across the same areas, with no glow, blocks or dark shadows. - The new log line fired 21 times, for example `pool 0x117c000000..0x117e000000: 2436 KB the CPU wrote since retired pool 0x117d000000's last sync kept from guest memory, not its mirror`. - No fps change I could see. - I'll add to this PR if it turns up again over longer play. - **Tests:** `python -I -B tests/test_mirror_inherit.py` passes (g++ 10). It fails on `main` without the change. Refs #993, #1120 🤖 Generated with [Claude Code](https://claude.com/claude-code)

duck-paradise commented

2026-11-09 13:37:14 +00:00

(Migrated from github.com)

CI note: the one failing check, tests/test_program_answer_domains.py (answers_in undeclared in its fixture), fails the same way on main at 8b2e1d3 (run 42588468881), so it is not from this change. This PR's own test passes on both Ubuntu and Windows.

CI note: the one failing check, `tests/test_program_answer_domains.py` (`answers_in` undeclared in its fixture), fails the same way on `main` at 8b2e1d3 (run 42588468881), so it is not from this change. This PR's own test passes on both Ubuntu and Windows.

Nitnek commented

2026-11-09 15:52:34 +00:00

(Migrated from github.com)

Tested on NVIDIA (RTX 3070, Windows 11), merged onto v2.0.8-rc.5, with the scripted castle-load run.

The rule runs here: pool 0x1065000000..0x106a000000: 40504 KB the CPU wrote since retired pool …'s last sync kept from guest memory, not its mirror. There were 13 such lines on one save load, from 52 KB to 40 MB.
The picture matches rc.5. No device lost.
Frame times are the same as rc.5: p50 17.8 ms, p99 36.9 ms, load hitch 815 ms.
A short run cannot show the 20-60 min fog blocks; I can do a long play test later.

Tested on NVIDIA (RTX 3070, Windows 11), merged onto v2.0.8-rc.5, with the scripted castle-load run. - The rule runs here: `pool 0x1065000000..0x106a000000: 40504 KB the CPU wrote since retired pool …'s last sync kept from guest memory, not its mirror`. There were 13 such lines on one save load, from 52 KB to 40 MB. - The picture matches rc.5. No device lost. - Frame times are the same as rc.5: p50 17.8 ms, p99 36.9 ms, load hitch 815 ms. - A short run cannot show the 20-60 min fog blocks; I can do a long play test later.

duck-paradise commented

2026-11-09 16:44:04 +00:00

(Migrated from github.com)

Thanks a lot for testing this on NVIDIA! Thirteen of those lines on a single save load is a useful data point: the remade-pool case is common on NVIDIA too, not only on AMD. On my RX 9070 XT, the fog blocks, glow and dark shadows have not come back in the sessions since the fix, a few hours of play so far. A long play test on your side would be great whenever you have time.

hckrman101 (Migrated from github.com) reviewed

2026-11-09 18:54:20 +00:00

hckrman101 (Migrated from github.com) left a comment

Nice find, and the diagnosis checks out. Taking a retired mirror's page only when page_gen <= r.synced_gen gives the same result as "inherit everything, then run mirror_sync from r.synced_gen", which is the right invariant. Pages the CPU wrote after the old mirror's last sync come from the full guest copy, and the capture pass after it still puts captured pages on top as they were at their submit, just like mirror_sync does. What I checked:

runs(): the clean-region shortcut only applies on region-aligned pages, partial first/last pages stay clamped to [a, b), runs merge correctly, and the dst ranges never overlap, so the multi-region vkCmdCopyBuffer is fine.
GPU writes in earlier submissions are stamped while they're recorded, and memory_end_submission then raises synced_gen past those stamps, so they're still inherited. The barrier/retired_value handling didn't change.
In serial replay, each recording is its own submission with its upload buffer first, so a draw in the same recording never reached the inherit copy before this PR either. Nothing gets worse there.
Locking matches mirror_sync (under pools_mu, atomic page_gen, plain region_gen read).
The base is 159 commits behind main, but the mirror_make hunk is unchanged there and the PR is mergeable. test_mirror_inherit.py passes on both runners (auto-discovered by run_all.py). The red "Python source checks" is test_program_answer_domains.py, the same failure main has.

Two things inline. The first is the one I'd like fixed or explicitly scoped out before merging. It only matters with RX_VK_PARALLEL, but it brings back the same "stale for good" bug, this time for GPU words.

Overlap: #1184 (no mirror_make inside a parallel segment) narrows the first point, though it doesn't close it. #1150 binds read-back pages in guest memory and touches a different hunk of mirror_make. No textual conflict, and taking guest memory for pages written after the sync is the right behaviour there too.

Nice find, and the diagnosis checks out. Taking a retired mirror's page only when `page_gen <= r.synced_gen` gives the same result as "inherit everything, then run `mirror_sync` from `r.synced_gen`", which is the right invariant. Pages the CPU wrote after the old mirror's last sync come from the full guest copy, and the capture pass after it still puts captured pages on top as they were at their submit, just like `mirror_sync` does. What I checked: - `runs()`: the clean-region shortcut only applies on region-aligned pages, partial first/last pages stay clamped to [a, b), runs merge correctly, and the dst ranges never overlap, so the multi-region `vkCmdCopyBuffer` is fine. - GPU writes in earlier submissions are stamped while they're recorded, and `memory_end_submission` then raises `synced_gen` past those stamps, so they're still inherited. The barrier/`retired_value` handling didn't change. - In serial replay, each recording is its own submission with its upload buffer first, so a draw in the same recording never reached the inherit copy before this PR either. Nothing gets worse there. - Locking matches `mirror_sync` (under `pools_mu`, atomic `page_gen`, plain `region_gen` read). - The base is 159 commits behind main, but the `mirror_make` hunk is unchanged there and the PR is mergeable. `test_mirror_inherit.py` passes on both runners (auto-discovered by run_all.py). The red "Python source checks" is `test_program_answer_domains.py`, the same failure main has. Two things inline. The first is the one I'd like fixed or explicitly scoped out before merging. It only matters with `RX_VK_PARALLEL`, but it brings back the same "stale for good" bug, this time for GPU words. Overlap: #1184 (no `mirror_make` inside a parallel segment) narrows the first point, though it doesn't close it. #1150 binds read-back pages in guest memory and touches a different hunk of `mirror_make`. No textual conflict, and taking guest memory for pages written after the sync is the right behaviour there too.

gpu/vk/vk_memory.cpp

					
				@ -2875,8 +2677,36 @@ bool mirror_make(Pool& pool) {

				                    barrier_done = true;

With RX_VK_PARALLEL, r.synced_gen can be behind the old mirror's contents. If r was synced in the current submission (r.synced_epoch == mirror_epoch), it's still in mirror_synced, and synced_gen is only raised in memory_end_submission. So the bindings that earlier segments of this submission wrote into r.mirror are stamped above r.synced_gen. When the recording after the segments remakes the pool, its upload buffer runs after those segments, so the old whole-part copy did pick up their writes. This version takes those pages from guest memory instead, and guest memory doesn't have them until the write-back at the end of the submission. Then p->synced_gen = gen_now marks them as synced, so the GPU's words are stale until the game writes the page again.

The serial path can't hit this, since each recording ends its own submission. A cheap guard would be to keep the whole-part copy (or use gen_now as the threshold) when r.synced_epoch == mirror_epoch.

With `RX_VK_PARALLEL`, `r.synced_gen` can be behind the old mirror's contents. If `r` was synced in the current submission (`r.synced_epoch == mirror_epoch`), it's still in `mirror_synced`, and `synced_gen` is only raised in `memory_end_submission`. So the bindings that earlier segments of this submission wrote into `r.mirror` are stamped above `r.synced_gen`. When the recording after the segments remakes the pool, its upload buffer runs after those segments, so the old whole-part copy did pick up their writes. This version takes those pages from guest memory instead, and guest memory doesn't have them until the write-back at the end of the submission. Then `p->synced_gen = gen_now` marks them as synced, so the GPU's words are stale until the game writes the page again. The serial path can't hit this, since each recording ends its own submission. A cheap guard would be to keep the whole-part copy (or use `gen_now` as the threshold) when `r.synced_epoch == mirror_epoch`.

Nit: with RX_VK_PROTECT_BARE=1, mirror_sync keeps the mirror's copy of pages a shader wrote through a bare entry (prot_page_since(page, p.synced_gen)) even when the CPU also wrote them. This rule takes those from guest memory, so the experiment now behaves differently depending on whether the pool was remade. Treating protect_bare_on() && prot_page_since(page, r.synced_gen) as inherited in the page_gen lambda would keep the two consistent.

Nit: with `RX_VK_PROTECT_BARE=1`, `mirror_sync` keeps the mirror's copy of pages a shader wrote through a bare entry (`prot_page_since(page, p.synced_gen)`) even when the CPU also wrote them. This rule takes those from guest memory, so the experiment now behaves differently depending on whether the pool was remade. Treating `protect_bare_on() && prot_page_since(page, r.synced_gen)` as inherited in the `page_gen` lambda would keep the two consistent.

Nitnek commented

2026-11-09 19:47:07 +00:00

(Migrated from github.com)

A first longer run on NVIDIA, as asked: RTX 3070, Windows 11, main 5d1fe99 + this PR (clean merge). 23 minutes of scripted walking back and forth in Hyrule Field from one save:

the rule ran 20 times (... kept from guest memory, not its mirror);
no device lost, frame rate steady (52.9 fps average over the last 45 s, max frame 35 ms);
no froxel blocks, glow or black squares in the 14 screenshots I sampled across the run.

Caveat: the route drifted into the river under a cliff for much of the run, so the sky and the fog were often out of view. Read this as "nothing broke in 23 minutes" rather than proof the blocks are gone. I'll do a proper hour of normal play with the sky in view when I can.

A first longer run on NVIDIA, as asked: RTX 3070, Windows 11, main 5d1fe99 + this PR (clean merge). 23 minutes of scripted walking back and forth in Hyrule Field from one save: - the rule ran 20 times (`... kept from guest memory, not its mirror`); - no device lost, frame rate steady (52.9 fps average over the last 45 s, max frame 35 ms); - no froxel blocks, glow or black squares in the 14 screenshots I sampled across the run. Caveat: the route drifted into the river under a cliff for much of the run, so the sky and the fog were often out of view. Read this as "nothing broke in 23 minutes" rather than proof the blocks are gone. I'll do a proper hour of normal play with the sky in view when I can.

duck-paradise (Migrated from github.com) reviewed

2026-11-09 23:54:08 +00:00

gpu/vk/vk_memory.cpp

					
				@ -2875,8 +2677,36 @@ bool mirror_make(Pool& pool) {

				                    barrier_done = true;

Fixed in a56dfe1. Confirmed: GPU writes go through memory_stamp (++gen_now), so in a submission where r was synced its pages from earlier segments sit above the not-yet-raised synced_gen.

A retired pool with r.synced_epoch == mirror_epoch now gets its whole part copied, as before this PR. Nothing newer is lost by that: submit_memory_work collects the CPU's writes before recording, so r's sync this submission already has them, and anything stamped after that is treated as synced at memory_end_submission anyway (same as mirror_sync). The threshold rule still covers the case the fog bug came from: a pool retired in an earlier submission and remade later. I went with the whole copy rather than gen_now as the threshold because it gives the same result without the page scan.

test_mirror_inherit.py now also checks that mirror_make keeps this guard.

Fixed in a56dfe1. Confirmed: GPU writes go through `memory_stamp` (`++gen_now`), so in a submission where `r` was synced its pages from earlier segments sit above the not-yet-raised `synced_gen`. A retired pool with `r.synced_epoch == mirror_epoch` now gets its whole part copied, as before this PR. Nothing newer is lost by that: `submit_memory_work` collects the CPU's writes before recording, so `r`'s sync this submission already has them, and anything stamped after that is treated as synced at `memory_end_submission` anyway (same as `mirror_sync`). The threshold rule still covers the case the fog bug came from: a pool retired in an earlier submission and remade later. I went with the whole copy rather than `gen_now` as the threshold because it gives the same result without the page scan. `test_mirror_inherit.py` now also checks that `mirror_make` keeps this guard.

duck-paradise (Migrated from github.com) reviewed

2026-11-09 23:54:10 +00:00

gpu/vk/vk_memory.cpp

					
				@ -2875,8 +2677,36 @@ bool mirror_make(Pool& pool) {

				                    barrier_done = true;

Done in a56dfe1. mirror_inherit::runs now takes a per-page predicate (page_current) instead of a generation and threshold. mirror_make passes page_gen(page) <= synced || (protect_bare_on() && prot_page_since(page, synced)), so a bare-entry page keeps the old mirror's copy, the same way mirror_sync keeps it. prot_page_since only runs for pages newer than the sync, and only with the flag on. I added a test case for a newer page that the predicate keeps.

Done in a56dfe1. `mirror_inherit::runs` now takes a per-page predicate (`page_current`) instead of a generation and threshold. `mirror_make` passes `page_gen(page) <= synced || (protect_bare_on() && prot_page_since(page, synced))`, so a bare-entry page keeps the old mirror's copy, the same way `mirror_sync` keeps it. `prot_page_since` only runs for pages newer than the sync, and only with the flag on. I added a test case for a newer page that the predicate keeps.

duck-paradise commented

2026-11-09 23:54:20 +00:00

(Migrated from github.com)

@Nitnek "Nothing broke in 23 minutes" with the rule running 20 times is still useful, since this change could have caused a device loss or a new kind of corruption, and it didn't. An hour with the sky in view would settle the blocks themselves. If you do it, please use the new head (a56dfe1). It only differs from what you tested with RX_VK_PARALLEL or RX_VK_PROTECT_BARE set, so your results so far still apply to the default path.

@Nitnek "Nothing broke in 23 minutes" with the rule running 20 times is still useful, since this change could have caused a device loss or a new kind of corruption, and it didn't. An hour with the sky in view would settle the blocks themselves. If you do it, please use the new head (a56dfe1). It only differs from what you tested with `RX_VK_PARALLEL` or `RX_VK_PROTECT_BARE` set, so your results so far still apply to the default path.

hckrman101 (Migrated from github.com) reviewed

2026-11-10 03:38:13 +00:00

gpu/vk/vk_memory.cpp

					
				@ -2875,8 +2677,36 @@ bool mirror_make(Pool& pool) {

				                    barrier_done = true;

Thanks, this looks right. A pool synced in this submission is in mirror_synced, so memory_end_submission raises its synced_gen to gen_now anyway. Copying its whole part gives the same result mirror_sync would, and the bug case (a pool retired in an earlier submission and remade later) still goes through the rule. Resolved from my side.

Thanks, this looks right. A pool synced in this submission is in `mirror_synced`, so `memory_end_submission` raises its `synced_gen` to `gen_now` anyway. Copying its whole part gives the same result `mirror_sync` would, and the bug case (a pool retired in an earlier submission and remade later) still goes through the rule. Resolved from my side.

hckrman101 (Migrated from github.com) reviewed

2026-11-10 03:38:16 +00:00

gpu/vk/vk_memory.cpp

					
				@ -2875,8 +2677,36 @@ bool mirror_make(Pool& pool) {

				                    barrier_done = true;

Looks good. The predicate matches mirror_sync (dirty && protect_bare_on() && prot_page_since(page, synced_gen), same >=, same prot_mu under pools_mu), and the new test case covers it.

Looks good. The predicate matches `mirror_sync` (`dirty && protect_bare_on() && prot_page_since(page, synced_gen)`, same `>=`, same `prot_mu` under `pools_mu`), and the new test case covers it.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 03:38:36 +00:00

hckrman101 (Migrated from github.com) left a comment

Re-review at a56dfe1: both points are addressed.

RX_VK_PARALLEL: a retired pool with r.synced_epoch == mirror_epoch now gets its whole part copied. That's correct: it's in mirror_synced, so memory_end_submission sets its synced_gen = gen_now anyway. Whatever is stamped after its sync this submission counts as synced either way, which is the same result mirror_sync gives. The fog case (a pool retired in an earlier submission and remade at the next memory_begin_submission or bind) still has an older synced_epoch, so it still goes through the rule.
RX_VK_PROTECT_BARE: the page_current predicate matches mirror_sync (same prot_page_since threshold, prot_mu taken under pools_mu as there), and it's only reached for pages newer than the sync with the flag on, so the default path costs nothing extra.

The runs() refactor is mechanical. The new test case (a kept newer page joining the run before it, the next one left to guest memory) checks the right thing. CI is all green now, test_mirror_inherit.py on both runners included. The PR is mergeable cleanly against main. LGTM, and thanks for the NVIDIA runs, @Nitnek.

Re-review at a56dfe1: both points are addressed. - `RX_VK_PARALLEL`: a retired pool with `r.synced_epoch == mirror_epoch` now gets its whole part copied. That's correct: it's in `mirror_synced`, so `memory_end_submission` sets its `synced_gen = gen_now` anyway. Whatever is stamped after its sync this submission counts as synced either way, which is the same result `mirror_sync` gives. The fog case (a pool retired in an earlier submission and remade at the next `memory_begin_submission` or bind) still has an older `synced_epoch`, so it still goes through the rule. - `RX_VK_PROTECT_BARE`: the `page_current` predicate matches `mirror_sync` (same `prot_page_since` threshold, `prot_mu` taken under `pools_mu` as there), and it's only reached for pages newer than the sync with the flag on, so the default path costs nothing extra. The `runs()` refactor is mechanical. The new test case (a kept newer page joining the run before it, the next one left to guest memory) checks the right thing. CI is all green now, `test_mirror_inherit.py` on both runners included. The PR is mergeable cleanly against main. LGTM, and thanks for the NVIDIA runs, @Nitnek.

duck-paradise commented

2026-11-10 03:49:40 +00:00

(Migrated from github.com)

Thanks for the careful re-review, @hckrman101! And thanks again @Nitnek for the NVIDIA runs.

NotRexed commented

2026-11-10 03:51:25 +00:00

(Migrated from github.com)

This landed in the v2.0.12 branch, including the follow-up commit (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch, including the follow-up commit (merged there for testing; it reaches `main` with that release), so closing. Thanks!

❤️ 1

NotRexed commented

2026-11-10 03:54:04 +00:00

(Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

👍 1

Commenting is not possible because the repository is archived.

No reviewers

hckrman101