WIP: fix(vk): let render targets go as newer ones together cover them, and pool their images at once #943
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
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!943
Loading…
Reference in a new issue
No description provided.
Delete branch "Amateurwizrd/Longshot:fix/targets-covered-together"
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: 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.pywith this PR'stests/test_target_overlap.cppagainst main'stexture_frame: an old target with newer targets over a sixteenth at its head and a sixteenth at its tailExpected: 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.
target_overlap.hreplacesshifted_overwithShiftedOver, which adds up the covered parts, merging them asby_starthands candidates over in start order. Views inside the target, the same-start case and targets writtenbefore its last use still count for nothing.
drawn_over_shiftedintexture_frameuses the sum and stops at thefirst point where an eighth is reached;
keep_finalizedfollows the same rule.destroy_later(tex, completed)ingpu/vk/vk_texture.cpppools a render target's image at eviction when itslast 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.test_target_overlapcutsdrawn_over_shiftedout oftexture_frameand runs it over stub textures, withcases for parts that add up;
test_target_image_reuse(new) runsstruct Dead,destroy_laterand the dead-listloop;
test_readback_timeout_source,test_readback_timeout_reuseandtest_target_image_poolfollow the newdestroy_latersignature.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 @
13e8b6dain both runs and pass on this PR @b8eab0f3in both; in thegame 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=6144Method: 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_reuseon this branch and on main with the test files copied in13e8b6dab8eab0f3tests/test_target_overlap.py(run 1 / run 2)tests/test_target_image_reuse.py(run 1 / run 2)Flags and settings
None (the existing
RX_VK_DEAD_TARGETSstill 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 a480x270 target (
#1192) uploaded over its drawing 7 times where the other runs had it at most once; one run, notexplained 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:
gpu/vk/vk_texture.cpporgpu/vk/target_overlap.hRisks
What could break: more targets are let go than before, among them finalized textures
keep_finalizedused tohold: 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=0turns 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
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.Possibly related, for context:
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.@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.
@Amateurwizrd only members of
BigBeanscan start the farm. One of them can approve this request by reacting 👍 to it.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.