feat(vk): say why each texture is evicted, and which GPU-drawn ones go with no copy of their texels #1879
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!1879
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/texture-retire-stale"
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?
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.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. ATallycounts them anddescribe()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 MBThe 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).
Tested:
python3 tests/test_evict_reason.pypasses: classification, the lost-texels rule, the tally, the formatting, and a short buffer. It also checks that the call sites are still invk_texture.cpp.tests/test_keep_rendered.pystill passes.vk_texture.cppwith the back end's flags is clean.docs/tested-configurations.mdonce a run is in.🤖 Generated with Claude Code
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:
classifymatches how the loop picks textures (shifted implies overwritten, so DEAD first is right), andtexels_lostis exactly the case the flush was skipped or failed for a GPU-drawn texture nothing drew over. Checkingflushed_okrather than trustinge.flushis the right call, sinceflush_texturescan fail per texture.nandtally.total()should always agree too, since nothing else takes textures out oftexturesbetween buildingorderand thegoneloop.Things to sort out:
#include "vram_compact.h"on the same line you putevict_reason.h, andtexture_framehas 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.test_evict_reason.pyandtest_keep_rendered.pypass on both runners. The red in Python source checks istest_antilag,nx2/test_draw_parameters(Ctx::mesh_fallback),test_wayland_libdecor_hintandnx2/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_finalizedpath 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 MBwould 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.
Nit:
sweepis always false here. In a sweep,if (sweep && !overwritten) continue;above means only overwritten textures get evicted, andtexels_lostneeds!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.Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!
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.Rebased onto current
main(2ed98c46). The conflicts were the include next tovram_compact.hand theevict.push_backline, which keeps main'senv_on("RX_VK_EVICT_NO_FLUSH")and adds this PR'soverwritten. It's still the only push intoevict.The suggestion is in too (
ca703929): the summary now readsGPU-drawn with no copy kept: 2 live, 5 finalized, 1 MB.Tallycounts the finalized ones (tex->released), andtest_evict_reasonchecks the split.tests/run_all.pyon Linux: 393 passed, 0 failed. It still needs the in-game run.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.hinclude andenv_on("RX_VK_EVICT_NO_FLUSH"), which are both kept.vk_texture.cpphasn't changed on main since2ed98c46(the two commits after it are reactive-mask/DLSS work).lost_finalizedcountstex->released,describeprintsN 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.sweep ?nit: fixed. The per-texture line now always says "over the budget" and has a comment saying why.CI:
test_evict_reasonpasses on both runners. The Windows reds aren't this PR's.test_native_astc_texture_countfails on main too (its fixture'sscanftrips-Werrorunder 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 Evictandevict.push_backlines, so whichever of the two lands second gets a small conflict there. #2368's kept shadow mapscontinuebefore 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.mdrow would still be good to have.github-sync referenced this pull request2026-11-12 06:22:54 +00:00