WIP: fix(vk): a texture whose upload failed is tried again, not left empty or stale #944
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/tooling
area/upscaling
beanstalk: approved
beanstalk: needs approval
beanstalk: running
ci: approved
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/macos
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
regression
risk
high
risk
low
risk
medium
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
ci: approved
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!944
Loading…
Reference in a new issue
No description provided.
Delete branch "Amateurwizrd/Longshot:fix/upload-failed-retry"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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.pyagainst main'stexture_touch: an upload that fails, first and as a re-uploadExpected: 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.
texture_touch(gpu/vk/vk_texture.cpp),bc_retry_framebecomesretry_frameand covers every texture.On a failed upload
uploadedandupload_genstay as they were,gpu_wrote_seengoes back to its value before thistouch, and an
evicted_from_memoryentry taken is put back, so the retry does not take the texture for renderedstorage and mark it uploaded empty. At most 40 log lines.
gpu_wrote_intoand the dirty scan, so a waitingtexture takes nothing as seen.
upload_neverholdsupload_inner's opening checks (no storage, multisampled, over2 GB and not sparse, packed depth-stencil): such images are still marked uploaded, not retried for ever.
tests/test_upload_retry.py(new) cuts the end oftexture_touchandupload_neverfrom the source and runsthem with stubs, and checks the order of the steps;
tests/test_bc_upload_retry.pykeeps 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 @
13e8b6dain both runs and passes on this PR @5aef051fin both; inthe 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=6144Method: 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_retryon this branch and on main with the test copied in13e8b6da5aef051ftests/test_upload_retry.py(run 1 / run 2)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_retryamong 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_dirtyupdatespage_map/map_sigbefore the upload, so its remapped ranges are still lost (Hookshot has thesame gap; closing it needs a whole re-upload or a saved remap list, left for another PR).
Must be tested:
upload failedlines to be followed by the texture drawing right 30 frames later instead of staying empty (by hand: a tester short of memory)gpu/vk/vk_texture.cppRisks
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
type(scope): description) of the status line's type.main.python -I -B tests/run_all.pypasses (any failure that also fails onmainis named).third_party/untouched (a new folder with aPROVENANCE.mdinstead).When it applies
Assisted-by:/Co-Authored-By:trailers.mainand passing here.tests/that reads changed source updated in this PR, not weakened.@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.
@Amateurwizrd only members of
BigBeanscan start the farm. One of them can approve this request by reacting 👍 to it.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 @
13e8b6daand 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.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.