MagicBeanDev/Hookshot

Archived

Watch

Fork 0

fix(vk): cyan after a pause in the background: keep the mirrors of pools the GPU writes #2154

Merged

rakefire-impala merged 3 commits from fix/mirror-gpu-only into main

2026-11-12 03:04:17 +00:00

rakefire-impala commented

2026-11-11 05:07:52 +00:00

(Migrated from github.com)

Update (2026-11-11): a second commit widens the fix to save reloads and logs the mirrors still given back. The pause fix below is tested in game; the second commit is not yet (see "Save reloads").

Problem

Pause the game, leave it in the background for 1–2 minutes (another window or the Start menu in front), then come back and resume: for a few seconds the picture has cyan patches. On an RX 5500M (RDNA1, 4 GB, Windows 11, AMD driver) this happened on every resume.

While the game is paused, most of its memory pools are not bound. After kIdleEpochs (600) submissions an unbound pool's VRAM mirror is idle, and mirrors_give_back gives it back once it has been idle past kLongIdle (6000 submissions) or when a wanted mirror does not fit. In one such pause the running count in the log went from 35 to 51 (51 mirrors given back (1136 MB in all)). At the pool's next binding, mirror_make makes a new mirror from guest memory (plus the CPU's newer writes from the captures).

Most of what the GPU writes into a mirror is copied back to guest memory: a written binding queues a write-back. But some of it is never copied back, by design, and is noted only as a hazard by memory_note_gpu_write: what a shader writes through a bare entry's guessed range, the pointer table's range or a wide binding. (Copying the whole guessed range back overwrote the CPU's own data next to it.) That data was in the old mirror alone. The new mirror starts without it, and the GPU goes on from stale data until it has rebuilt it, which is the few seconds of cyan.

Change

Pool::gpu_only: set by memory_note_gpu_write on every imported pool its range reaches.
mirrors_give_back leaves an idle gpu_only pool's mirror alone. Its other idle mirrors go as before.
A pool that takes others in (pool_lookup's merge) keeps their mark: their mirrors' contents come with them.
The give-back's log line also says how many MB of idle mirrors are kept this way.
Second commit: Pool::gpu_written, set by gpu_buffer_bind for a written binding of the mirror, carried over a merge like gpu_only, and the idle give-back keeps those mirrors too.
Second commit: every mirror still given back is logged (the first 200), with why (retired, retired and not inherited in time, idle, or idle for room), how many submissions since its sync, and the pool's marks.

A pool that is refused a mirror for the budget meanwhile is read in system memory, which is slower but not wrong. Retired pools are unchanged: their GPU contents already go to the pool that took them in (inherit_pending).

Tested

RX 5500M, Windows 11, AMD driver, main 92907f7 with #1581 and #2125, same settings throughout. Each pause was 1–2 minutes with the game in the background:

	cyan after resume	mirrors given back during the run
main (`92907f7` + #1581 + #2125)	every resume (tried at Auto), for a few seconds	16 in one pause
`RX_VK_MIRROR_KEEP=1` (no give-back at all)	none, one pause at Auto and one at 33% Render resolution	0
this PR	none, one pause at Auto and one at 33%	48 in the session; 112 MB of idle mirrors kept at the last count

A very short black flash right on resume is still there at both resolutions. At Auto it has been there since the first builds on this card, including the RX_VK_MIRROR_KEEP=1 run, so it looks separate.

Save reloads

With the first commit, cyan came back after reloading a save within a run (Hyrule Castle, Hyrule Field), in play and over the Start menu, with no pause and no not imported pools in the log. (That run also had RX_VK_FIFO=1, which turned out not to matter.) Against RX_VK_MIRROR_KEEP=1, which never gives a mirror back:

	run 1	run 2
first commit only	cyan after each reload (castle, field)	—
`RX_VK_MIRROR_KEEP=1`	three reloads, no cyan over the Start menu	fewer flashes, some on the third reload

So mirrors given back explain most of the cyan after a reload, but not all of it. The first commit keeps only pools the GPU wrote past the write-back. The pools whose mirrors still went are the ones the GPU writes through a normal binding, which the second commit keeps too.

Not tested in game yet: the second commit. I haven't played a build with it, so I don't know yet whether it matches RX_VK_MIRROR_KEEP=1 or what it costs in VRAM. Its log line is there to answer both, and to show which mirrors are still given back before any flash that's left.

Tests

tests/test_mirror_gpu_only.py (new) reads the source and checks four things: memory_note_gpu_write marks the pools it reaches, gpu_buffer_bind marks a pool written through its mirror, the idle give-back leaves marked pools, and a merge keeps both marks. It fails on current main and passes with this change. nx2/test_import_windows.py builds pool_lookup against a Pool of its own, which now has both fields.

tests/run_all.py on main 4d68bc83 with this branch merged: 311 passed. The 6 that fail are the same 6 that fail on plain main on this PC.

Related: #1005 (cyan and UI flashes after focus loss), #2125.

> **Update (2026-11-11):** a second commit widens the fix to save reloads and logs the mirrors still given back. The pause fix below is tested in game; the second commit is not yet (see "Save reloads"). ## Problem Pause the game, leave it in the background for 1–2 minutes (another window or the Start menu in front), then come back and resume: for a few seconds the picture has cyan patches. On an RX 5500M (RDNA1, 4 GB, Windows 11, AMD driver) this happened on every resume. While the game is paused, most of its memory pools are not bound. After `kIdleEpochs` (600) submissions an unbound pool's VRAM mirror is idle, and `mirrors_give_back` gives it back once it has been idle past `kLongIdle` (6000 submissions) or when a wanted mirror does not fit. In one such pause the running count in the log went from 35 to 51 (`51 mirrors given back (1136 MB in all)`). At the pool's next binding, `mirror_make` makes a new mirror from guest memory (plus the CPU's newer writes from the captures). Most of what the GPU writes into a mirror is copied back to guest memory: a written binding queues a write-back. But some of it is never copied back, by design, and is noted only as a hazard by `memory_note_gpu_write`: what a shader writes through a bare entry's guessed range, the pointer table's range or a wide binding. (Copying the whole guessed range back overwrote the CPU's own data next to it.) That data was in the old mirror alone. The new mirror starts without it, and the GPU goes on from stale data until it has rebuilt it, which is the few seconds of cyan. ## Change - `Pool::gpu_only`: set by `memory_note_gpu_write` on every imported pool its range reaches. - `mirrors_give_back` leaves an idle `gpu_only` pool's mirror alone. Its other idle mirrors go as before. - A pool that takes others in (`pool_lookup`'s merge) keeps their mark: their mirrors' contents come with them. - The give-back's log line also says how many MB of idle mirrors are kept this way. - Second commit: `Pool::gpu_written`, set by `gpu_buffer_bind` for a written binding of the mirror, carried over a merge like `gpu_only`, and the idle give-back keeps those mirrors too. - Second commit: every mirror still given back is logged (the first 200), with why (retired, retired and not inherited in time, idle, or idle for room), how many submissions since its sync, and the pool's marks. A pool that is refused a mirror for the budget meanwhile is read in system memory, which is slower but not wrong. Retired pools are unchanged: their GPU contents already go to the pool that took them in (`inherit_pending`). ## Tested RX 5500M, Windows 11, AMD driver, main `92907f7` with #1581 and #2125, same settings throughout. Each pause was 1–2 minutes with the game in the background: | | cyan after resume | mirrors given back during the run | |---|---|---| | main (`92907f7` + #1581 + #2125) | every resume (tried at Auto), for a few seconds | 16 in one pause | | `RX_VK_MIRROR_KEEP=1` (no give-back at all) | none, one pause at Auto and one at 33% Render resolution | 0 | | this PR | none, one pause at Auto and one at 33% | 48 in the session; 112 MB of idle mirrors kept at the last count | A very short black flash right on resume is still there at both resolutions. At Auto it has been there since the first builds on this card, including the `RX_VK_MIRROR_KEEP=1` run, so it looks separate. ## Save reloads With the first commit, cyan came back after reloading a save within a run (Hyrule Castle, Hyrule Field), in play and over the Start menu, with no pause and no `not imported` pools in the log. (That run also had `RX_VK_FIFO=1`, which turned out not to matter.) Against `RX_VK_MIRROR_KEEP=1`, which never gives a mirror back: | | run 1 | run 2 | |---|---|---| | first commit only | cyan after each reload (castle, field) | — | | `RX_VK_MIRROR_KEEP=1` | three reloads, no cyan over the Start menu | fewer flashes, some on the third reload | So mirrors given back explain most of the cyan after a reload, but not all of it. The first commit keeps only pools the GPU wrote past the write-back. The pools whose mirrors still went are the ones the GPU writes through a normal binding, which the second commit keeps too. **Not tested in game yet:** the second commit. I haven't played a build with it, so I don't know yet whether it matches `RX_VK_MIRROR_KEEP=1` or what it costs in VRAM. Its log line is there to answer both, and to show which mirrors are still given back before any flash that's left. ## Tests `tests/test_mirror_gpu_only.py` (new) reads the source and checks four things: `memory_note_gpu_write` marks the pools it reaches, `gpu_buffer_bind` marks a pool written through its mirror, the idle give-back leaves marked pools, and a merge keeps both marks. It fails on current main and passes with this change. `nx2/test_import_windows.py` builds `pool_lookup` against a `Pool` of its own, which now has both fields. `tests/run_all.py` on main `4d68bc83` with this branch merged: 311 passed. The 6 that fail are the same 6 that fail on plain main on this PC. Related: #1005 (cyan and UI flashes after focus loss), #2125.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 16:28:55 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, the diagnosis is clear and the first commit is a well-targeted fix: data that memory_note_gpu_write deliberately never copies back really does live only in the mirror, so giving that mirror back can't be right. CI is green.

My concern is the second commit, which you've flagged as untested in game.

VRAM / budget cost. The VRAM ceiling doesn't change: kept mirrors still count in mirror_bytes and mirror_make still refuses past mirror_budget(). What changes is who gets that budget. Both marks are sticky (never cleared, and ORed through merges). gpu_written is set by any written binding, and with RX_VK_WB_MAX_KB unset (the default) every one of those is queued for write-back, so its data is already in guest memory. In practice this probably marks most pools the game touches with compute. Once kept mirrors fill the budget, the "idle, for room" pass has nothing to free, and new or hot pools stay in system memory and are read over PCIe. That's the "stayed in system memory for good" behaviour the give-back was added to fix (see the comment above kIdleEpochs). The budget is small exactly on the cards this PR is about: on a 4 GB card it's VRAM/8 = 512 MB, or 256 MB with the Minimum preset. The first commit alone kept 112 MB. On 8 GB+ cards the budget is up to 3 GB, so long-idle mirrors that used to go after kLongIdle now stay resident.

Interaction with #2198. That PR turns the texture compaction on by default, and it holds textures at 85% of the allowance (80% for the reserve). Mirrors aren't something it can evict, so mirrors kept around permanently shrink the room left for textures, and the hold starts evicting textures sooner on cards that are already tight. Neither PR breaks the other in code (they don't touch the same files), but it's one more reason to measure the second commit before it ships. Could you post the MB of them idle and kept figure and any stays in system memory lines from a session with a few reloads, ideally on the 5500M?

Suggestions, any one would do:

Let the "for room" pass free gpu_written-only mirrors (keep only gpu_only absolute). Their data was written back, so remaking them from guest memory is correct, and only the long-idle free would be skipped.
Or split the second commit into a follow-up, once a run shows it matches RX_VK_MIRROR_KEEP=1 without pools being refused.

If the save-reload cyan does come from written-back pools, then the write-back or the remake from captures is missing something, and keeping the mirror would hide that rather than fix it. The new log line should show which.

Also, a small hole that stays open: a retired gpu_only pool whose successor makes no mirror within 600 submissions still loses its data (now logged as inherit_lost). That's fine to leave as it is, since the log will show whether it ever happens.

Thanks, the diagnosis is clear and the first commit is a well-targeted fix: data that `memory_note_gpu_write` deliberately never copies back really does live only in the mirror, so giving that mirror back can't be right. CI is green. My concern is the second commit, which you've flagged as untested in game. **VRAM / budget cost.** The VRAM ceiling doesn't change: kept mirrors still count in `mirror_bytes` and `mirror_make` still refuses past `mirror_budget()`. What changes is who gets that budget. Both marks are sticky (never cleared, and ORed through merges). `gpu_written` is set by any written binding, and with `RX_VK_WB_MAX_KB` unset (the default) every one of those is queued for write-back, so its data is already in guest memory. In practice this probably marks most pools the game touches with compute. Once kept mirrors fill the budget, the "idle, for room" pass has nothing to free, and new or hot pools stay in system memory and are read over PCIe. That's the "stayed in system memory for good" behaviour the give-back was added to fix (see the comment above `kIdleEpochs`). The budget is small exactly on the cards this PR is about: on a 4 GB card it's VRAM/8 = 512 MB, or 256 MB with the Minimum preset. The first commit alone kept 112 MB. On 8 GB+ cards the budget is up to 3 GB, so long-idle mirrors that used to go after `kLongIdle` now stay resident. **Interaction with #2198.** That PR turns the texture compaction on by default, and it holds textures at 85% of the allowance (80% for the reserve). Mirrors aren't something it can evict, so mirrors kept around permanently shrink the room left for textures, and the hold starts evicting textures sooner on cards that are already tight. Neither PR breaks the other in code (they don't touch the same files), but it's one more reason to measure the second commit before it ships. Could you post the `MB of them idle and kept` figure and any `stays in system memory` lines from a session with a few reloads, ideally on the 5500M? Suggestions, any one would do: - Let the "for room" pass free `gpu_written`-only mirrors (keep only `gpu_only` absolute). Their data was written back, so remaking them from guest memory is correct, and only the long-idle free would be skipped. - Or split the second commit into a follow-up, once a run shows it matches `RX_VK_MIRROR_KEEP=1` without pools being refused. If the save-reload cyan does come from written-back pools, then the write-back or the remake from captures is missing something, and keeping the mirror would hide that rather than fix it. The new log line should show which. Also, a small hole that stays open: a retired `gpu_only` pool whose successor makes no mirror within 600 submissions still loses its data (now logged as `inherit_lost`). That's fine to leave as it is, since the log will show whether it ever happens.

gpu/vk/vk_memory.cpp

Nit: imported pools are disjoint (pool_lookup merges overlaps) and sorted by lo, so once hi <= guest nothing further down can reach guest, and the loop could break right there. As written it walks every pool within 1 GB below, while holding pools_mu, on every call.

Nit: imported pools are disjoint (pool_lookup merges overlaps) and sorted by `lo`, so once `hi <= guest` nothing further down can reach `guest`, and the loop could break right there. As written it walks every pool within 1 GB below, while holding `pools_mu`, on every call.

gpu/vk/vk_memory.cpp

This marks every pool with any written binding, including those whose write-back is queued just below (all of them with RX_VK_WB_MAX_KB unset). Their contents are in guest memory, so a mirror remade from it should be correct. Maybe only mark when the write-back is skipped (wb_max && size > wb_max), or let the for-room pass free these. See the review body.

This marks every pool with any written binding, including those whose write-back is queued just below (all of them with `RX_VK_WB_MAX_KB` unset). Their contents are in guest memory, so a mirror remade from it should be correct. Maybe only mark when the write-back is skipped (`wb_max && size > wb_max`), or let the for-room pass free these. See the review body.

gpu/vk/vk_memory.cpp

Kept pools are skipped by the for-room pass too, not just the long-idle one. Once they fill mirror_budget() (512 MB on a 4 GB card, 256 MB with the Minimum preset), every new pool is refused and read over PCIe. Keeping them from the kLongIdle free but still letting them go when a wanted mirror doesn't fit (at least the gpu_written-only ones) would avoid that.

Kept pools are skipped by the for-room pass too, not just the long-idle one. Once they fill `mirror_budget()` (512 MB on a 4 GB card, 256 MB with the Minimum preset), every new pool is refused and read over PCIe. Keeping them from the `kLongIdle` free but still letting them go when a wanted mirror doesn't fit (at least the `gpu_written`-only ones) would avoid that.

Joey-Magnus (Migrated from github.com) reviewed

2026-11-11 21:25:39 +00:00

Joey-Magnus (Migrated from github.com) left a comment

The first commit is right: data memory_note_gpu_write never copies back lives only in the VRAM mirror, so keeping gpu_only pools' mirrors fixes the cause.

The second commit is untested in game by your own note. It sets gpu_written on every written binding (vk_memory.cpp:2760) and never clears it, and mirrors_give_back then never frees those mirrors (:2901), not even in the "for room" pass. With write-back on by default, that data is already in guest memory. On a 4 GB card (512 MB mirror budget, 256 MB on Minimum) other pools get pushed into system memory for good. Either ship the first commit alone, or let the "for room" pass free gpu_written-only mirrors, and attach a 5500M log with save reloads.

The first commit is right: data `memory_note_gpu_write` never copies back lives only in the VRAM mirror, so keeping `gpu_only` pools' mirrors fixes the cause. The second commit is untested in game by your own note. It sets `gpu_written` on every written binding (`vk_memory.cpp:2760`) and never clears it, and `mirrors_give_back` then never frees those mirrors (`:2901`), not even in the "for room" pass. With write-back on by default, that data is already in guest memory. On a 4 GB card (512 MB mirror budget, 256 MB on Minimum) other pools get pushed into system memory for good. Either ship the first commit alone, or let the "for room" pass free `gpu_written`-only mirrors, and attach a 5500M log with save reloads.