WIP: fix(vk): let render targets go as newer ones together cover them, and pool their images at once #943

Draft
Amateurwizrd wants to merge 1 commit from Amateurwizrd/Longshot:fix/targets-covered-together into main
Contributor

Type: fix · Risk: medium · Tested on: Windows 11, RTX 5070 Ti 16 GB · Needs: a card short of VRAM, Linux

Why

The renderer keeps a dead render target's image until one newer target draws over an eighth of it. Each newer target
is judged alone, so an old target covered a sixteenth at its head by one and a sixteenth at its tail by another is
kept, though together they drew over an eighth of it. In Hookshot that let target images grow to 3-4 GB in one room
until VRAM reached the driver's allowance and the game stopped (Hookshot-old's 7bba12ad, its issue 2180). On a 6 GB card #939 shows the same
kind of growth (render-target images 111 to 232 over a 31-minute tour, VRAM past the allowance).

How to reproduce

Seen on: the game-free check below, Windows 11, RTX 5070 Ti, driver 621.01; not seen in the game here (no scene tried grew the target images on a 16 GB card)
Settings and switches: defaults
Steps: run tests/test_target_overlap.py with this PR's tests/test_target_overlap.cpp against main's texture_frame: an old target with newer targets over a sixteenth at its head and a sixteenth at its tail
Expected: the old target counts as drawn over and is let go
Actual: it is kept

What changed

The parts of an old target that newer targets drew over are now added up, so several partial overlaps that together
reach an eighth retire it. A retired target's image goes into the target pool at once when the GPU is done with it, so
the next frame's target of the same shape can reuse it.

  • vk: target_overlap.h replaces shifted_over with ShiftedOver, which adds up the covered parts, merging them as
    by_start hands candidates over in start order. Views inside the target, the same-start case and targets written
    before its last use still count for nothing. drawn_over_shifted in texture_frame uses the sum and stops at the
    first point where an eighth is reached; keep_finalized follows the same rule.
  • vk: destroy_later(tex, completed) in gpu/vk/vk_texture.cpp pools a render target's image at eviction when its
    last use is complete (value <= completed); anything still in flight goes through the dead list as before.
    Hookshot pools at once with no condition; here a texture whose write-back timed out can still be read by the GPU,
    and the unconditional rule fails main's test_readback_timeout_source.
  • tests: test_target_overlap cuts drawn_over_shifted out of texture_frame and runs it over stub textures, with
    cases for parts that add up; test_target_image_reuse (new) runs struct Dead, destroy_later and the dead-list
    loop; test_readback_timeout_source, test_readback_timeout_reuse and test_target_image_pool follow the new
    destroy_later signature.

The rule was first seen in Hookshot-old's 7bba12ad; the code is written for Longshot.

Measurements

Verdict: it passes: both checks fail on main @ 13e8b6da in both runs and pass on this PR @ b8eab0f3 in both; in the
game no change was measurable on a 16 GB card (the case needs a card short of VRAM).

Hardware: Windows 11 Pro 26200, RTX 5070 Ti 16 GB, driver 621.01, Ryzen 7 7800X3D, 32 GB
Scene: the game-free checks; and Hyrule Field (child) by debug teleport, standing 25 s then 5 s in the pause menu, RX_VK_TEST_ALLOWED_MB=6144
Method: the checks run 2 times on each side, interleaved, main with this PR's test files copied in. In the game, 3 rounds alternating main and a build of main with this PR and two other open eviction fixes (#941, #944)
How to reproduce: python -I -B tests/run_all.py target_overlap target_image_reuse on this branch and on main with the test files copied in

Before: main @ 13e8b6da After: this PR @ b8eab0f3
tests/test_target_overlap.py (run 1 / run 2) FAIL / FAIL pass / pass
tests/test_target_image_reuse.py (run 1 / run 2) FAIL / FAIL pass / pass
Hyrule Field at 6,144 MB: VRAM in use at the end, a run 6444, 5924, 5926 MB 6126, 5958, 5924 MB (the batch build)
main @ 13e8b6da with this PR's test files:
FAIL a sixteenth over its head and one over its tail, together an eighth, are at line 66
FAIL and the edges still add up beside one at line 70
this PR @ b8eab0f3:
target overlap: all checks passed

Flags and settings

None (the existing RX_VK_DEAD_TARGETS still turns the rule off).

Testing

Tested: on Windows 11 with the pinned llvm-mingw, the 38 checks that read this diff, one at a time, on this PR @
b8eab0f3 (main @ 13e8b6da): all pass. In the game, the runs above: no device loss, the HUD clean. One batch run had a
480x270 target (#1192) uploaded over its drawing 7 times where the other runs had it at most once; one run, not
explained yet.

Not tested: a scene whose target images grow (a card short of VRAM, or Hookshot's issue 2180 room); eviction counts
over a long session, which this changes.

Must be tested:

  • Windows, an 8 GB NVIDIA or AMD card: Hyrule Field and Castle Town for a few minutes on main and on this PR, expecting the log's render target images to stay lower here, no device loss and no target shown stale (Test Farm: @Beanstalk windows, a 6 or 8 GB card)
  • Linux (Test Farm: @Beanstalk linux, build the renderer): the renderer builds with this PR, expecting no error in gpu/vk/vk_texture.cpp or gpu/vk/target_overlap.h

Risks

What could break: more targets are let go than before, among them finalized textures keep_finalized used to
hold: their storage was drawn over, but a game that reads such a texture again would get it uploaded from memory.
Pooling at once reuses an image a frame earlier than before, only once its last use is complete.

Turning it off: RX_VK_DEAD_TARGETS=0 turns the dead-target rule off; or revert the commit.

Compatibility: Nothing changes.

Issues

See #939 (a 6 GB card: render-target images 111 to 232 after a tour, VRAM past the allowance, Hyrule Field 31 to 14 fps)

Checklist

Everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description) of the status line's type.
  • Why gives the reason plainly and links the issues that fit (the tracker searched).
  • Measurements has a verdict, or one line saying why there is nothing to measure.
  • Testing names what was tested, what wasn't, and what must still be tested, where and how.
  • Rebased on current main.
  • python -I -B tests/run_all.py passes (any failure that also fails on main is named).
  • 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.

When it applies

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (Windows here; Linux is in Must be tested)
**Type:** fix · **Risk:** medium · **Tested on:** Windows 11, RTX 5070 Ti 16 GB · **Needs:** a card short of VRAM, Linux ## Why The renderer keeps a dead render target's image until one newer target draws over an eighth of it. Each newer target is judged alone, so an old target covered a sixteenth at its head by one and a sixteenth at its tail by another is kept, though together they drew over an eighth of it. In Hookshot that let target images grow to 3-4 GB in one room until VRAM reached the driver's allowance and the game stopped (Hookshot-old's 7bba12ad, its issue 2180). On a 6 GB card #939 shows the same kind of growth (render-target images 111 to 232 over a 31-minute tour, VRAM past the allowance). ## How to reproduce **Seen on:** the game-free check below, Windows 11, RTX 5070 Ti, driver 621.01; not seen in the game here (no scene tried grew the target images on a 16 GB card) **Settings and switches:** defaults **Steps:** run `tests/test_target_overlap.py` with this PR's `tests/test_target_overlap.cpp` against main's `texture_frame`: an old target with newer targets over a sixteenth at its head and a sixteenth at its tail **Expected:** the old target counts as drawn over and is let go **Actual:** it is kept ## What changed The parts of an old target that newer targets drew over are now added up, so several partial overlaps that together reach an eighth retire it. A retired target's image goes into the target pool at once when the GPU is done with it, so the next frame's target of the same shape can reuse it. - vk: `target_overlap.h` replaces `shifted_over` with `ShiftedOver`, which adds up the covered parts, merging them as `by_start` hands candidates over in start order. Views inside the target, the same-start case and targets written before its last use still count for nothing. `drawn_over_shifted` in `texture_frame` uses the sum and stops at the first point where an eighth is reached; `keep_finalized` follows the same rule. - vk: `destroy_later(tex, completed)` in `gpu/vk/vk_texture.cpp` pools a render target's image at eviction when its last use is complete (`value <= completed`); anything still in flight goes through the dead list as before. Hookshot pools at once with no condition; here a texture whose write-back timed out can still be read by the GPU, and the unconditional rule fails main's `test_readback_timeout_source`. - tests: `test_target_overlap` cuts `drawn_over_shifted` out of `texture_frame` and runs it over stub textures, with cases for parts that add up; `test_target_image_reuse` (new) runs `struct Dead`, `destroy_later` and the dead-list loop; `test_readback_timeout_source`, `test_readback_timeout_reuse` and `test_target_image_pool` follow the new `destroy_later` signature. The rule was first seen in Hookshot-old's 7bba12ad; the code is written for Longshot. ## Measurements **Verdict:** it passes: both checks fail on main @ 13e8b6da in both runs and pass on this PR @ b8eab0f3 in both; in the game no change was measurable on a 16 GB card (the case needs a card short of VRAM). Hardware: Windows 11 Pro 26200, RTX 5070 Ti 16 GB, driver 621.01, Ryzen 7 7800X3D, 32 GB Scene: the game-free checks; and Hyrule Field (child) by debug teleport, standing 25 s then 5 s in the pause menu, `RX_VK_TEST_ALLOWED_MB=6144` Method: the checks run 2 times on each side, interleaved, main with this PR's test files copied in. In the game, 3 rounds alternating main and a build of main with this PR and two other open eviction fixes (#941, #944) How to reproduce: `python -I -B tests/run_all.py target_overlap target_image_reuse` on this branch and on main with the test files copied in | | Before: main @ 13e8b6da | After: this PR @ b8eab0f3 | |---|---|---| | `tests/test_target_overlap.py` (run 1 / run 2) | FAIL / FAIL | pass / pass | | `tests/test_target_image_reuse.py` (run 1 / run 2) | FAIL / FAIL | pass / pass | | Hyrule Field at 6,144 MB: VRAM in use at the end, a run | 6444, 5924, 5926 MB | 6126, 5958, 5924 MB (the batch build) | ``` main @ 13e8b6da with this PR's test files: FAIL a sixteenth over its head and one over its tail, together an eighth, are at line 66 FAIL and the edges still add up beside one at line 70 this PR @ b8eab0f3: target overlap: all checks passed ``` ### Flags and settings None (the existing `RX_VK_DEAD_TARGETS` still turns the rule off). ## Testing **Tested:** on Windows 11 with the pinned llvm-mingw, the 38 checks that read this diff, one at a time, on this PR @ b8eab0f3 (main @ 13e8b6da): all pass. In the game, the runs above: no device loss, the HUD clean. One batch run had a 480x270 target (`#1192`) uploaded over its drawing 7 times where the other runs had it at most once; one run, not explained yet. **Not tested:** a scene whose target images grow (a card short of VRAM, or Hookshot's issue 2180 room); eviction counts over a long session, which this changes. **Must be tested:** - Windows, an 8 GB NVIDIA or AMD card: Hyrule Field and Castle Town for a few minutes on main and on this PR, expecting the log's render target images to stay lower here, no device loss and no target shown stale (Test Farm: @Beanstalk windows, a 6 or 8 GB card) - Linux (Test Farm: @Beanstalk linux, build the renderer): the renderer builds with this PR, expecting no error in `gpu/vk/vk_texture.cpp` or `gpu/vk/target_overlap.h` ## Risks **What could break:** more targets are let go than before, among them finalized textures `keep_finalized` used to hold: their storage was drawn over, but a game that reads such a texture again would get it uploaded from memory. Pooling at once reuses an image a frame earlier than before, only once its last use is complete. **Turning it off:** `RX_VK_DEAD_TARGETS=0` turns the dead-target rule off; or revert the commit. **Compatibility:** Nothing changes. ## Issues See #939 (a 6 GB card: render-target images 111 to 232 after a tour, VRAM past the allowance, Hyrule Field 31 to 14 fps) ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [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. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. (Windows here; Linux is in Must be tested)
fix(vk): let render targets go as newer ones together cover them, and pool their images at once
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 / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (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
Website / Website on Cloudflare (pull_request_target) Has been skipped
beanstalk/approval waiting for a BigBean's 👍 on the request
PR description / Description follows the template (pull_request_target) Successful in 4s
b8eab0f388
The dead transient targets (texture_frame's drawn_over_shifted) were judged against one newer target at a time: each had to cover an eighth of the old one. A target two newer ones had covered between them, one over its head and one over its tail, each under an eighth, was kept. Hookshot-old saw its target images grow to 3-4 GB that way, VRAM at the driver's limit, the threads stopped.

- target_overlap.h: ShiftedOver adds up the parts newer targets from other starts drew over, merged in start order so an overlap counts once. A view inside the target and one written before its last use still count for nothing.
- vk_texture.cpp drawn_over_shifted uses it (keep_finalized's check goes with it).
- vk_texture.cpp destroy_later takes what the GPU has finished: a render target's image it is done with goes to the target pool at eviction, not at the next present, after the next frame was already recorded without it. One the GPU may still read (a timed-out write-back's source) waits in the dead list as before; Hookshot-old's version pooled it at once, which tests/test_readback_timeout_source.py catches.
- Tests: test_target_overlap cuts drawn_over_shifted out of texture_frame and runs it over stub textures (new cases for parts that add up); new test_target_image_reuse runs destroy_later and the dead list with stubs; the read-back and pool checks follow the new destroy_later signature.

First seen in Hookshot-old's 7bba12ad (https://github.com/NotRexed/Hookshot/pull/2766, https://github.com/NotRexed/Hookshot/issues/2180).

Assisted-by: Claude
Author
Contributor

@Beanstalk linux, build the renderer at this head: it should build with no error in the files this PR changes; no change in play expected from the build alone.

@Beanstalk linux, build the renderer at this head: it should build with no error in the files this PR changes; no change in play expected from the build alone.
Member

@Amateurwizrd only members of BigBeans can start the farm. One of them can approve this request by reacting 👍 to it.

@Amateurwizrd only members of `BigBeans` can start the farm. One of them can approve this request by reacting 👍 to it.
Collaborator

Possibly related, for context:

  • issue #939 Game: on a 6 GB card VRAM goes over the driver's allowance after a few area changes, Hyrule Field falls from 31 to 14 fps until a restart (GTX 1060, nightly-189) — Open report of render-target growth and VRAM pressure on a 6 GB card.
  • PR #518 WIP: fix(vk): cap the render-target image pool and count it in the texture budget — Open PR overlaps render-target image pool behavior.
Possibly related, for context: - issue #939 Game: on a 6 GB card VRAM goes over the driver's allowance after a few area changes, Hyrule Field falls from 31 to 14 fps until a restart (GTX 1060, nightly-189) — Open report of render-target growth and VRAM pressure on a 6 GB card. - PR #518 WIP: fix(vk): cap the render-target image pool and count it in the texture budget — Open PR overlaps render-target image pool behavior.
Author
Contributor

Tested in game: Windows 11, RTX 5070 Ti 16 GB, driver 621.01, Hyrule Field (child) by debug teleport, 25 s standing and 5 s in the pause menu at RX_VK_TEST_ALLOWED_MB=6144, 3 rounds alternating main @ 13e8b6da and a build of main with #941, #943 and #944 together. No device loss, the HUD clean on both sides; #941's skip fired once a run on the same remap. Not run in the game alone at this head.

Tested in game: Windows 11, RTX 5070 Ti 16 GB, driver 621.01, Hyrule Field (child) by debug teleport, 25 s standing and 5 s in the pause menu at RX_VK_TEST_ALLOWED_MB=6144, 3 rounds alternating main @ 13e8b6da and a build of main with #941, #943 and #944 together. No device loss, the HUD clean on both sides; #941's skip fired once a run on the same remap. Not run in the game alone at this head.
Author
Contributor

@Beanstalk windows, a 6 or 8 GB card: a tour of several areas (as in #939: Hyrule Field, Kokiri Forest, Lost Woods, Market, Kakariko) on main and on this PR, comparing the log's render-target image count and VRAM against the allowance at the end, and Hyrule Field's frame rate at the start and the end.

@Beanstalk windows, a 6 or 8 GB card: a tour of several areas (as in #939: Hyrule Field, Kokiri Forest, Lost Woods, Market, Kakariko) on main and on this PR, comparing the log's render-target image count and VRAM against the allowance at the end, and Hyrule Field's frame rate at the start and the end.
Member

@Amateurwizrd only members of BigBeans can start the farm. One of them can approve this request by reacting 👍 to it.

@Amateurwizrd only members of `BigBeans` can start the farm. One of them can approve this request by reacting 👍 to it.
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 / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (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
Website / Website on Cloudflare (pull_request_target) Has been skipped
beanstalk/approval waiting for a BigBean's 👍 on the request
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/targets-covered-together:Amateurwizrd-fix/targets-covered-together'
git switch 'Amateurwizrd-fix/targets-covered-together'
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!943
No description provided.