WIP: fix(vk): a texture whose upload failed is tried again, not left empty or stale #944

Draft
Amateurwizrd wants to merge 1 commit from Amateurwizrd/Longshot:fix/upload-failed-retry into main
Contributor

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

Why

When a texture's upload fails, for example for want of staging memory with VRAM or RAM full, only a BC3 texture is
tried again. Every other kind is marked uploaded: a first upload that failed leaves an empty image, drawn as it is
until the game writes that memory again, which for most textures is never; a re-upload that failed drops what the CPU
wrote since the last upload, and the GPU buffer write just marked as seen. Searched the tracker: no issue covers it.

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 upload failed in the scenes tried)
Settings and switches: defaults
Steps: run tests/test_upload_retry.py against main's texture_touch: an upload that fails, first and as a re-upload
Expected: the texture is tried again and keeps what it still owes
Actual: a texture whose first upload failed is marked uploaded, empty

What changed

A failed upload of any kind is now tried again 30 frames later, as BC3 already was, and nothing it still owes is taken
as done meanwhile.

  • vk: in texture_touch (gpu/vk/vk_texture.cpp), bc_retry_frame becomes retry_frame and covers every texture.
    On a failed upload uploaded and upload_gen stay as they were, gpu_wrote_seen goes back to its value before this
    touch, and an evicted_from_memory entry taken is put back, so the retry does not take the texture for rendered
    storage and mark it uploaded empty. At most 40 log lines.
  • vk: the retry's wait sits right after the epoch check, before gpu_wrote_into and the dirty scan, so a waiting
    texture takes nothing as seen. upload_never holds upload_inner's opening checks (no storage, multisampled, over
    2 GB and not sparse, packed depth-stencil): such images are still marked uploaded, not retried for ever.
  • tests: tests/test_upload_retry.py (new) cuts the end of texture_touch and upload_never from the source and runs
    them with stubs, and checks the order of the steps; tests/test_bc_upload_retry.py keeps its BC3 checks.

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

Measurements

Verdict: it passes: the new check fails on main @ 13e8b6da in both runs and passes on this PR @ 5aef051f in both; in
the game no upload failed on either side, so the retry was not exercised there.

Hardware: Windows 11 Pro 26200, RTX 5070 Ti 16 GB, driver 621.01, Ryzen 7 7800X3D, 32 GB
Scene: the game-free check; 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 check run 2 times on each side, interleaved, main with this PR's test file copied in. In the game, 3 rounds alternating main and a build of main with this PR and two other open eviction fixes (#941, #943)
How to reproduce: python -I -B tests/run_all.py upload_retry on this branch and on main with the test copied in

Before: main @ 13e8b6da After: this PR @ 5aef051f
tests/test_upload_retry.py (run 1 / run 2) FAIL / FAIL pass / pass
Failed uploads, a run (Hyrule Field, 6,144 MB) 0, 0, 0 0, 0, 0 (the batch build)

Flags and settings

None

Testing

Tested: on Windows 11 with the pinned llvm-mingw, the 38 checks that read this diff, one at a time, on this PR @
5aef051f (main @ 13e8b6da): all pass, test_bc_upload_retry among them. In the game, the runs above: no device loss.

Not tested: a failed upload in the game (none happened here); a virtual-pool texture whose partial re-upload fails:
texture_dirty updates page_map/map_sig before the upload, so its remapped ranges are still lost (Hookshot has the
same gap; closing it needs a whole re-upload or a saved remap list, left for another PR).

Must be tested:

  • Windows, an 8 GB card or 16 GB of RAM: a long session (Hyrule Field, Castle Town, Lake Hylia), expecting any upload failed lines to be followed by the texture drawing right 30 frames later instead of staying empty (by hand: a tester short of memory)
  • Linux (Test Farm: @Beanstalk linux, build the renderer): the renderer builds with this PR, expecting no error in gpu/vk/vk_texture.cpp

Risks

What could break: a texture whose upload keeps failing is now tried every 30 frames instead of drawn empty, which
costs a staging attempt each time while memory stays short.

Turning it off: revert the commit.

Compatibility: Nothing changes.

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:** low · **Tested on:** Windows 11, RTX 5070 Ti 16 GB · **Needs:** a card short of VRAM or RAM, Linux ## Why When a texture's upload fails, for example for want of staging memory with VRAM or RAM full, only a BC3 texture is tried again. Every other kind is marked uploaded: a first upload that failed leaves an empty image, drawn as it is until the game writes that memory again, which for most textures is never; a re-upload that failed drops what the CPU wrote since the last upload, and the GPU buffer write just marked as seen. Searched the tracker: no issue covers it. ## 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 upload failed in the scenes tried) **Settings and switches:** defaults **Steps:** run `tests/test_upload_retry.py` against main's `texture_touch`: an upload that fails, first and as a re-upload **Expected:** the texture is tried again and keeps what it still owes **Actual:** a texture whose first upload failed is marked uploaded, empty ## What changed A failed upload of any kind is now tried again 30 frames later, as BC3 already was, and nothing it still owes is taken as done meanwhile. - vk: in `texture_touch` (`gpu/vk/vk_texture.cpp`), `bc_retry_frame` becomes `retry_frame` and covers every texture. On a failed upload `uploaded` and `upload_gen` stay as they were, `gpu_wrote_seen` goes back to its value before this touch, and an `evicted_from_memory` entry taken is put back, so the retry does not take the texture for rendered storage and mark it uploaded empty. At most 40 log lines. - vk: the retry's wait sits right after the epoch check, before `gpu_wrote_into` and the dirty scan, so a waiting texture takes nothing as seen. `upload_never` holds `upload_inner`'s opening checks (no storage, multisampled, over 2 GB and not sparse, packed depth-stencil): such images are still marked uploaded, not retried for ever. - tests: `tests/test_upload_retry.py` (new) cuts the end of `texture_touch` and `upload_never` from the source and runs them with stubs, and checks the order of the steps; `tests/test_bc_upload_retry.py` keeps its BC3 checks. The rule was first seen in Hookshot-old's 71f26852; the code is written for Longshot. ## Measurements **Verdict:** it passes: the new check fails on main @ 13e8b6da in both runs and passes on this PR @ 5aef051f in both; in the game no upload failed on either side, so the retry was not exercised there. Hardware: Windows 11 Pro 26200, RTX 5070 Ti 16 GB, driver 621.01, Ryzen 7 7800X3D, 32 GB Scene: the game-free check; 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 check run 2 times on each side, interleaved, main with this PR's test file copied in. In the game, 3 rounds alternating main and a build of main with this PR and two other open eviction fixes (#941, #943) How to reproduce: `python -I -B tests/run_all.py upload_retry` on this branch and on main with the test copied in | | Before: main @ 13e8b6da | After: this PR @ 5aef051f | |---|---|---| | `tests/test_upload_retry.py` (run 1 / run 2) | FAIL / FAIL | pass / pass | | Failed uploads, a run (Hyrule Field, 6,144 MB) | 0, 0, 0 | 0, 0, 0 (the batch build) | ### Flags and settings None ## Testing **Tested:** on Windows 11 with the pinned llvm-mingw, the 38 checks that read this diff, one at a time, on this PR @ 5aef051f (main @ 13e8b6da): all pass, `test_bc_upload_retry` among them. In the game, the runs above: no device loss. **Not tested:** a failed upload in the game (none happened here); a virtual-pool texture whose partial re-upload fails: `texture_dirty` updates `page_map`/`map_sig` before the upload, so its remapped ranges are still lost (Hookshot has the same gap; closing it needs a whole re-upload or a saved remap list, left for another PR). **Must be tested:** - Windows, an 8 GB card or 16 GB of RAM: a long session (Hyrule Field, Castle Town, Lake Hylia), expecting any `upload failed` lines to be followed by the texture drawing right 30 frames later instead of staying empty (by hand: a tester short of memory) - Linux (Test Farm: @Beanstalk linux, build the renderer): the renderer builds with this PR, expecting no error in `gpu/vk/vk_texture.cpp` ## Risks **What could break:** a texture whose upload keeps failing is now tried every 30 frames instead of drawn empty, which costs a staging attempt each time while memory stays short. **Turning it off:** revert the commit. **Compatibility:** Nothing changes. ## 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): a texture whose upload failed is tried again, not left empty or stale
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 3s
5aef051fdb
When upload() fails in texture_touch (no staging memory, the VRAM or RAM full), only a BC3 texture was tried again 30 frames on. Every other kind was marked uploaded with upload_gen moved on: a first upload that failed left an empty image, drawn so until the game wrote that memory again, and a re-upload that failed dropped what the CPU had written since and the GPU buffer write gpu_wrote_into had just marked seen. One evicted and made again over storage render targets took also lost its evicted_from_memory entry, so the next try took it for rendered storage and marked it uploaded empty.

- One rule for every kind: Texture::bc_retry_frame becomes retry_frame. On a failed upload, uploaded and upload_gen stay as they were, gpu_wrote_seen goes back to its value from before this touch, a taken evicted_from_memory entry is put back, and the texture is tried again 30 frames on. A line is logged (40 at most); the BC3 line keeps its text.
- The wait moves to right after the epoch check, before gpu_wrote_into and the dirty scan, so a waiting texture takes nothing as seen.
- upload_never: the checks upload_inner starts with (no storage, multisampled, over 2 GB and not sparse, packed depth-stencil). Such an image is still marked uploaded, not tried every 30 frames for ever.
- Tests: new test_upload_retry cuts the end of texture_touch and upload_never out of the source and runs them with stubs, and reads the order of the checks; test_bc_upload_retry keeps its BC3-specific checks.

First seen in Hookshot-old's 71f26852 (https://github.com/NotRexed/Hookshot/pull/3003).

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.
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.
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 3s
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/upload-failed-retry:Amateurwizrd-fix/upload-failed-retry'
git switch 'Amateurwizrd-fix/upload-failed-retry'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 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!944
No description provided.