Skip to content

fix(vk): remake write-back shadows given back under VRAM pressure - #2767

Open
Revens2 wants to merge 1 commit into
NotRexed:mainfrom
Revens2:fix/issue-2238-fps-drops-heavily-and-stays-low-afte
Open

Revens2 wants to merge 1 commit into
NotRexed:mainfrom
Revens2:fix/issue-2238-fps-drops-heavily-and-stays-low-afte

Conversation

@Revens2

@Revens2 Revens2 commented Nov 12, 2026 •

Copy link
Copy Markdown

Why

When VRAM is tight the renderer gives back its write-back shadows, and it made one again only when a pool's own write-back reached 256 KB. After a scene change a pool's write-backs can stay under that size for good, so the shadow never came back: in the log on #2238, 48 of 352 MB stayed whole copies for the rest of the session while the others were made again. That leaves the pool without a shadow and its next write-back reads the mirror instead.

Fixes #2238 - FPS drops heavily and stays low after entering a new scene

Issue: #2238

What changed

  • gpu/vk/vk_memory.cpp: a pool whose write-back was worth a shadow is marked (Pool::shadow_wanted), and the mark stays after VRAM takes the shadow.
  • shadows_check_room now runs every 120 write-back submissions when a wanted pool has a mirror and no shadow, even with shadow_bytes at 0. Once the pressure rule says there is room, it makes up to 8 shadows per check (each is an allocation of the pool's whole size), skipping a pool whose mirror is gone or whose shadow cannot fit now without spending a try. It logs what this check made, not the session total.
  • tests/test_wb_shadow_remake.py runs the real functions from the source over stubs: the remake after a give-back, only wanted pools, the issue's exact give-back sequence, 8 per check, retired pools never, mirror-less and too-big pools not spending tries, the make lines, and VRAM unknown.

Testing

Game-free: python -I -B tests/run_all.py in the worktree, full suite.

  • 462 passed, 1 failed, 33 skipped (Windows, llvm-mingw).
  • Failed: tests/test_input_background.py, a declared known environment failure on this PC (no MSVC SDK).
  • The two declared environment failures tests/test_precompile_compute_guard.py and tests/test_recorder_sync.py passed in this run.
  • Focused: test_wb_shadow_remake passes.

In game: NVIDIA GeForce RTX 5070 12 GB, driver 32.0.16.2114 (Vulkan 1.4), Windows 11 build 26200, i5-14600KF. Hookshot v2.1.0, Vulkan back end, windowed 2560x1440, default quality preset, frame generation on, frame pacing on, 240 cap, Kokiri Forest clearing save. Pressure harness: RX_VK_TEST_ALLOWED_MB=4700 + RX_VK_VRAM_MB=4096. The allowance is synthetic: the driver really allows 11175 MB, so this exercises the rule but cannot reproduce real paging.

build run avg FPS 1% low p99 ms VRAM max
control v2.1.0 ctrl-p4700-a 94.75 77.16 12.96 5368 MB
control v2.1.0 ctrl-p4700-b 93.54 76.45 13.08 4908 MB
this PR p2238-p4700-a 96.10 77.76 12.86 4890 MB
this PR p2238-p4700-b 93.66 77.46 12.91 5368 MB
control mean 94.15 76.81 13.02
PR mean 94.88 77.61 12.89

Path evidence: the shadows given back fired in every run (1 + 1 per build) at 5322 MB used of the synthetic allowance, and over the driver's video-memory allowance fired once per run. shadows made again (the branch this PR adds) was 0 in all four runs: after the give-back the count stays above 97% of the synthetic allowance on this route, and the pools involved (112 MB + 16 MB) had no room under shadow_fits. The issue's repro needs a scene transition (Dodongo's Cavern); this route has none. fps is neutral within run-to-run spread (93.5-96.1 for both builds, same-binary repeats up to 4.5% apart). Not a real VRAM test: the driver never paged.

Relation to #2368

#2368 keeps drawn shadow maps through evictions (the cached 12288x12288 map sampled again). This PR remakes write-back shadows after a give-back under pressure. They affect different textures and files and can land in either order.

AI assistance

Assisted-by: oc-impl-4 (opencode/deepseek-v4.1-flash), oc-upstream-1 (opencode/deepseek-v4.1-flash-free)

Tracking: #993 (where #2238 was consolidated).

Assisted-by: oc-upstream-1 (opencode/deepseek-v4.1-flash-free)
Trace-Issue: NotRexed#2238

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged this into current main and ran tests/test_wb_shadow_remake.py and the memory, shadow, write-back, mirror and vram checks from run_all (18 pass). It also merges cleanly with #2759 and #2766, which touch the same files. CI hasn't run on this head yet.

The change is sound. shadow_wanted is set only where a shadow was first made, and the remake in shadows_check_room goes through shadow_room, which reads VRAM again before each allocation. Shadows are only dropped above 95/99% (shadows_go) and only remade below 90/97% (shadow_fits), and that gap keeps it from making and dropping them every 120 submissions. Capping at 8 attempts per check and skipping pools without a mirror both make sense.

As the description says, the new branch fired 0 times in all four runs, so this is untested in game. The fps table only shows the change does no harm. A run that reaches the remake would show it works: a scene change under the synthetic allowance, with the allowance raised after the give-back, until write-back diff: N shadows made again appears in the log. Dodongo's Cavern from #2238 would be ideal.

@hckrman101 hckrman101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, and for being upfront that the new branch never fired in your runs.

The code is safe. I checked what happens when a shadow is made again partway through a frame:

  • A remade shadow starts with shadow_known all zero. shadows_check_room runs at the top of the write-back block, after shadows_forget_written. So the first write-back that uses the shadow copies each page it touches from guest memory into the shadow, in the command stream, before the diff pass reads it. That's the same state as a brand-new shadow from the 256 KB path.
  • Writes the GPU hasn't written back yet are in the mirror, and a remake doesn't touch the mirror. The diff compares the mirror against a shadow just filled from guest memory, so every word the GPU changed still gets written.
  • The old shadow goes through dead_shadows, so it isn't destroyed until its submissions are done. Pools whose mirror was freed or that were retired are skipped. tries++ == 8 allows exactly 8 attempts.

Joey-Magnus already covered the hysteresis between shadows_go and shadow_fits, and I agree with him.

What's missing is evidence that this fixes #2238. Your table shows the change does no harm, but the remake count was 0 in all four runs, so the fix hasn't been seen working. I'm also not convinced this is what caused the drop. The pools that stay without a shadow are, by construction, the ones whose write-backs stay under 256 KB per submission. Their whole copies are small. The 48 MB in the issue's log is the size of the shadows they'd get, not how much they copy each frame. For a "drops heavily and stays low" report on an 8 GB laptop card, I'd expect the cause to be the VRAM pressure itself rather than those copies.

It also changes a trade-off. These pools now get a shadow the size of the whole pool, under pressure, for write-backs that the original 256 KB rule decided weren't worth one. shadow_fits limits the risk, but the gain needs showing.

Could you get one run where the branch actually fires? Same synthetic allowance, raise it after the give-back (or take a route with a scene change, ideally Death Mountain into Dodongo's Cavern from the issue), until shadows made again shows up in the log. Then post fps and p99 for that scene before and after the remake. Logs with RX_VK_LOG_WRITEBACK=1 would also show how many bytes those pools really copy each frame. If the gain is real it'll show there. If it isn't, this probably belongs in #993 as a finding rather than as the fix for #2238.

The test reads the real functions from the source, declares RUN_ALL_ARGS, and covers the cases that matter (mirror-less pools and pools that are too big don't use up a try, retired pools aren't remade).

Merge order: this merges cleanly with main, including #2731 (now merged), and Joey checked #2759 and #2766. #2794 (sparse mirrors) adds its own fields to Pool right after shadow_known, the same spot as shadow_wanted, so whichever lands second has a one-line conflict to resolve. Nothing interacts beyond that: a shadow is still sized to the whole pool under sparse mirrors.

CI on this head is waiting for workflow approval. So far only the title check has run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FPS drops heavily and stays low after entering a new scene

3 participants