feat(vk): say why each texture is evicted, and which GPU-drawn ones go with no copy of their texels #1879

Merged
Joey-Magnus merged 2 commits from fix/texture-retire-stale into main 2026-11-11 18:53:18 +00:00
Joey-Magnus commented 2026-11-10 19:13:26 +00:00 (Migrated from github.com)

This adds diagnostics for the worklist row "ground and outdoor textures garble after a while of play, fixed by a restart" (16 GB AMD cards, plus an RTX 4080 report). It changes no behaviour, only log lines.

Why: the eviction pass logged only N textures retired (left, MB), and the user logs we have can't answer the open question.

  • One 9070 XT log stays at 3.3 GB of its 6 GB texture budget, yet retires 28,000 textures.
  • Another sits at the budget and retires 18,500.

Neither log says whether those were budget evictions or render-target sweeps. It also can't say whether a texture the GPU drew went without a write-back while nothing had drawn over it. A texture like that, made again over the same storage, starts empty, which was the ground streaks of issues 107 and 386.

What changes:

  • gpu/vk/evict_reason.h (new): classify(shifted, overwritten) sorts each evicted texture as budget, drawn over or a dead target. texels_lost(gpu_written, overwritten, flushed) flags the risky case. A Tally counts them and describe() formats the summary.
  • vk_texture.cpp, the retire pass: it records which write-backs succeeded (flushed_ok), tallies each evicted texture, and appends the breakdown to the line:
    vk: 140 textures retired (1176 left, 5374 MB): budget 12, drawn over 3, dead targets 125; GPU-drawn with no copy kept: 1 live, 1 finalized, 1 MB
    The first 100 risky evictions each get a line with size, NVN format, finalized or live, frames unused, and that the budget took them (a sweep lets only drawn-over textures go, which lose nothing).
  • Nothing parses the old line (checked with grep).

Tested:

  • python3 tests/test_evict_reason.py passes: classification, the lost-texels rule, the tally, the formatting, and a short buffer. It also checks that the call sites are still in vk_texture.cpp.
  • tests/test_keep_rendered.py still passes.
  • A syntax-only clang++ compile of vk_texture.cpp with the back end's flags is clean.
  • Not yet run in game. The test machine is in use. I'll add a row to docs/tested-configurations.md once a run is in.

🤖 Generated with Claude Code

This adds diagnostics for the worklist row "ground and outdoor textures garble after a while of play, fixed by a restart" (16 GB AMD cards, plus an RTX 4080 report). It changes no behaviour, only log lines. **Why:** the eviction pass logged only `N textures retired (left, MB)`, and the user logs we have can't answer the open question. - One 9070 XT log stays at 3.3 GB of its 6 GB texture budget, yet retires 28,000 textures. - Another sits at the budget and retires 18,500. Neither log says whether those were budget evictions or render-target sweeps. It also can't say whether a texture the GPU drew went without a write-back while nothing had drawn over it. A texture like that, made again over the same storage, starts empty, which was the ground streaks of issues 107 and 386. **What changes:** - `gpu/vk/evict_reason.h` (new): `classify(shifted, overwritten)` sorts each evicted texture as budget, drawn over or a dead target. `texels_lost(gpu_written, overwritten, flushed)` flags the risky case. A `Tally` counts them and `describe()` formats the summary. - `vk_texture.cpp`, the retire pass: it records which write-backs succeeded (`flushed_ok`), tallies each evicted texture, and appends the breakdown to the line: `vk: 140 textures retired (1176 left, 5374 MB): budget 12, drawn over 3, dead targets 125; GPU-drawn with no copy kept: 1 live, 1 finalized, 1 MB` The first 100 risky evictions each get a line with size, NVN format, finalized or live, frames unused, and that the budget took them (a sweep lets only drawn-over textures go, which lose nothing). - Nothing parses the old line (checked with grep). **Tested:** - `python3 tests/test_evict_reason.py` passes: classification, the lost-texels rule, the tally, the formatting, and a short buffer. It also checks that the call sites are still in `vk_texture.cpp`. - `tests/test_keep_rendered.py` still passes. - A syntax-only clang++ compile of `vk_texture.cpp` with the back end's flags is clean. - **Not yet run in game.** The test machine is in use. I'll add a row to `docs/tested-configurations.md` once a run is in. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 02:17:05 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks, this is a useful line to have for the garbling reports. I read the retire pass end to end and the logic holds up: classify matches how the loop picks textures (shifted implies overwritten, so DEAD first is right), and texels_lost is exactly the case the flush was skipped or failed for a GPU-drawn texture nothing drew over. Checking flushed_ok rather than trusting e.flush is the right call, since flush_textures can fail per texture. n and tally.total() should always agree too, since nothing else takes textures out of textures between building order and the gone loop.

Things to sort out:

  • Conflicts with main. GitHub shows the PR as not mergeable. Main added #include "vram_compact.h" on the same line you put evict_reason.h, and texture_frame has picked up the RX_VK_COMPACT changes (evict_to, compact.in_pass) since your base. Those don't touch the lines you changed beyond the include, so the rebase should be mechanical, but it needs doing.
  • CI. test_evict_reason.py and test_keep_rendered.py pass on both runners. The red in Python source checks is test_antilag, nx2/test_draw_parameters (Ctx::mesh_fallback), test_wayland_libdecor_hint and nx2/test_nup_out_of_space. None of those touch this code, so I don't think it's the PR, but a rebase onto current main should clear or at least change that set.

Optional: the summary lumps finalized and live textures into "GPU-drawn with no copy kept". Both matter (the keep_finalized path is there because finalized terrain normal maps came back empty), but for the question this is meant to answer, splitting it as ... no copy kept: 2 live, 5 finalized, 1 MB would save reading the per-texture lines, especially once the first 100 are used up.

Nit inline. It still needs an in-game run, as you say. Once it's rebased this looks good to me.

Thanks, this is a useful line to have for the garbling reports. I read the retire pass end to end and the logic holds up: `classify` matches how the loop picks textures (shifted implies overwritten, so DEAD first is right), and `texels_lost` is exactly the case the flush was skipped or failed for a GPU-drawn texture nothing drew over. Checking `flushed_ok` rather than trusting `e.flush` is the right call, since `flush_textures` can fail per texture. `n` and `tally.total()` should always agree too, since nothing else takes textures out of `textures` between building `order` and the `gone` loop. Things to sort out: - **Conflicts with main.** GitHub shows the PR as not mergeable. Main added `#include "vram_compact.h"` on the same line you put `evict_reason.h`, and `texture_frame` has picked up the RX_VK_COMPACT changes (`evict_to`, `compact.in_pass`) since your base. Those don't touch the lines you changed beyond the include, so the rebase should be mechanical, but it needs doing. - **CI.** `test_evict_reason.py` and `test_keep_rendered.py` pass on both runners. The red in Python source checks is `test_antilag`, `nx2/test_draw_parameters` (`Ctx::mesh_fallback`), `test_wayland_libdecor_hint` and `nx2/test_nup_out_of_space`. None of those touch this code, so I don't think it's the PR, but a rebase onto current main should clear or at least change that set. Optional: the summary lumps finalized and live textures into "GPU-drawn with no copy kept". Both matter (the `keep_finalized` path is there because finalized terrain normal maps came back empty), but for the question this is meant to answer, splitting it as `... no copy kept: 2 live, 5 finalized, 1 MB` would save reading the per-texture lines, especially once the first 100 are used up. Nit inline. It still needs an in-game run, as you say. Once it's rebased this looks good to me.
hckrman101 (Migrated from github.com) commented 2026-11-11 02:17:05 +00:00

Nit: sweep is always false here. In a sweep, if (sweep && !overwritten) continue; above means only overwritten textures get evicted, and texels_lost needs !overwritten, so this line only ever prints "over the budget". Harmless, but the ternary suggests the sweep can lose texels when it can't. Dropping it, or noting it can't happen, would be clearer.

Nit: `sweep` is always false here. In a sweep, `if (sweep && !overwritten) continue;` above means only overwritten textures get evicted, and `texels_lost` needs `!overwritten`, so this line only ever prints "over the budget". Harmless, but the ternary suggests the sweep can lose texels when it can't. Dropping it, or noting it can't happen, would be clearer.
hckrman101 commented 2026-11-11 15:51:00 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 18:41:01 +00:00
Joey-Magnus (Migrated from github.com) commented 2026-11-11 18:41:01 +00:00

Dropped in ca703929: the line always says "over the budget", with a note that a sweep lets only drawn-over textures go, and those lose nothing.

Dropped in ca703929: the line always says "over the budget", with a note that a sweep lets only drawn-over textures go, and those lose nothing.
Joey-Magnus commented 2026-11-11 18:41:03 +00:00 (Migrated from github.com)

Rebased onto current main (2ed98c46). The conflicts were the include next to vram_compact.h and the evict.push_back line, which keeps main's env_on("RX_VK_EVICT_NO_FLUSH") and adds this PR's overwritten. It's still the only push into evict.

The suggestion is in too (ca703929): the summary now reads GPU-drawn with no copy kept: 2 live, 5 finalized, 1 MB. Tally counts the finalized ones (tex->released), and test_evict_reason checks the split.

tests/run_all.py on Linux: 393 passed, 0 failed. It still needs the in-game run.

Rebased onto current `main` (2ed98c46). The conflicts were the include next to `vram_compact.h` and the `evict.push_back` line, which keeps main's `env_on("RX_VK_EVICT_NO_FLUSH")` and adds this PR's `overwritten`. It's still the only push into `evict`. The suggestion is in too (ca703929): the summary now reads `GPU-drawn with no copy kept: 2 live, 5 finalized, 1 MB`. `Tally` counts the finalized ones (`tex->released`), and `test_evict_reason` checks the split. `tests/run_all.py` on Linux: 393 passed, 0 failed. It still needs the in-game run.
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 18:49:03 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the rebase and the split. I compared the old and new patches file by file. The only changes are the ones you described, plus main's vram_compact.h include and env_on("RX_VK_EVICT_NO_FLUSH"), which are both kept.

  • Rebase: fixed. GitHub shows it as mergeable. vk_texture.cpp hasn't changed on main since 2ed98c46 (the two commits after it are reactive-mask/DLSS work).
  • Live/finalized split: done. lost_finalized counts tex->released, describe prints N live, M finalized, X MB, and the test checks the new string. 160 bytes is still plenty for the line, even with 10-digit counts.
  • The sweep ? nit: fixed. The per-texture line now always says "over the budget" and has a comment saying why.

CI: test_evict_reason passes on both runners. The Windows reds aren't this PR's. test_native_astc_texture_count fails on main too (its fixture's scanf trips -Werror under the Windows CRT). test_recorder_sync ("thread oversize: 0 calls made directly") is a recorder-ring test that passed on main's run. This PR doesn't touch anything it reads, so it looks like a flake. A re-run should clear it.

Heads-up: #2198 edits the same struct Evict and evict.push_back lines, so whichever of the two lands second gets a small conflict there. #2368's kept shadow maps continue before the push, so they never reach the tally and the counts stay right.

It's log-only, so I'm fine with it going in before the in-game run. The run plus the tested-configurations.md row would still be good to have.

Thanks for the rebase and the split. I compared the old and new patches file by file. The only changes are the ones you described, plus main's `vram_compact.h` include and `env_on("RX_VK_EVICT_NO_FLUSH")`, which are both kept. - **Rebase:** fixed. GitHub shows it as mergeable. `vk_texture.cpp` hasn't changed on main since 2ed98c46 (the two commits after it are reactive-mask/DLSS work). - **Live/finalized split:** done. `lost_finalized` counts `tex->released`, `describe` prints `N live, M finalized, X MB`, and the test checks the new string. 160 bytes is still plenty for the line, even with 10-digit counts. - **The `sweep ?` nit:** fixed. The per-texture line now always says "over the budget" and has a comment saying why. CI: `test_evict_reason` passes on both runners. The Windows reds aren't this PR's. `test_native_astc_texture_count` fails on main too (its fixture's `scanf` trips `-Werror` under the Windows CRT). `test_recorder_sync` ("thread oversize: 0 calls made directly") is a recorder-ring test that passed on main's run. This PR doesn't touch anything it reads, so it looks like a flake. A re-run should clear it. Heads-up: #2198 edits the same `struct Evict` and `evict.push_back` lines, so whichever of the two lands second gets a small conflict there. #2368's kept shadow maps `continue` before the push, so they never reach the tally and the counts stay right. It's log-only, so I'm fine with it going in before the in-game run. The run plus the `tested-configurations.md` row would still be good to have.
Commenting is not possible because the repository is archived.
No description provided.