perf(vk): re-upload only the part of a texture a GPU write covered #2876

Open

github-sync wants to merge 2 commits from github/pr/2876 into main

github-sync commented

2026-11-12 19:00:53 +00:00

Owner

@bluetoe-99 opened this pull request on GitHub · 2026-11-12 18:10 UTC · original

Why. A storage buffer bound for writing is recorded as its whole bound range, and any texture whose storage overlaps that range is then re-uploaded whole. In Castle Town one of the game's compute pipelines binds a 1.75 MB write buffer whose range lands inside a finalized 2560x1440 render target's storage, and every frame of that burst the renderer copied 15 MB of guest memory over the image, on a path where nothing at all had been written by the CPU (upload over a drawn image ... 0 KB dirty in 0 ranges, a GPU buffer wrote it). This is one of the costs measured under #2780, where the resident/mirror bookkeeping grows with each area change.

What changes.

gpu_wrote_into reports the ranges it matched, clipped to the texture's storage and merged, in image-relative offsets.
texture_touch uploads those ranges instead of the whole image, and the CPU's writes since the last upload in the same pass. That second half is required: the whole upload used to cover those pages, and the upload moves upload_gen past them, so a partial that skipped them would lose them permanently. An event where the CPU also wrote still uploads the full amount.
Sparse images keep the whole path; RX_VK_NO_PARTIAL and RX_VK_GPU_WRITE_WHOLE=1 restore the old behaviour for A/B.
A tally on the path, so anyone can see whether it is hot in their own scene rather than guessing: vk: gpu-write uploads: N (X MB copied, the whole image would have been Y MB).

Measured (Linux, RTX 5070 Ti 16 GB, one save, one process, five area changes: Hyrule Field, Castle Town Market, Kokiri Forest, Hyrule Field, Castle Town Market). Both arms were built from the same tree with the same flags, differing only by this patch, and the tour was run twice per build:

	stock	patched
per bind-only write-back event	15,360 KB (whole image)	1,800 KB (the bound range)
event where the CPU had also written	15,360 KB	15,360 KB (the union, by design)
the path over a tour, sample 1	150 events, 1,462 MB	150 events, 648 MB
the path over a tour, sample 2	150 events, 1,434 MB	150 events, 632 MB
retirement past the budget, last leg	2,910 textures (841 retire lines)	3,264 textures (840 lines): no eviction regression
fps leg medians	91.8/94.0/96.0/81.9/90.3, then 91.4/88.9/90.1/93.1/88.2	88.0/92.5/77.1/85.5/87.4, then 91.3/90.5/91.5/92.4/92.0

So this is a copy and staging reduction on a path that arrives in bursts, not a frame-rate win on a 16 GB card: 56% less copied on the path, 802-814 MB not copied per tour. No fps effect is claimed, and none was measurable: the same build in the same leg varied by up to 19 fps between samples, which is the noise floor these runs established. On the 8-12 GB cards of #2690 and #2780 the same events compete with a texture budget that is already being evicted past, which is where the saved copying should matter more.

On the measured base. The runs above are from de866017. The branch is rebased onto fe5e2cb8; upstream's twelve hunks in vk_texture.cpp do not touch texture_gpu_wrote, gpu_wrote_into or the gpu_dirty decision, and the rebase applied cleanly apart from an adjacent slow-upload logging line. I did not re-measure on the newer base because MKWVK_API_VERSION moved from 19 to 21 in between, so a main renderer will not load under a de866017 game folder's host binary and restating the numbers needs a full game build. Say so if you want that before merging.

Cache cost for players: none. nx2/build.py's HOOKSHOT_SHADER_STAMP hashes the translator sources (eden/, sirit/, compat/, vk_shader.cpp, vk_bank.cpp, vk_state.h, vk_api.h) and deliberately not vk_texture.cpp, so existing translated programs, pipeline notes and hookshot_prepared.bin stay valid after this ships.

Tests. tests/test_gpu_write_part.py (new) cuts the recorder and the reporter, texture_gpu_wrote and gpu_wrote_into, out of the source and runs them against stand-ins: a write inside reports its part; one spanning either end is clipped; adjacent writes merge; an unfinished submission is not reported until it completes; the same write is not reported to the same texture twice; a write elsewhere reports nothing. Plus source guards for the switch, the CPU-write union and the sparse exclusion. python -I -B tests/run_all.py: 411 passed, 13 failed, 67 skipped on the measured base (those 13 fail identically on the main branch too), and 453 passed, 1 failed, 69 skipped rebased, where the single failure is tests/test_readme_menu_rows.py, which also fails on clean fe5e2cb8.

Also in this branch. One readme line, in a separate commit (fix(docs)). tests/test_readme_menu_rows.py wants a row for every Insert menu entry, and main has been red on it since #2827 added the Gameplay tab's "Skip opening" without one; that is what the Python source checks (ubuntu, part 1/2) failure was showing on the first push. It has nothing to do with the renderer change and I will split it out if a reviewer would rather it land on its own.

Not claimed. No visual change is intended. The change copies a strict subset of what the old path copied, so the known hazards of partial upload (the rainbow-sky case, where an unrelated variable sharing an edge page re-uploaded garbage over a drawn LUT) get narrower, not wider. No screenshots attached because nothing on screen should differ; both arms' frames from the tour are available if a reviewer wants a particular scene checked.

> [@bluetoe-99](https://github.com/bluetoe-99) opened this pull request on GitHub · 2026-11-12 18:10 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2876) **Why.** A storage buffer bound for writing is recorded as its whole bound range, and any texture whose storage overlaps that range is then re-uploaded whole. In Castle Town one of the game's compute pipelines binds a 1.75 MB write buffer whose range lands inside a finalized 2560x1440 render target's storage, and every frame of that burst the renderer copied 15 MB of guest memory over the image, on a path where nothing at all had been written by the CPU (`upload over a drawn image ... 0 KB dirty in 0 ranges, a GPU buffer wrote it`). This is one of the costs measured under #2780, where the resident/mirror bookkeeping grows with each area change. **What changes.** - `gpu_wrote_into` reports the ranges it matched, clipped to the texture's storage and merged, in image-relative offsets. - `texture_touch` uploads those ranges instead of the whole image, **and** the CPU's writes since the last upload in the same pass. That second half is required: the whole upload used to cover those pages, and the upload moves `upload_gen` past them, so a partial that skipped them would lose them permanently. An event where the CPU also wrote still uploads the full amount. - Sparse images keep the whole path; `RX_VK_NO_PARTIAL` and `RX_VK_GPU_WRITE_WHOLE=1` restore the old behaviour for A/B. - A tally on the path, so anyone can see whether it is hot in their own scene rather than guessing: `vk: gpu-write uploads: N (X MB copied, the whole image would have been Y MB)`. **Measured** (Linux, RTX 5070 Ti 16 GB, one save, one process, five area changes: Hyrule Field, Castle Town Market, Kokiri Forest, Hyrule Field, Castle Town Market). Both arms were built from the same tree with the same flags, differing only by this patch, and the tour was run twice per build: | | stock | patched | |---|---|---| | per bind-only write-back event | 15,360 KB (whole image) | **1,800 KB** (the bound range) | | event where the CPU had also written | 15,360 KB | 15,360 KB (the union, by design) | | the path over a tour, sample 1 | 150 events, 1,462 MB | 150 events, **648 MB** | | the path over a tour, sample 2 | 150 events, 1,434 MB | 150 events, **632 MB** | | retirement past the budget, last leg | 2,910 textures (841 retire lines) | 3,264 textures (840 lines): no eviction regression | | fps leg medians | 91.8/94.0/96.0/81.9/90.3, then 91.4/88.9/90.1/93.1/88.2 | 88.0/92.5/77.1/85.5/87.4, then 91.3/90.5/91.5/92.4/92.0 | So this is a copy and staging reduction on a path that arrives in bursts, not a frame-rate win on a 16 GB card: 56% less copied on the path, 802-814 MB not copied per tour. **No fps effect is claimed**, and none was measurable: the same build in the same leg varied by up to 19 fps between samples, which is the noise floor these runs established. On the 8-12 GB cards of #2690 and #2780 the same events compete with a texture budget that is already being evicted past, which is where the saved copying should matter more. **On the measured base.** The runs above are from `de866017`. The branch is rebased onto `fe5e2cb8`; upstream's twelve hunks in `vk_texture.cpp` do not touch `texture_gpu_wrote`, `gpu_wrote_into` or the `gpu_dirty` decision, and the rebase applied cleanly apart from an adjacent slow-upload logging line. I did not re-measure on the newer base because `MKWVK_API_VERSION` moved from 19 to 21 in between, so a `main` renderer will not load under a `de866017` game folder's host binary and restating the numbers needs a full game build. Say so if you want that before merging. **Cache cost for players: none.** `nx2/build.py`'s `HOOKSHOT_SHADER_STAMP` hashes the translator sources (`eden/`, `sirit/`, `compat/`, `vk_shader.cpp`, `vk_bank.cpp`, `vk_state.h`, `vk_api.h`) and deliberately not `vk_texture.cpp`, so existing translated programs, pipeline notes and `hookshot_prepared.bin` stay valid after this ships. **Tests.** `tests/test_gpu_write_part.py` (new) cuts the recorder and the reporter, `texture_gpu_wrote` and `gpu_wrote_into`, out of the source and runs them against stand-ins: a write inside reports its part; one spanning either end is clipped; adjacent writes merge; an unfinished submission is not reported until it completes; the same write is not reported to the same texture twice; a write elsewhere reports nothing. Plus source guards for the switch, the CPU-write union and the sparse exclusion. `python -I -B tests/run_all.py`: 411 passed, 13 failed, 67 skipped on the measured base (those 13 fail identically on the main branch too), and 453 passed, 1 failed, 69 skipped rebased, where the single failure is `tests/test_readme_menu_rows.py`, which also fails on clean `fe5e2cb8`. **Also in this branch.** One readme line, in a separate commit (`fix(docs)`). `tests/test_readme_menu_rows.py` wants a row for every Insert menu entry, and `main` has been red on it since #2827 added the Gameplay tab's "Skip opening" without one; that is what the `Python source checks (ubuntu, part 1/2)` failure was showing on the first push. It has nothing to do with the renderer change and I will split it out if a reviewer would rather it land on its own. **Not claimed.** No visual change is intended. The change copies a strict subset of what the old path copied, so the known hazards of partial upload (the rainbow-sky case, where an unrelated variable sharing an edge page re-uploaded garbage over a drawn LUT) get narrower, not wider. No screenshots attached because nothing on screen should differ; both arms' frames from the tour are available if a reviewer wants a particular scene checked.

github-sync added 2 commits

2026-11-12 19:00:53 +00:00

perf(vk): re-upload only the part of a texture a GPU write covered 8d108b3af0

A buffer bound for writing marks its whole bound range written, though the shader may touch less
of it, and the game's scratch lands on the storage of a target the GPU drew and finalized: in
Castle Town a 1.75 MB bind made the renderer re-upload a 15 MB 2560x1440 target every frame of
the burst (#2780). gpu_wrote_into now reports the ranges it matched, clipped to the image and
merged, and texture_touch uploads those plus the CPU's writes since: the whole upload used to
cover them, and the upload moves upload_gen past them, so carrying them along is required. Sparse
images and RX_VK_NO_PARTIAL keep the old path; RX_VK_GPU_WRITE_WHOLE=1 asks for all of it again.
The new "gpu-write uploads" line counts the path: over a five-area tour, run twice, 150 events
copied 648 MB where the whole-image rule would have copied 1,462 MB (second sample 632 against
1,434). Per event 15,360 KB to 1,800 KB for a bind-only write-back, and an event where the CPU had
also written still uploads its full 15,360 KB. No fps effect across two samples per side, and the
tour showed up to 19 fps of run-to-run noise for the same build, so none is claimed.
tests/test_gpu_write_part.py runs the marking and the range report against stand-ins.

fix(docs): the readme's menu table gains the "Skip opening" row #2827 left out 0c457b36ac

tests/test_readme_menu_rows.py wants a line for every Insert menu row, in the menu's order, and
the Gameplay tab's "Skip opening" (added by #2827, 435d83d3) never got one: the check has been
red on main since that merge. The row takes the menu's own wording and its default, On.

github-sync commented

2026-11-12 19:03:14 +00:00

Author

Owner

@bluetoe-99 commented on GitHub · 2026-11-12 19:02 UTC · original

Status after the two pushes:

The first push failed Python source checks on Windows in both shards: my new test compiles a fixture cut from vk_texture.cpp, which calls getenv, and the MSVC CRT deprecation plus -Werror turned it into an error. Fixed with the flag the sibling tests already pass (-D_CRT_SECURE_NO_WARNINGS), verified with clang++ and g++.
The ubuntu part 1/2 failure was pre-existing on main: tests/test_readme_menu_rows.py has been red since #2827 added the Gameplay tab's "Skip opening" row without its readme line. The second commit (fix(docs)) adds that one row. It fails identically with this branch's renderer change stashed out.
Locally, with the Vulkan/SPIR-V headers available: python -I -B tests/run_all.py gives 488 passed, 0 failed, 35 skipped (the skips need spirv-val and a few Windows-only tools).
The checks for the current head are still queued behind what looks like a repo-wide Actions backlog right now, so I have not seen them run yet.
The measured numbers in the description are from de866017; the branch is rebased onto fe5e2cb8. Restating them on the newer base needs a full game build because MKWVK_API_VERSION moved 19 to 21 in between; happy to do it if a reviewer wants it there.

> [@bluetoe-99](https://github.com/bluetoe-99) commented on GitHub · 2026-11-12 19:02 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2876#issuecomment-6833120570) Status after the two pushes: - The first push failed `Python source checks` on Windows in both shards: my new test compiles a fixture cut from `vk_texture.cpp`, which calls `getenv`, and the MSVC CRT deprecation plus `-Werror` turned it into an error. Fixed with the flag the sibling tests already pass (`-D_CRT_SECURE_NO_WARNINGS`), verified with `clang++` and `g++`. - The ubuntu `part 1/2` failure was pre-existing on `main`: `tests/test_readme_menu_rows.py` has been red since #2827 added the Gameplay tab's "Skip opening" row without its readme line. The second commit (`fix(docs)`) adds that one row. It fails identically with this branch's renderer change stashed out. - Locally, with the Vulkan/SPIR-V headers available: `python -I -B tests/run_all.py` gives 488 passed, 0 failed, 35 skipped (the skips need `spirv-val` and a few Windows-only tools). - The checks for the current head are still queued behind what looks like a repo-wide Actions backlog right now, so I have not seen them run yet. - The measured numbers in the description are from `de866017`; the branch is rebased onto `fe5e2cb8`. Restating them on the newer base needs a full game build because `MKWVK_API_VERSION` moved 19 to 21 in between; happy to do it if a reviewer wants it there.

Commenting is not possible because the repository is archived.

No reviewers