WIP: fix(vk): keep the shadow map and other drawn textures the game samples again through evictions #211

Draft
TrashPanda wants to merge 2 commits from TrashPanda/Longshot:fix/keep-resampled-drawn into main
Contributor

Why

The game draws its large shadow map once and then samples it for many frames. When textures went over the VRAM budget, the renderer evicted it and made it again empty, so shadows went missing or wrong and the ground turned dark (Hookshot issue 2209) until the game drew it again some 40 frames later. This was Hookshot PR 2368. It wasn't merged before the GitHub repo went down, so I'm bringing it here, bounded so it can't add VRAM pressure on small cards.

What changed

  • vk: gpu/vk/keep_resampled.h (new) and gpu/vk/vk_texture.cpp's eviction pass. A texture the GPU drew that the game samples again after finalizing it is marked (Texture::resampled, gpu/vk/vk_texture.h) and kept through evictions over the budget. It stays a candidate only while it was used in the last 1,800 presents and its storage is still the GPU's (gpu_owned, the check keep_finalized already makes). When another render target takes that range, for example after an area change, the pin goes right away.
  • vk: the rule is bounded. Its cap for each pass is the smallest of RX_VK_KEEP_RESAMPLED_MB (512), an eighth of the texture budget (128 MB on a 1 GB budget, 200 MB on 1.6 GB) and keep_finalized's cap. It is 0 once VRAM use reaches 97% of what the driver allows, the same line keep_rendered uses. Past that line the textures go as they did on main, and the small ones are keep_finalized's again.
  • vk: one cap for both rules. What this rule may hold in a pass (its candidates' total, at most its cap) comes off keep_finalized's 512 MB. Together the two hold no more than keep_finalized held alone on main. When the cap is reached, the least recently used go first.
  • tests: tests/test_keep_resampled.py and .cpp check each rule: the budget/8 cap, nothing kept at 97% (the same line as keep_rendered, checked across a range), the pin released once the storage is no longer the GPU's, and the shared cap never exceeded. The source checks find the pass's calls. Without the change the checks fail.
  • docs: the switch in docs/development.md, and the capped-VRAM runs in docs/tested-configurations.md (from Hookshot, before this bound).

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
RX_VK_KEEP_RESAMPLED_MB=<MB> Most the drawn-and-resampled textures may hold through evictions. The real cap is also at most an eighth of the texture budget and comes out of RX_VK_KEEP_FINALIZED_MB's room. It is 0 at 97% of the driver's limit. 0 gives the old behavior 512 On by default: it only acts over the budget, takes no VRAM beyond what keep_finalized could already hold, and steps aside at the driver's limit Cards that go over the texture budget with headroom left (a budget of 2.6 GB or more keeps both shadow maps) Cards that never evict. Budgets under about 2.6 GB keep only what fits in an eighth (the 32 MB map, not the 288 MB one). Cards already at the driver's limit AMD, Intel, Steam Deck, Windows; any Longshot in-game run (see Testing)

Measurements

No Longshot measurement yet. The numbers below are from Hookshot, measured on the version the review saw (a 512 MB cap of its own, no budget or driver-limit bound, no shared cap). They show what the rule costs when it acts, but they don't measure this head. The PR stays WIP until T-DCLN and T-J6DC are done.

main: Hookshot 6c8c1bfb (2026-11-11)
this PR: Hookshot fix/keep-resampled-drawn @ 3fbad818
Hardware: RTX 4070 Ti 12 GB, 595.99.02, Ryzen 7 9800X3D, 32 GB, Ubuntu 26.04.1
Scene: Hyrule Field, 2560x1440 window, 2048x1152 output, HOS_FPS_MAX=240
Method: alternating scripted 3-minute runs with RX_VK_VRAM_MB at 4000, 2000 and 1000, and at 4000 with 3.8 GB of VRAM held by another process; RX_VK_LOG_REMADE=1
How to reproduce: the row dated 2026-11-11 in docs/tested-configurations.md

main @ 6c8c1bfb this PR @ 3fbad818
Median frame time (all settings) 10.3-11.6 ms within 0.3 ms of main
Evictions over the budget 249-251 248-256
Sampled EMPTY 0 0
(the rows in docs/tested-configurations.md, dated 2026-11-11)

In those Hyrule Field runs the rule kept nothing. It kept textures only in Hookshot's Dodongo's Cavern run at a 512 MB budget (both shadow maps no longer sampled EMPTY). Under the new bound, that budget's cap is 64 MB, so the 288 MB map would now go there by design. T-DCLN therefore uses a 3000 MB budget.

Testing

Tested: python -I -B tests/run_all.py on Linux with the Vulkan and SPIR-V headers, on main f3fe1763: 513 passed. test_lift_cache and test_menu_i18n fail, and both fail the same way on main. test_keep_resampled passes, and its checks fail with the rule's bounds taken out one at a time (no budget/8, no 97% line, no gpu_owned, no shared cap). gpu/vk/vk_texture.cpp compiles (clang++ with the renderer's flags, syntax only). The Hookshot runs above are of the earlier version.

Not tested: any in-game run of Longshot with this branch. I could not run the game here, and this head with the bound and the shared cap has not been run in the game anywhere. Windows build.

Needs other testers:

  • a run where the rule keeps textures: RX_VK_VRAM_MB=3000, Dodongo's Cavern and Hyrule Field shadows, against main and RX_VK_KEEP_RESAMPLED_MB=0. The author's RTX 4070 Ti can do this one: T-DCLN
  • a 4-8 GB card at the driver's limit (Hyrule Field, Castle Town market), watching VRAM use and any device lost against main: T-J6DC

Screenshots

Nothing new on screen when it works: the shadows stay as they should.

Risk and rollback

The VRAM kept over the budget is no more than on main. The two rules share RX_VK_KEEP_FINALIZED_MB's 512 MB, this rule's part is at most an eighth of the budget, and it is nothing at 97% of the driver's limit. The trade: while the shadow maps are kept (320 MB in Dodongo's Cavern), keep_finalized has 192 MB for the terrain's small finalized textures (the square streaks on the ground that keep_finalized was added for) instead of 512 MB. If those streaks come back over the budget, that is why. RX_VK_KEEP_RESAMPLED_MB=0 brings back the old behavior.

Reviewer notes

keep_resampled.h holds the rule: candidate, cap, finalized_room and keep. vk_texture.cpp's eviction pass calls it. The cap uses the budget as a size (size_budget), before the 90% rule and the compaction's hold take an excess off it, so it stays steady from pass to pass. The driver's limit is the 97% line's job.

Issues

Ports Hookshot PR 2368, which addresses Hookshot issue 2209. Bounded with Longshot issue 130 in mind. No Longshot issue tracks the shadow map yet.

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (On Hookshot, before the bound; T-DCLN repeats it on Longshot.)
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags noted, and the switch reference updated (docs/development.md, the README).
  • A row in docs/tested-configurations.md for each measured run.
  • A game-free test added for the fix, failing on main and passing here.
  • Docs updated.
## Why The game draws its large shadow map once and then samples it for many frames. When textures went over the VRAM budget, the renderer evicted it and made it again empty, so shadows went missing or wrong and the ground turned dark ([Hookshot issue 2209](https://github.com/NotRexed/Hookshot/issues/2209)) until the game drew it again some 40 frames later. This was [Hookshot PR 2368](https://github.com/NotRexed/Hookshot/pull/2368). It wasn't merged before the GitHub repo went down, so I'm bringing it here, bounded so it can't add VRAM pressure on small cards. ## What changed - vk: `gpu/vk/keep_resampled.h` (new) and `gpu/vk/vk_texture.cpp`'s eviction pass. A texture the GPU drew that the game samples again after finalizing it is marked (`Texture::resampled`, `gpu/vk/vk_texture.h`) and kept through evictions over the budget. It stays a candidate only while it was used in the last 1,800 presents and its storage is still the GPU's (`gpu_owned`, the check `keep_finalized` already makes). When another render target takes that range, for example after an area change, the pin goes right away. - vk: the rule is bounded. Its cap for each pass is the smallest of `RX_VK_KEEP_RESAMPLED_MB` (512), an eighth of the texture budget (128 MB on a 1 GB budget, 200 MB on 1.6 GB) and `keep_finalized`'s cap. It is 0 once VRAM use reaches 97% of what the driver allows, the same line `keep_rendered` uses. Past that line the textures go as they did on main, and the small ones are `keep_finalized`'s again. - vk: one cap for both rules. What this rule may hold in a pass (its candidates' total, at most its cap) comes off `keep_finalized`'s 512 MB. Together the two hold no more than `keep_finalized` held alone on main. When the cap is reached, the least recently used go first. - tests: `tests/test_keep_resampled.py` and `.cpp` check each rule: the budget/8 cap, nothing kept at 97% (the same line as `keep_rendered`, checked across a range), the pin released once the storage is no longer the GPU's, and the shared cap never exceeded. The source checks find the pass's calls. Without the change the checks fail. - docs: the switch in `docs/development.md`, and the capped-VRAM runs in `docs/tested-configurations.md` (from Hookshot, before this bound). ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `RX_VK_KEEP_RESAMPLED_MB=<MB>` | Most the drawn-and-resampled textures may hold through evictions. The real cap is also at most an eighth of the texture budget and comes out of `RX_VK_KEEP_FINALIZED_MB`'s room. It is 0 at 97% of the driver's limit. `0` gives the old behavior | 512 | On by default: it only acts over the budget, takes no VRAM beyond what `keep_finalized` could already hold, and steps aside at the driver's limit | Cards that go over the texture budget with headroom left (a budget of 2.6 GB or more keeps both shadow maps) | Cards that never evict. Budgets under about 2.6 GB keep only what fits in an eighth (the 32 MB map, not the 288 MB one). Cards already at the driver's limit | AMD, Intel, Steam Deck, Windows; any Longshot in-game run (see Testing) | ## Measurements No Longshot measurement yet. The numbers below are from Hookshot, measured on the version the review saw (a 512 MB cap of its own, no budget or driver-limit bound, no shared cap). They show what the rule costs when it acts, but they don't measure this head. The PR stays WIP until T-DCLN and T-J6DC are done. main: Hookshot `6c8c1bfb` (2026-11-11) this PR: Hookshot `fix/keep-resampled-drawn` @ `3fbad818` Hardware: RTX 4070 Ti 12 GB, 595.99.02, Ryzen 7 9800X3D, 32 GB, Ubuntu 26.04.1 Scene: Hyrule Field, 2560x1440 window, 2048x1152 output, `HOS_FPS_MAX=240` Method: alternating scripted 3-minute runs with `RX_VK_VRAM_MB` at 4000, 2000 and 1000, and at 4000 with 3.8 GB of VRAM held by another process; `RX_VK_LOG_REMADE=1` How to reproduce: the row dated 2026-11-11 in `docs/tested-configurations.md` | | main @ 6c8c1bfb | this PR @ 3fbad818 | |---|---|---| | Median frame time (all settings) | 10.3-11.6 ms | within 0.3 ms of main | | Evictions over the budget | 249-251 | 248-256 | | Sampled EMPTY | 0 | 0 | ``` (the rows in docs/tested-configurations.md, dated 2026-11-11) ``` In those Hyrule Field runs the rule kept nothing. It kept textures only in Hookshot's Dodongo's Cavern run at a 512 MB budget (both shadow maps no longer sampled EMPTY). Under the new bound, that budget's cap is 64 MB, so the 288 MB map would now go there by design. T-DCLN therefore uses a 3000 MB budget. ## Testing **Tested:** `python -I -B tests/run_all.py` on Linux with the Vulkan and SPIR-V headers, on `main` `f3fe1763`: 513 passed. `test_lift_cache` and `test_menu_i18n` fail, and both fail the same way on `main`. `test_keep_resampled` passes, and its checks fail with the rule's bounds taken out one at a time (no budget/8, no 97% line, no `gpu_owned`, no shared cap). `gpu/vk/vk_texture.cpp` compiles (clang++ with the renderer's flags, syntax only). The Hookshot runs above are of the earlier version. **Not tested:** any in-game run of Longshot with this branch. I could not run the game here, and this head with the bound and the shared cap has not been run in the game anywhere. Windows build. **Needs other testers:** - a run where the rule keeps textures: `RX_VK_VRAM_MB=3000`, Dodongo's Cavern and Hyrule Field shadows, against main and `RX_VK_KEEP_RESAMPLED_MB=0`. The author's RTX 4070 Ti can do this one: T-DCLN - a 4-8 GB card at the driver's limit (Hyrule Field, Castle Town market), watching VRAM use and any device lost against main: T-J6DC ## Screenshots Nothing new on screen when it works: the shadows stay as they should. ## Risk and rollback The VRAM kept over the budget is no more than on main. The two rules share `RX_VK_KEEP_FINALIZED_MB`'s 512 MB, this rule's part is at most an eighth of the budget, and it is nothing at 97% of the driver's limit. The trade: while the shadow maps are kept (320 MB in Dodongo's Cavern), `keep_finalized` has 192 MB for the terrain's small finalized textures (the square streaks on the ground that `keep_finalized` was added for) instead of 512 MB. If those streaks come back over the budget, that is why. `RX_VK_KEEP_RESAMPLED_MB=0` brings back the old behavior. ## Reviewer notes `keep_resampled.h` holds the rule: `candidate`, `cap`, `finalized_room` and `keep`. `vk_texture.cpp`'s eviction pass calls it. The cap uses the budget as a size (`size_budget`), before the 90% rule and the compaction's hold take an excess off it, so it stays steady from pass to pass. The driver's limit is the 97% line's job. ## Issues Ports [Hookshot PR 2368](https://github.com/NotRexed/Hookshot/pull/2368), which addresses [Hookshot issue 2209](https://github.com/NotRexed/Hookshot/issues/2209). Bounded with [Longshot issue 130](https://git.magicbean.dev/MagicBeanDev/Longshot/issues/130) in mind. No Longshot issue tracks the shadow map yet. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [ ] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. (On Hookshot, before the bound; T-DCLN repeats it on Longshot.) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md, the README). - [x] A row in `docs/tested-configurations.md` for each measured run. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] Docs updated.
fix(vk): keep the textures the GPU drew that the game samples again after finalizing them (a cached shadow map)
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
c75a3c3ef2
The game draws its 12288 x 12288 shadow map once, finalizes it and samples it for many frames. It is depth, so it cannot be
written back; over the budget the eviction pass let it go, it was made again over storage the GPU rendered (not uploaded) and
sampled EMPTY until the game drew it again some 40 frames later: shadows missing or wrong, and the dark ground of Hookshot
issue 2209. Such a texture is now marked where it is seen sampled again after finalizing (Texture::resampled) and kept
through evictions over the budget, up to RX_VK_KEEP_RESAMPLED_MB (512 by default; 0: as before).

- The candidates' bytes are summed before the pass; walking least recently used first, the oldest go until the rest fit, so
  the cap is spent on the most recently used (as keep_finalized does).
- Only one used in the last 1,800 presents is a candidate (the threshold after which the reserve lets a long-unused
  texture go).
- resampled is cleared wherever released goes back to false (the texture is drawn again).
- keep_finalized leaves out what keep_resampled counts while keep_resampled is on, so no texture is in both sums.
- keep_resampled.h, tests/test_keep_resampled.py; the capped-VRAM runs against main are in docs/tested-configurations.md.

Ported from Hookshot (NotRexed/Hookshot#2368, branch fix/keep-resampled-drawn).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-authored-by: Cursor <[email protected]>
TrashPanda force-pushed fix/keep-resampled-drawn from c75a3c3ef2
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 87610ac035
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
2026-11-13 14:45:21 +00:00
Compare
HumanBean left a comment

Code review only (no build or run). The fix itself looks right. A finalized texture the GPU drew is marked resampled when a sampling touch reaches it, and the mark is cleared at both places released is cleared. The pass sums the candidates first and lets the oldest go until the rest fit the cap, the same way as keep_finalized. Nothing new for threads: resampled is written and read where released and last_frame already are.

What I'd like changed before this leaves WIP is the VRAM it can pin on small cards:

  • Nothing lets it go at the driver's limit. The keep runs before every other check in the pass. It holds up to 512 MB whatever drove the pass: the budget, the 90% rule or the driver's limit. keep_rendered drops from 3600 to 600 presents once use is at 97% of what the driver allows. This rule has no step like that, and no step that scales with the budget. On a 4 GB card or an iGPU (budget about 1-1.6 GB), 512 MB is a third to a half of the budget. That is the state of #130 (an 8 GB RX 6600 at the driver's limit, then device lost). Please bound it: for example a cap tied to the budget (min(512 MB, budget/8)), and 0 or the old behaviour when known && used >= allowed/100*97.
  • It raises the total kept, not only moves it. Resampled textures used to count inside keep_finalized's 512 MB. Now they have their own 512 MB, and they free their old room in keep_finalized for other textures. Together the two rules can hold 1 GB over the budget instead of 512 MB.
  • Area changes. A pin goes only with age (1,800 presents: 7.5 s at 240 fps, 90 s at 20) or when another target is drawn at the same start. After an area change, when the new area loads and pressure is highest, the old area's shadow maps can stay for that long. finalized_once already checks that the storage still belongs to the GPU (gpu_owned); candidate doesn't. Adding that check would release a pin as soon as the game gives its memory to something else.
  • Evidence. The tested-configurations row says the measured Hyrule Field runs kept nothing: the rule held textures only in the two runs that were left out. Only the 12 GB card at a 512 MB budget (Dodongo's Cavern) shows it working. To take it out of WIP: one Longshot run where it keeps textures, plus a run on a 4-8 GB card at the driver's limit (VRAM used, any device lost), as the description already plans.

Smaller points:

Code review only (no build or run). The fix itself looks right. A finalized texture the GPU drew is marked `resampled` when a sampling touch reaches it, and the mark is cleared at both places `released` is cleared. The pass sums the candidates first and lets the oldest go until the rest fit the cap, the same way as `keep_finalized`. Nothing new for threads: `resampled` is written and read where `released` and `last_frame` already are. What I'd like changed before this leaves WIP is the VRAM it can pin on small cards: - **Nothing lets it go at the driver's limit.** The keep runs before every other check in the pass. It holds up to 512 MB whatever drove the pass: the budget, the 90% rule or the driver's limit. `keep_rendered` drops from 3600 to 600 presents once use is at 97% of what the driver allows. This rule has no step like that, and no step that scales with the budget. On a 4 GB card or an iGPU (budget about 1-1.6 GB), 512 MB is a third to a half of the budget. That is the state of https://git.magicbean.dev/MagicBeanDev/Longshot/issues/130 (an 8 GB RX 6600 at the driver's limit, then device lost). Please bound it: for example a cap tied to the budget (min(512 MB, budget/8)), and 0 or the old behaviour when `known && used >= allowed/100*97`. - **It raises the total kept, not only moves it.** Resampled textures used to count inside `keep_finalized`'s 512 MB. Now they have their own 512 MB, and they free their old room in `keep_finalized` for other textures. Together the two rules can hold 1 GB over the budget instead of 512 MB. - **Area changes.** A pin goes only with age (1,800 presents: 7.5 s at 240 fps, 90 s at 20) or when another target is drawn at the same start. After an area change, when the new area loads and pressure is highest, the old area's shadow maps can stay for that long. `finalized_once` already checks that the storage still belongs to the GPU (`gpu_owned`); `candidate` doesn't. Adding that check would release a pin as soon as the game gives its memory to something else. - **Evidence.** The tested-configurations row says the measured Hyrule Field runs kept nothing: the rule held textures only in the two runs that were left out. Only the 12 GB card at a 512 MB budget (Dodongo's Cavern) shows it working. To take it out of WIP: one Longshot run where it keeps textures, plus a run on a 4-8 GB card at the driver's limit (VRAM used, any device lost), as the description already plans. Smaller points: - The PR's "Issues" section is still "See #". The code comments cite Hookshot's "#2198" and "issue 2209" bare, and those read as Longshot numbers. Please use full URLs: https://github.com/NotRexed/Hookshot/pull/2198, https://github.com/NotRexed/Hookshot/issues/2209 and https://github.com/NotRexed/Hookshot/pull/2368. - The pre-sum loop checks `resampled`, `gpu_written` and `recent` twice (`candidate` checks them again). In `test_keep_resampled.py`, one needle is a substring of the one before it.
@ -0,0 +14,4 @@
// rendered, not uploaded, and sampled EMPTY until the game drew it again some 40 frames later (Dodongo's Cavern at a 512 MB
// budget, issue 2209's dark ground in Hyrule Field).
//
// Only while used in the last kMaxUnused presents (unused for fewer), the compaction reserve's own line (#2198): one from an
Owner

"#2198" and "issue 2209" are Hookshot numbers, but read here as Longshot's. Full URLs please (AGENTS.md): https://github.com/NotRexed/Hookshot/pull/2198, https://github.com/NotRexed/Hookshot/issues/2209.

"#2198" and "issue 2209" are Hookshot numbers, but read here as Longshot's. Full URLs please (AGENTS.md): https://github.com/NotRexed/Hookshot/pull/2198, https://github.com/NotRexed/Hookshot/issues/2209.
@ -0,0 +19,4 @@
// unused 400 presents before the eviction took it.
constexpr uint64_t kMaxUnused = vram_compact::kReserveUnused;
inline bool recent(uint64_t unused) { return unused < kMaxUnused; }
inline bool candidate(bool overwritten, bool gpu_written, bool resampled, uint64_t unused) {
Owner

The only things that release a pin are age (1,800 presents: 90 s at 20 fps) and a GPU draw at the same start (drawn_over, or drawn_over_shifted for single-level render targets). If the game frees the shadow map's storage on an area change and uploads ordinary textures there, the old map stays pinned through the new area's load. finalized_once checks that gpu_owned still covers the texture's range; could the candidate check do the same, so the pin goes once the storage is someone else's?

The only things that release a pin are age (1,800 presents: 90 s at 20 fps) and a GPU draw at the same start (`drawn_over`, or `drawn_over_shifted` for single-level render targets). If the game frees the shadow map's storage on an area change and uploads ordinary textures there, the old map stays pinned through the new area's load. `finalized_once` checks that `gpu_owned` still covers the texture's range; could the candidate check do the same, so the pin goes once the storage is someone else's?
@ -4552,0 +4560,4 @@
// over the budget up to RX_VK_KEEP_RESAMPLED_MB (512 by default, the most recently used first; 0: evicted as before). Their
// total first, as finalized_kept's: keep_resampled.h
static const uint64_t keep_resampled_max =
(uint64_t)(getenv("RX_VK_KEEP_RESAMPLED_MB") ? std::max(0, atoi(getenv("RX_VK_KEEP_RESAMPLED_MB"))) : 512) << 20;
Owner

A fixed 512 MB default, separate from RX_VK_KEEP_FINALIZED_MB's 512. On an iGPU or 4 GB card with a budget of about 1-1.6 GB, this alone is a third to a half of it. The resampled textures also leave keep_finalized's total (line 4576), so that rule now has room for 512 MB of other textures. The two can hold 1 GB over the budget where main held 512 MB. Suggest min(512 MB, budget / 8) or similar, or one shared cap.

A fixed 512 MB default, separate from `RX_VK_KEEP_FINALIZED_MB`'s 512. On an iGPU or 4 GB card with a budget of about 1-1.6 GB, this alone is a third to a half of it. The resampled textures also leave `keep_finalized`'s total (line 4576), so that rule now has room for 512 MB of other textures. The two can hold 1 GB over the budget where main held 512 MB. Suggest `min(512 MB, budget / 8)` or similar, or one shared cap.
@ -4563,0 +4580,4 @@
uint64_t resampled_total = 0;
if (keep_resampled_max)
for (auto& [last, tex] : order)
if (tex->resampled && tex->gpu_written && keep_resampled::recent(frame_no - last) &&
Owner

Nit: tex->resampled && tex->gpu_written && keep_resampled::recent(...) repeats what candidate checks. candidate(drawn_over_shifted(tex) || drawn_over(tex), true, true, frame_no - last) after the cheap checks would read more simply. The order matters, though: keep the cheap checks first so the overlap search runs only for the few that are marked.

Nit: `tex->resampled && tex->gpu_written && keep_resampled::recent(...)` repeats what `candidate` checks. `candidate(drawn_over_shifted(tex) || drawn_over(tex), true, true, frame_no - last)` after the cheap checks would read more simply. The order matters, though: keep the cheap checks first so the overlap search runs only for the few that are marked.
@ -4625,0 +4648,4 @@
// (above. Oldest first, as keep_finalized. Past its cap, a small finalized one is keep_finalized's again, below)
const bool resampled_candidate =
keep_resampled::candidate(overwritten, tex->gpu_written, tex->resampled, frame_no - last);
if (resampled_candidate && keep_resampled::keep(tex->bytes, resampled_total, keep_resampled_max)) {
Owner

This keep runs before every other check in the pass. It holds up to 512 MB whatever drove the pass: the budget, the 90% rule or the driver's limit. keep_rendered::presents (a few lines up) shortens to 600 presents once used >= allowed/100*97. This rule has no step like that, and on a 4-8 GB card at the driver's limit it keeps half a GB pinned (the state of #130). Could it give way near the limit (cap 0, or the old behaviour, when known && used >= allowed/100*97) and scale its cap with budget?

This keep runs before every other check in the pass. It holds up to 512 MB whatever drove the pass: the budget, the 90% rule or the driver's limit. `keep_rendered::presents` (a few lines up) shortens to 600 presents once `used >= allowed/100*97`. This rule has no step like that, and on a 4-8 GB card at the driver's limit it keeps half a GB pinned (the state of https://git.magicbean.dev/MagicBeanDev/Longshot/issues/130). Could it give way near the limit (cap 0, or the old behaviour, when `known && used >= allowed/100*97`) and scale its cap with `budget`?
@ -0,0 +16,4 @@
"keep_resampled::candidate(overwritten, tex->gpu_written, tex->resampled, frame_no - last);",
"if (resampled_candidate && keep_resampled::keep(tex->bytes, resampled_total, keep_resampled_max)) {",
"keep_resampled::keep(tex->bytes, resampled_total, keep_resampled_max)",
"if (keep_resampled_max && tex->resampled && keep_resampled::recent(frame_no - tex->last_frame)) return false;",
Owner

Nit: this needle is a substring of the one before it, so it checks nothing new.

Nit: this needle is a substring of the one before it, so it checks nothing new.
HumanBean added this to the v3.0.0 milestone 2026-11-13 16:53:40 +00:00
TrashPanda force-pushed fix/keep-resampled-drawn from 87610ac035
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to ff0dbae4a8
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
2026-11-13 18:23:06 +00:00
Compare
Author
Contributor

Thanks @HumanBean. Rebased on main (fbd42024), new head ff0dbae4. The original commit is unchanged apart from the rebase (b6fe2703), and the answer to the review is one commit on top of it (ff0dbae4). Point by point:

Nothing lets it go at the driver's limit. ... Please bound it: for example a cap tied to the budget (min(512 MB, budget/8)), and 0 or the old behaviour when known && used >= allowed/100*97.

(also line comment on vk_texture.cpp, "Could it give way near the limit ... and scale its cap with budget?")

Done in ff0dbae4. keep_resampled::cap returns 0 when known && used >= allowed / 100 * 97, the same line keep_rendered::presents steps at. With the cap at 0 the pass behaves as on main: keep_finalized counts the small resampled textures again and the big ones go. Otherwise the cap is min(RX_VK_KEEP_RESAMPLED_MB, size_budget / 8, keep_finalized's cap). I used size_budget, the budget as a size before the 90% rule and the compaction's hold take an excess off it, so the cap doesn't swing from pass to pass. Near the driver's limit, the 97% line makes the decision. On a 1-1.6 GB budget the cap is 128-200 MB. The test checks 1 GB → 128, 1.6 GB → 200, 2.5 GB → 320 and 6 GB → 512. It also checks that, across 90-101% of the allowance, the cap is 0 exactly where keep_rendered steps to 600.

It raises the total kept, not only moves it. ... Together the two rules can hold 1 GB over the budget instead of 512 MB.

(also line comment on keep_resampled_max, "Suggest min(512 MB, budget / 8) or similar, or one shared cap.")

Done in ff0dbae4, with one shared cap. keep_finalized_max (RX_VK_KEEP_FINALIZED_MB, 512) is the cap for both rules. What keep_resampled may hold in a pass, min(its candidates' total, its cap), comes off keep_finalized's room (keep_resampled::finalized_room), and its own cap is never above the shared one. Together they hold at most 512 MB, the same as keep_finalized alone on main. The test checks this sum over budgets of 0.5-16 GB and totals of 0-1 GB.

Why a shared cap instead of putting the resampled textures back inside keep_finalized: keep_finalized only takes textures of 4 MB or less that were drawn in one frame. The shadow maps (288 MB and 32 MB) were never inside its 512 MB, so counting them "inside it again" would need its own size and drawn-once rules changed. A shared cap leaves keep_finalized as it is and bounds the total all the same. Resampled textures take their part first because the game has been seen sampling them again. keep_finalized's textures are kept on the chance that the game remakes them. The cost: while 320 MB of shadow maps are kept, keep_finalized has 192 MB for the terrain's small textures instead of 512. The description's Risk section says so. The kept finalized textures over N MB log line now prints that room, so this can be seen in a log.

Area changes. ... finalized_once already checks that the storage still belongs to the GPU (gpu_owned); candidate doesn't.

(also line comment on keep_resampled.h's candidate)

Done in ff0dbae4. candidate(overwritten, gpu_owned, gpu_written, resampled, unused) now requires the storage to still be the GPU's. vk_texture.cpp has one gpu_owns lambda, the check finalized_once made, and both rules use it. One limit, so it's on record: gpu_owned changes when a render target is drawn over that range (at the same start, or covering it), not when the CPU writes there. If the game puts render targets in the old area's memory, the pin goes at once. If it only uploads ordinary textures there, the pin still goes by age (1,800 presents) or when the texture is next touched and found written. The test covers the release (candidate(false, false, true, true, 0) is false).

Evidence. ... To take it out of WIP: one Longshot run where it keeps textures, plus a run on a 4-8 GB card at the driver's limit (VRAM used, any device lost).

Not done yet: I couldn't run the game here, so the PR stays WIP and Testing says so. I've opened two test requests:

  • T-DCLN: a run where the rule keeps textures. RX_VK_VRAM_MB=3000 (cap 375 MB, so both shadow maps fit), Dodongo's Cavern for 5 min, then Hyrule Field, against main and against RX_VK_KEEP_RESAMPLED_MB=0. TrashPanda's RTX 4070 Ti 12 GB can run this one. Note that the old Dodongo's Cavern run at a 512 MB budget would now keep only 64 MB, so the 288 MB map goes there by design. That's why this run uses a higher budget.
  • T-J6DC: a 4-8 GB card at the driver's limit, in Hyrule Field and the Castle Town market, with default settings, against main. It records the highest vk: VRAM: line and any device lost.

The relay's T-LYJB (from the testing wanted label) was claimed before this push. @d4nkn3ss, please test head ff0dbae4, not 87610ac0.

The PR's "Issues" section is still "See #". The code comments cite Hookshot's "#2198" and "issue 2209" bare ... Please use full URLs

(also line comment on keep_resampled.h)

Done in ff0dbae4 and in the description. keep_resampled.h links https://github.com/NotRexed/Hookshot/pull/2198 and https://github.com/NotRexed/Hookshot/issues/2209, and the test's message no longer says "#2198". The Issues section links Hookshot PR 2368, Hookshot issue 2209 and Longshot issue 130.

The pre-sum loop checks resampled, gpu_written and recent twice (candidate checks them again).

(also line comment on the pre-sum loop)

Done in ff0dbae4. The loop reads only the tex->resampled field first, then calls candidate(drawn_over_shifted(tex) || drawn_over(tex), gpu_owns(tex), tex->gpu_written, true, frame_no - last). The cheap check still comes first, so the overlap search runs only for the textures that are marked. gpu_written stays an argument: resampled doesn't imply it, because two places clear gpu_written and leave resampled set (the write-back into memory is one). The pass uses the same form.

In test_keep_resampled.py, one needle is a substring of the one before it.

(also line comment on the test)

Done in ff0dbae4. The needle is removed. No needle in the list is now a substring of another; I checked this mechanically. The new needles find cap(...), finalized_room(...), gpu_owns(tex) in both candidate calls and finalized_kept <= finalized_room. The test also fails if finalized_kept <= keep_finalized_max comes back.

Tests: python -I -B tests/run_all.py: 510 passed. test_lift_cache fails, and it fails the same way on main. test_recorder_sync timed out once while the PC was under heavy load and passed when run alone. I also took each new rule out of keep_resampled.h in turn (the budget/8 bound, the 97% line, the gpu_owned check, the shared room), and test_keep_resampled failed each time. One fix outside the new code: test_target_overlap reads finalized_once's last line, so that line keeps its exact text.

Thanks @HumanBean. Rebased on main (`fbd42024`), new head `ff0dbae4`. The original commit is unchanged apart from the rebase (`b6fe2703`), and the answer to the review is one commit on top of it (`ff0dbae4`). Point by point: > **Nothing lets it go at the driver's limit.** ... Please bound it: for example a cap tied to the budget (min(512 MB, budget/8)), and 0 or the old behaviour when `known && used >= allowed/100*97`. (also line comment on `vk_texture.cpp`, "Could it give way near the limit ... and scale its cap with `budget`?") Done in `ff0dbae4`. `keep_resampled::cap` returns 0 when `known && used >= allowed / 100 * 97`, the same line `keep_rendered::presents` steps at. With the cap at 0 the pass behaves as on main: `keep_finalized` counts the small resampled textures again and the big ones go. Otherwise the cap is `min(RX_VK_KEEP_RESAMPLED_MB, size_budget / 8, keep_finalized's cap)`. I used `size_budget`, the budget as a size before the 90% rule and the compaction's hold take an excess off it, so the cap doesn't swing from pass to pass. Near the driver's limit, the 97% line makes the decision. On a 1-1.6 GB budget the cap is 128-200 MB. The test checks 1 GB → 128, 1.6 GB → 200, 2.5 GB → 320 and 6 GB → 512. It also checks that, across 90-101% of the allowance, the cap is 0 exactly where `keep_rendered` steps to 600. > **It raises the total kept, not only moves it.** ... Together the two rules can hold 1 GB over the budget instead of 512 MB. (also line comment on `keep_resampled_max`, "Suggest `min(512 MB, budget / 8)` or similar, or one shared cap.") Done in `ff0dbae4`, with one shared cap. `keep_finalized_max` (`RX_VK_KEEP_FINALIZED_MB`, 512) is the cap for both rules. What `keep_resampled` may hold in a pass, `min(its candidates' total, its cap)`, comes off `keep_finalized`'s room (`keep_resampled::finalized_room`), and its own cap is never above the shared one. Together they hold at most 512 MB, the same as `keep_finalized` alone on main. The test checks this sum over budgets of 0.5-16 GB and totals of 0-1 GB. Why a shared cap instead of putting the resampled textures back inside `keep_finalized`: `keep_finalized` only takes textures of 4 MB or less that were drawn in one frame. The shadow maps (288 MB and 32 MB) were never inside its 512 MB, so counting them "inside it again" would need its own size and drawn-once rules changed. A shared cap leaves `keep_finalized` as it is and bounds the total all the same. Resampled textures take their part first because the game has been seen sampling them again. `keep_finalized`'s textures are kept on the chance that the game remakes them. The cost: while 320 MB of shadow maps are kept, `keep_finalized` has 192 MB for the terrain's small textures instead of 512. The description's Risk section says so. The `kept finalized textures over N MB` log line now prints that room, so this can be seen in a log. > **Area changes.** ... `finalized_once` already checks that the storage still belongs to the GPU (`gpu_owned`); `candidate` doesn't. (also line comment on `keep_resampled.h`'s `candidate`) Done in `ff0dbae4`. `candidate(overwritten, gpu_owned, gpu_written, resampled, unused)` now requires the storage to still be the GPU's. `vk_texture.cpp` has one `gpu_owns` lambda, the check `finalized_once` made, and both rules use it. One limit, so it's on record: `gpu_owned` changes when a render target is drawn over that range (at the same start, or covering it), not when the CPU writes there. If the game puts render targets in the old area's memory, the pin goes at once. If it only uploads ordinary textures there, the pin still goes by age (1,800 presents) or when the texture is next touched and found written. The test covers the release (`candidate(false, false, true, true, 0)` is false). > **Evidence.** ... To take it out of WIP: one Longshot run where it keeps textures, plus a run on a 4-8 GB card at the driver's limit (VRAM used, any device lost). Not done yet: I couldn't run the game here, so the PR stays WIP and Testing says so. I've opened two test requests: - **T-DCLN**: a run where the rule keeps textures. `RX_VK_VRAM_MB=3000` (cap 375 MB, so both shadow maps fit), Dodongo's Cavern for 5 min, then Hyrule Field, against main and against `RX_VK_KEEP_RESAMPLED_MB=0`. TrashPanda's RTX 4070 Ti 12 GB can run this one. Note that the old Dodongo's Cavern run at a 512 MB budget would now keep only 64 MB, so the 288 MB map goes there by design. That's why this run uses a higher budget. - **T-J6DC**: a 4-8 GB card at the driver's limit, in Hyrule Field and the Castle Town market, with default settings, against main. It records the highest `vk: VRAM:` line and any device lost. The relay's T-LYJB (from the `testing wanted` label) was claimed before this push. @d4nkn3ss, please test head `ff0dbae4`, not `87610ac0`. > The PR's "Issues" section is still "See #". The code comments cite Hookshot's "#2198" and "issue 2209" bare ... Please use full URLs (also line comment on `keep_resampled.h`) Done in `ff0dbae4` and in the description. `keep_resampled.h` links https://github.com/NotRexed/Hookshot/pull/2198 and https://github.com/NotRexed/Hookshot/issues/2209, and the test's message no longer says "#2198". The Issues section links Hookshot PR 2368, Hookshot issue 2209 and Longshot issue 130. > The pre-sum loop checks `resampled`, `gpu_written` and `recent` twice (`candidate` checks them again). (also line comment on the pre-sum loop) Done in `ff0dbae4`. The loop reads only the `tex->resampled` field first, then calls `candidate(drawn_over_shifted(tex) || drawn_over(tex), gpu_owns(tex), tex->gpu_written, true, frame_no - last)`. The cheap check still comes first, so the overlap search runs only for the textures that are marked. `gpu_written` stays an argument: `resampled` doesn't imply it, because two places clear `gpu_written` and leave `resampled` set (the write-back into memory is one). The pass uses the same form. > In `test_keep_resampled.py`, one needle is a substring of the one before it. (also line comment on the test) Done in `ff0dbae4`. The needle is removed. No needle in the list is now a substring of another; I checked this mechanically. The new needles find `cap(...)`, `finalized_room(...)`, `gpu_owns(tex)` in both candidate calls and `finalized_kept <= finalized_room`. The test also fails if `finalized_kept <= keep_finalized_max` comes back. Tests: `python -I -B tests/run_all.py`: 510 passed. `test_lift_cache` fails, and it fails the same way on main. `test_recorder_sync` timed out once while the PC was under heavy load and passed when run alone. I also took each new rule out of `keep_resampled.h` in turn (the budget/8 bound, the 97% line, the `gpu_owned` check, the shared room), and `test_keep_resampled` failed each time. One fix outside the new code: `test_target_overlap` reads `finalized_once`'s last line, so that line keeps its exact text.
Owner

@TrashPanda please add measurements

@TrashPanda please add measurements
TrashPanda force-pushed fix/keep-resampled-drawn from ff0dbae4a8
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
to d6219eb738
Some checks failed
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
2026-11-13 18:29:38 +00:00
Compare
Author
Contributor

Main moved while I worked, so I rebased again onto f3fe1763. The new head is d6219eb7. git range-diff shows both commits unchanged (738cda2d = b6fe2703, d6219eb7 = ff0dbae4), so everything in the answer above still applies, and so do the test requests T-DCLN and T-J6DC (their notes name ff0dbae4: same change). Tests on the rebased head: 513 passed. test_lift_cache and test_menu_i18n fail, and both fail the same way on main f3fe1763.

Main moved while I worked, so I rebased again onto `f3fe1763`. The new head is `d6219eb7`. `git range-diff` shows both commits unchanged (`738cda2d` = `b6fe2703`, `d6219eb7` = `ff0dbae4`), so everything in the answer above still applies, and so do the test requests T-DCLN and T-J6DC (their notes name `ff0dbae4`: same change). Tests on the rebased head: 513 passed. `test_lift_cache` and `test_menu_i18n` fail, and both fail the same way on `main` `f3fe1763`.
TrashPanda force-pushed fix/keep-resampled-drawn from d6219eb738
Some checks failed
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 3s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to aad9cc15c4
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 4s
2026-11-13 18:30:32 +00:00
Compare
Author
Contributor

Reworded d6219eb7's message only (an unwanted tool trailer removed), no code change: it is now aad9cc15. The test requests' code (ff0dbae4 / d6219eb7) is the same as aad9cc15.

Reworded d6219eb7's message only (an unwanted tool trailer removed), no code change: it is now aad9cc15. The test requests' code (ff0dbae4 / d6219eb7) is the same as aad9cc15.
Contributor

Test request T-LYJB: mixed. No regression with the rule on, but I couldn't reproduce the dark ground / missing shadows with it off, so this doesn't confirm the fix.

Hardware: RTX 3080 10 GB, NVIDIA 621.14, i9-12900K, Windows 11. Built 87610ac0 (this PR's head; it conflicts with main now, so the runs are on that head as it was). Headless, scripted input only, RX_VK_VRAM_MB capped so textures are evicted; an in-build A/B: RX_VK_KEEP_RESAMPLED_MB=0 (the old behaviour) against unset (512).

What I ran:

  • Main pass, RX_VK_VRAM_MB=1024: a warm-up, then four runs interleaved off/on/off/on. Each run: two laps of Kakariko → Hyrule Field → Market → Hyrule Field by the debug teleport, and 12 shots 2 s apart from 3 s after each Field arrival (48 a run). For each shot I measured the ground's mean brightness (the lower part of the picture), and counted a "dip" as a shot more than 25% darker than its visit's median.
  • Two more off runs, to find a place the bug shows: one walking out of the tunnel the Field teleport lands in, one in Dodongo's Cavern at RX_VK_VRAM_MB=512, as in your Hookshot walk.
off, run 1 on, run 1 off, run 2 on, run 2
over-budget evictions (sum of textures retired ...: budget N) 22,967 22,029 22,807 21,938
dark dips in 48 Field shots 0 1 0 0
device-lost / die lines 0 0 0 0
  • Evictions: about 4% fewer over-budget evictions with the rule on (22.0k against 22.9k), so it does keep textures through them.
  • No dips with the rule off, here or in the extra runs (0 of 48 in the open field after the walk out, 0 of 48 in Dodongo's Cavern at 512 MB). The one dip with the rule on is the camera, not the ground: the visit's other shots and the picture don't show a shadow fault.
  • The Field teleport lands Link in the tunnel by the Lost Woods bridge, in fog, where the shadow map hardly shows, which is why I tried the walk out and the cavern.
  • Nothing else drawn wrong in the shots I looked at with the rule on.

So the bug didn't show on this card at these spots and caps, and I can't say the PR fixes it. A run that shows it on the old behaviour first, ideally an 8 GB card in sunlit Hyrule Field away from the tunnel as you asked, or a screenshot pair from your Dodongo's Cavern walk, would settle it. The shots stay on this PC (game imagery from my dump), so I'm not attaching them.

Test request T-LYJB: mixed. No regression with the rule on, but I couldn't reproduce the dark ground / missing shadows with it off, so this doesn't confirm the fix. Hardware: RTX 3080 10 GB, NVIDIA 621.14, i9-12900K, Windows 11. Built 87610ac0 (this PR's head; it conflicts with main now, so the runs are on that head as it was). Headless, scripted input only, `RX_VK_VRAM_MB` capped so textures are evicted; an in-build A/B: `RX_VK_KEEP_RESAMPLED_MB=0` (the old behaviour) against unset (512). What I ran: - **Main pass, `RX_VK_VRAM_MB=1024`:** a warm-up, then four runs interleaved off/on/off/on. Each run: two laps of Kakariko → Hyrule Field → Market → Hyrule Field by the debug teleport, and 12 shots 2 s apart from 3 s after each Field arrival (48 a run). For each shot I measured the ground's mean brightness (the lower part of the picture), and counted a "dip" as a shot more than 25% darker than its visit's median. - **Two more off runs, to find a place the bug shows:** one walking out of the tunnel the Field teleport lands in, one in Dodongo's Cavern at `RX_VK_VRAM_MB=512`, as in your Hookshot walk. | | off, run 1 | on, run 1 | off, run 2 | on, run 2 | |---|---|---|---|---| | over-budget evictions (sum of `textures retired ...: budget N`) | 22,967 | 22,029 | 22,807 | 21,938 | | dark dips in 48 Field shots | 0 | 1 | 0 | 0 | | device-lost / die lines | 0 | 0 | 0 | 0 | - **Evictions:** about 4% fewer over-budget evictions with the rule on (22.0k against 22.9k), so it does keep textures through them. - **No dips with the rule off**, here or in the extra runs (0 of 48 in the open field after the walk out, 0 of 48 in Dodongo's Cavern at 512 MB). The one dip with the rule on is the camera, not the ground: the visit's other shots and the picture don't show a shadow fault. - **The Field teleport lands Link in the tunnel by the Lost Woods bridge**, in fog, where the shadow map hardly shows, which is why I tried the walk out and the cavern. - **Nothing else drawn wrong** in the shots I looked at with the rule on. So the bug didn't show on this card at these spots and caps, and I can't say the PR fixes it. A run that shows it on the old behaviour first, ideally an 8 GB card in sunlit Hyrule Field away from the tunnel as you asked, or a screenshot pair from your Dodongo's Cavern walk, would settle it. The shots stay on this PC (game imagery from my dump), so I'm not attaching them.
HumanBean left a comment

Re-review at aad9cc15. Code only: I didn't build it, run the tests or run the game.

Earlier points:

  • Nothing lets it go at the driver's limit: addressed. keep_resampled::cap is min(RX_VK_KEEP_RESAMPLED_MB, keep_finalized's cap, size_budget / 8) and 0 when known && used >= allowed / 100 * 97. That is the same used, allowed and line as keep_rendered::presents, and the test checks that both rules step at the same point. With the cap at 0, everything goes back to main's behaviour: resampled_candidate is false, keep_finalized no longer leaves resampled textures out, and finalized_room is the full 512 MB. Using size_budget (before the 90% rule shrinks it) is fine: the 97% line covers the driver's limit.
  • It raises the total kept: addressed. What this rule may hold comes off keep_finalized's room (shared - min(shared, total, cap)), and its cap is never above the shared one. I worked through the case where a resampled texture goes past the cap and joins finalized_kept: the two rules still hold at most 512 MB together. The Risk section names the trade-off (192 MB for the terrain's textures while 320 MB of shadow maps are kept).
  • Area changes: addressed. candidate now requires gpu_owns(tex), the same check finalized_once makes, in the pre-sum and in the pass. Thanks for writing down the limit (a CPU upload into the old range doesn't release the pin; age or the next touch does).
  • Evidence: not addressed yet. See "Measurements" below.
  • Full URLs: addressed in keep_resampled.h, in the test and in the Issues section.
  • Pre-sum repeats candidate's checks: addressed. The cheap tex->resampled check comes first, and gpu_written stays an argument for the reason you gave.
  • Test needle that was a substring of another: addressed. The new needles cover the cap, the room, gpu_owns in both calls, and the old <= keep_finalized_max line coming back.

New since the review: I found nothing wrong in the rework. resampled is cleared at both places released is, and the source check makes sure that stays true. The comment beside finalized_after_resampled says honestly that older textures walked before it were judged without it. That only keeps a little more, and the 512 MB bound still holds.

Measurements ("please add measurements"): there are no Longshot measurements yet. The Measurements section still has only the Hookshot runs of the earlier version (512 MB cap, no bound), and it says so. That's why the description check fails on the main: and this PR: lines and the Measured box is unticked. T-LYJB doesn't fill this gap either:

  • d4nkn3ss built 87610ac0, the head from before the bound. At their RX_VK_VRAM_MB=1024 this head's cap is 128 MB, not 512, so their numbers (about 4% fewer evictions over the budget, no regression) don't carry over.
  • They couldn't make the bug show with the rule off (0 dips in 48 Field shots, 0 at 512 MB in Dodongo's Cavern). So there is still no Longshot run where the fix is seen working.
  • The Testing section doesn't mention T-LYJB yet. Please add it, with what it does and doesn't show.

Verdict: the code is good to me. Only the WIP gates are left, and they are the earlier "Evidence" point:

  1. A Longshot run of this head (aad9cc15 or a later rebase) where the rule keeps textures, such as T-DCLN: RX_VK_VRAM_MB=3000, Dodongo's Cavern then Hyrule Field, against main and RX_VK_KEEP_RESAMPLED_MB=0. It should show the bug with the rule off first: the shadow maps' sampled EMPTY lines with RX_VK_LOG_REMADE=1, and the dark ground. Then it should show them gone, with the kept, not evicted lines, with the rule on. Give frame times from at least 2 interleaved runs each way.
  2. A 4-8 GB card at the driver's limit (T-J6DC): the highest vk: VRAM: line and any device lost, against main.
  3. Then the Measurements section with the main: <hash> (<date>) and this PR: <branch> @ <hash> lines and Longshot numbers, the Measured box ticked, a tested-configurations row for each run, and pr ready.

Main has moved past the base (two menu and input commits). They don't touch these files, so no rebase is needed for review.

Re-review at aad9cc15. Code only: I didn't build it, run the tests or run the game. Earlier points: - **Nothing lets it go at the driver's limit:** addressed. `keep_resampled::cap` is `min(RX_VK_KEEP_RESAMPLED_MB, keep_finalized's cap, size_budget / 8)` and 0 when `known && used >= allowed / 100 * 97`. That is the same `used`, `allowed` and line as `keep_rendered::presents`, and the test checks that both rules step at the same point. With the cap at 0, everything goes back to main's behaviour: `resampled_candidate` is false, `keep_finalized` no longer leaves resampled textures out, and `finalized_room` is the full 512 MB. Using `size_budget` (before the 90% rule shrinks it) is fine: the 97% line covers the driver's limit. - **It raises the total kept:** addressed. What this rule may hold comes off `keep_finalized`'s room (`shared - min(shared, total, cap)`), and its cap is never above the shared one. I worked through the case where a resampled texture goes past the cap and joins `finalized_kept`: the two rules still hold at most 512 MB together. The Risk section names the trade-off (192 MB for the terrain's textures while 320 MB of shadow maps are kept). - **Area changes:** addressed. `candidate` now requires `gpu_owns(tex)`, the same check `finalized_once` makes, in the pre-sum and in the pass. Thanks for writing down the limit (a CPU upload into the old range doesn't release the pin; age or the next touch does). - **Evidence:** not addressed yet. See "Measurements" below. - **Full URLs:** addressed in `keep_resampled.h`, in the test and in the Issues section. - **Pre-sum repeats `candidate`'s checks:** addressed. The cheap `tex->resampled` check comes first, and `gpu_written` stays an argument for the reason you gave. - **Test needle that was a substring of another:** addressed. The new needles cover the cap, the room, `gpu_owns` in both calls, and the old `<= keep_finalized_max` line coming back. New since the review: I found nothing wrong in the rework. `resampled` is cleared at both places `released` is, and the source check makes sure that stays true. The comment beside `finalized_after_resampled` says honestly that older textures walked before it were judged without it. That only keeps a little more, and the 512 MB bound still holds. **Measurements ("please add measurements"):** there are no Longshot measurements yet. The Measurements section still has only the Hookshot runs of the earlier version (512 MB cap, no bound), and it says so. That's why the description check fails on the `main:` and `this PR:` lines and the Measured box is unticked. T-LYJB doesn't fill this gap either: - d4nkn3ss built 87610ac0, the head from before the bound. At their `RX_VK_VRAM_MB=1024` this head's cap is 128 MB, not 512, so their numbers (about 4% fewer evictions over the budget, no regression) don't carry over. - They couldn't make the bug show with the rule off (0 dips in 48 Field shots, 0 at 512 MB in Dodongo's Cavern). So there is still no Longshot run where the fix is seen working. - The Testing section doesn't mention T-LYJB yet. Please add it, with what it does and doesn't show. Verdict: the code is good to me. Only the WIP gates are left, and they are the earlier "Evidence" point: 1. A Longshot run of this head (aad9cc15 or a later rebase) where the rule keeps textures, such as T-DCLN: `RX_VK_VRAM_MB=3000`, Dodongo's Cavern then Hyrule Field, against main and `RX_VK_KEEP_RESAMPLED_MB=0`. It should show the bug with the rule off first: the shadow maps' `sampled EMPTY` lines with `RX_VK_LOG_REMADE=1`, and the dark ground. Then it should show them gone, with the `kept, not evicted` lines, with the rule on. Give frame times from at least 2 interleaved runs each way. 2. A 4-8 GB card at the driver's limit (T-J6DC): the highest `vk: VRAM:` line and any device lost, against main. 3. Then the Measurements section with the `main: <hash> (<date>)` and `this PR: <branch> @ <hash>` lines and Longshot numbers, the Measured box ticked, a tested-configurations row for each run, and `pr ready`. Main has moved past the base (two menu and input commits). They don't touch these files, so no rebase is needed for review.
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 4s
This pull request is marked as a work in progress.
Some workflows are waiting to be reviewed.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'fix/keep-resampled-drawn:TrashPanda-fix/keep-resampled-drawn'
git switch 'TrashPanda-fix/keep-resampled-drawn'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!211
No description provided.