WIP: chore: restore 81 Hookshot PRs merged on GitHub after Longshot branched #209
No reviewers
Labels
No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
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/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Longshot!209
Loading…
Reference in a new issue
No description provided.
Delete branch "TrashPanda/Longshot:restore/github-after-split"
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?
Why
Longshot branched from Hookshot's
mainat4dd972a8(#2888). Ninety more pull requests were merged on GitHub after that, beforeNotRexed/Hookshotwent down, and none of them are in Longshot. Their fixes, tested-configuration rows and tools exist only in mirrors. This PR brings back the 24 that still apply, each as its own commit under its original author.What changed
Each commit is the merged Hookshot PR, carried through Longshot's rename (
hookshottolongshot,.github/workflowsto.forgejo/workflows). Where Longshot had changed the same lines since, the conflict was resolved on Longshot's side, and the commit message says how ("Ported to Longshot: ...").The first 24 (as when this PR opened):
place_toolnow does the whole-folder move throughmove_retry. The Python setup half is left out because Longshot removed the Python Builder.work/ffx-sdkfolder name had a form feed in place of/f(#2553, readme part only)run_all.py --changedruns the checks that read what changed (#2887)tests/srccut.py, with 80 checks moved to it (#2914).forgejo/workflows/ci.yml, started by hand like the other full checks.pr_inforeads git.magicbean.dev and keeps Longshot's writer check)scripts/dev_packs.py(#2732)longshot_bench.py, a repeatable benchmark route through the control bridge (#2714)57 more, merged on GitHub in its last hours (they were in a mirror's later fetch):
One commit of mine at the end: stand-ins in four checks that cut code these commits changed (
ViewFastfrom #2960, Longshot'scompile_all()in #2982's check). Upstream's tip had theViewFastthree failing too.Left out, with the reason:
scripts/linux/appimage.shhost/nn_vi.cand its check. Left for its author or a follow-upFlags and settings
All new switches default to the upstream behavior as merged; each
=0gives the old path back for comparisons.RX_VK_ASTC_BC1=0RX_VK_GAME_ORDERRX_VK_IMAGE_BLOCK_MB=<n>RX_VK_VIEW_FAST=0RX_VK_STAGING_PREWARM=<n>RX_VK_SPLIT_UBFLUSH=1RX_VK_TEMPORAL_CUT=0RX_VK_FSR3_HDR=0RX_VK_NATIVE_NARROW=0RX_VK_IMPORT_FENCE=0RX_VK_IMAGE_PREALLOC=<blocks>RX_VK_TEST_ALLOWED_MB=<MiB>HOS_CV_SIGNAL_ALWAYS=1RX_VK_UNSWIZZLE_IMPORT=1RX_VK_LOWER_SKIP=<key prefix,...>RX_VK_IRRX_VK_PREDICT_S=<s>dev_packs.py --predict-s: stop building predicted pipelines after N seconds (#2732)LONGSHOT_NO_PREPARE=1,RX_VK_ONLY,RX_RELEASE,RX_TARGET,LONGSHOT_KEEP_BUILD_FILES=1scripts/dev_packs.py(#2732) sets for its buildsHOS_INPUT_BACKGROUND=1HOS_FS_OVERLAY=<folder>Measurements
No new numbers: these are Hookshot PRs that were reviewed and merged with their own measurements, which are in each commit's message and in the tested-configurations rows. The two pipeline lists change what is prepared ahead and were measured upstream (#3029, #3030).
Testing
Tested:
python -I -B tests/run_all.pyon Linux (CachyOS, Python 3.14, Vulkan headers found): 522 passed, 3 failed, 30 skipped. Longshotmain@0c74253don the same PC: 489 passed with the same 3 failing (test_display_pointer,test_lift_cache, andtest_texture_staging_publication, which #215 fixes), so this branch adds no failures. I re-ran the affected checks after each resolved conflict. Rebased on2331f46c(#210, README only) after that run.Not tested: a game build or run of this branch. The game-facing commits were tested in the game on Hookshot before they merged. The Windows setup retry (#3021) has not run on Windows. The lavapipe job (#2866) has not run on Longshot's Forgejo runners.
Needs other testers:
Screenshots
Nothing on screen
Risk and rollback
Each commit is separate and reverts on its own. The riskiest are the memory and texture changes (#2554, #2557, #2960, #2967, #3020), each with an
=0switch or a row to turn it off, and #2830's test-request pinning, which changesscripts/test_requests.py's commands. Old cards still replay, and reverting that commit restores Longshot's current client.Reviewer notes
Please merge this one with rebase, not squash, so that each of the 29 authors keeps their commit. If you'd rather review it in parts, I can split it into the first 24 and the later 57, or by area (input and menu, rendering, memory and speed, pipelines, platform).
The Hookshot PR numbers in the commit messages point at
github.com/NotRexed/Hookshot, which is gone; mirrors of it still have the history.Issues
See #
Checklist
For everyone
type(scope): description).main.python -I -B tests/run_all.pyrun and passing (any failure that also fails onmainis named above).third_party/untouched (a new folder with aPROVENANCE.mdinstead).If applicable
Assisted-by:/Co-Authored-By:trailers.docs/tested-configurations.mdfor each measured run.tests/that reads changed source updated in this PR, not weakened.de866017(#2715) c5fd81ffee49f75dc8and the last main before #2602 (#2840) fbae68f3b7Testing a change in the game means a new full build: a new work folder translates the game again, and a new game folder prepares every shader from cold, 69 minutes in all on an RX 9070 XT. Most of that is the same for every change. This adds a development tool that builds the game once as a base and each change as a small pack over it. ## What changed - **`scripts/dev_packs.py`** (new, development only): - `base` builds a checkout at a commit. `--seed` copies an older game folder's shader caches first, as an update keeps them. - `pack` builds a change. Commits, uncommitted edits and untracked files all count. - **Renderer alone** (`RX_VK_ONLY`, seconds): when every changed file is a renderer file the game isn't built from. Those files are found in the checkout from `host/`'s includes and `build.py`'s dependencies. - **Otherwise `make_game.py`**, which builds both modules. - **What a pack keeps:** - **A module** when an object in its link list (`objs.rsp`, `vk_objs.rsp`) differs from the base's. Bytes can't decide this: `nx2/version.py` puts a fingerprint of every source into both modules. - **Both modules** when a build script, a non-translator data file under `nx2/`, or a non-C file changed. - **Generated files that differ**, such as the shader seed and the prompts. Files the base's own build made that the change no longer makes are removed. - `rt` makes a runtime: the base with `romfs` hard-linked and the other files copied, the packs' files over them. It runs a prepare when the shader translator or the seed changed. Two packs that bring the same file are refused, and so is a pack built on another base. - **`docs/development.md`:** a new section, *Testing a change against a base*. - **`AGENTS.md`:** one bullet pointing agents to it. - **Nothing players build changes.** It calls `make_game.py` and `build.py` as they are; no build or CI file is touched. ## Testing - **`tests/test_dev_packs.py`** (no game): - the build route, and the renderer files the game reads; - that the translator list matches `build.py`'s stamp, and that the version-stamped objects are `build.py`'s; - that link-only inputs keep both modules; - the lift key's dependence on the folders; - generated files that change or are removed, and a clean output folder; - a runtime from packs, and every refusal by its own reason. - **Negative controls:** every rule above has a sabotaged variant of the script that fails the test. - **Ten review rounds** with gpt-6.1-sol, every finding fixed; the last found none. - **End to end on Windows** (Ryzen 7 5700X3D, RX 9070 XT), packs over a full build of57ed3aa8: | Pack | Changed | Kept | Time | |---|---|---|---| | host only | `host/nvn_native.c` | `hookshot.exe` | 14 min first (one translation), 48 s later | | host + renderer | `host/nvdrv.c`, `vk_memory.cpp` | both | 34 s | | renderer only | `vk_replay.cpp` | `hookshot_vk.dll` | 16 s first, 2 s later | | runtime of two packs | | exe from one, DLL from the other | 3 min 51 s | Each pack's modules are byte-identical to that change's own earlier full build, apart from 2 PE header bytes. The renderer-only route gives the same DLL as `make_game.py` for the same tree. - **Seeded base** on main46780503from 57ed3aa8's caches: - 52.6 min, against 69 min for57ed3aa8cold. - **Shader programs:** 290 s against 822 s. main changed the translator, so the program cache was rebuilt, but the driver's pipeline cache carried over. - **Precompile:** 258 s against 602 s. - **Predicted pipelines:** 1,790 s against 2,150 s. - **In the game:** a runtime made by `rt` from two packs over57ed3aa8(the host-only pack's exe, with its new switches off, and the renderer-only pack's DLL): - **The run:** the standard Hyrule scene (32:9, unlocked frame rate, camera turning), 3 runs, RX 9070 XT, Windows 11. - **Result:** the game started and played the scene every time, at 40.4–41.9 fps. 57ed3aa8's own runtime measured 39.8 fps on the same test that morning. - **Not a measurement:** n=1 after two warm-ups, and the game clock dipped to 0.94–0.98 in these runs, as it did in the base's own runs that day. - **No tested-configurations row:** this was a development runtime with unmerged changes, not a player build. - **Not tested:** Linux. The module names there are `hookshot` and `hookshot_vk.so`, and `RX_TARGET=windows` uses the Windows names; neither `pack` nor `rt` has run there. Related: the dev-loop proposal (#2636). This covers part of its "middle loop" without changing the build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: NotRexed <[email protected]>123 checks carry their own copy of a brace counter (`def block`) to cut a function out of a source file. The copies count braces inside strings, character literals and comments too, and when a function is renamed or reshaped they stop with a bare `ValueError` or `IndexError` that doesn't say which source or marker: the trap AGENTS.md warns about ("Tests read the source"). This gives the checks one cutter and moves 80 of them to it, each shown to cut exactly what it cut before. The dev-loop proposal's D2 (#2636). ## Change - **`tests/srccut.py` (new, standard library):** - `block(text, marker)`: from the marker to the brace that closes the first `{` after it, counting braces outside string and character literals (raw strings too, and C++14 digit separators), comments and preprocessor lines; - `between(text, start, end)`; - a marker that is missing, or that appears twice, stops the check with one message naming the source and the marker (`unique=False` takes the first on purpose); - `SRCCUT_CHECK_NAIVE=1` also cuts each block the old way and stops if the two differ: what a migration runs to show it changed nothing. - **80 checks import it** in place of their copy, one commit per source file they read (`vk_menu.cpp`, `vk_draw.cpp`, `vk_memory.cpp`, `vk_texture.cpp`, `hookshot_env.h`, `hos_main.c`, ...). Only exact copies of the plain counter moved. - **Not moved:** - three checks whose `block` isn't the plain counter (a start offset; positions returned): restored as they were; - 13 checks whose marker appears more than once in the source (a declaration before the definition, for example). The old counter took the first one silently, so each needs a look rather than `unique=False`; - 51 checks with other cutters (`function`, `body`, `between`, ...): later, by source file. ## Testing - **Every moved check, against today's `main`:** each run as `main` has it and as moved with `SRCCUT_CHECK_NAIVE=1`, with its declared tools (the Builder's clang, the Vulkan headers): **80 of 80 pass both ways**, no cut differs. - **`tests/test_srccut.py` (new):** braces in strings, character literals, raw strings, comments and preprocessor lines; a fixture the old counter cut wrongly; a missing marker and a repeated one name the source; `between`; an unclosed block; the comparison with the old counter. - A follow-up after #2887: `run_all.py --changed` runs a test helper's importers (srccut's) rather than every check. See #2636 (D2). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]> Co-authored-by: NotRexed <[email protected]> Ported to Longshot: test_builder_update_back.py and test_linux_update_skip.py are left out (Longshot removed them with the Builder window, #181). Co-authored-by: Cursor <[email protected]>In Hookshot Builder, a game folder that was built from a supported `.nup` can be rebuilt from an extracted dump. When that dump is of another game version, the build is refused ("This is build X. Hookshot supports build Y"), which is right, but `nx2/make_game.py` checked the version only after the dump's modules had replaced the folder's saved copy (`.hookshot_exefs`). The stamp of the earlier `.nup` stayed. Choosing that `.nup` again then said "reading it again is skipped", built from the refused modules and was refused too, until the folder was deleted. ## Change In the extracted-dump branch of `main()`: - `check_supported_build` runs right after the dump's modules are unpacked, before the romfs is linked in and before `.hookshot_exefs` is touched. A refused dump leaves the folder as it was. - An accepted dump removes the `.nup` stamp (`.hookshot_romfs`), since the saved copies are no longer that `.nup`'s. The next build from the `.nup` reads it again. The `.nup` branch already checks the build before it replaces anything; it is unchanged. ## Testing - `tests/test_make_game_cache.py`, two new cases (the cache test's fake `.nup` and modules, no game): - A refused extracted dump: the folder's files are byte for byte as before, and the earlier `.nup` then reuses its copy and reaches the lift with the supported build. - An accepted extracted dump: its modules are saved, the `.nup` stamp is gone, and the earlier `.nup` is read again. - Both fail on main and pass here. - Full suite on Linux: 473 passed, 53 skipped, 2 failed, both the same on main on this machine (`test_bake_portability`: `zstandard` is not installed; `nx2/test_texture_init_barrier.py`: `clang++` is not installed). - No real game build was run.Two costs found by profiling a 4-core handheld (Steam Deck, RADV Van Gogh, 15 W), both generic — no Deck-specific defaults, both behind an env switch that keeps the old behaviour available. ## 1. Keep the staging arenas, and make them before the game's memory Making a staging arena while ~2.5 GB of pools and textures are held took **12–25 ms** — and an 8 MB one could take **116 ms**, so it is the allocator, not the size. The arena the last burst used goes stale after `RX_VK_STAGING_IDLE_S` (30 s) and is given back, so the first upload of the next burst makes another. A camera turn brings new textures in, so it paid for one every time: **28 slow creations in one session**, each a stall of the replay thread. Four are now made at replay start, on an empty device, where they are cheap (`RX_VK_STAGING_PREWARM`, default 4; 0 restores the old behaviour). Measured over a 240 s run, same save and settings: **28 slow creations before, 1 after.** This is the same failure the comment above the idle free already describes on other hardware ("a scene change made several right after the idle ones were freed"), so it should help anywhere memory pressure makes a 64 MB host-visible buffer expensive. ## 2. Answer repeated texture views from a direct-mapped front cache `texture_view_of` was **0.99 ms a frame** for ~10,600 lookups, each a hash-map find, and the same slots come round again and again (14.9 descriptors a draw, 725 draws). A 1024-entry direct-mapped cache now answers the repeats. It holds the key, the slot's version, the destroy generation and the texture epoch, and does the epoch's touch itself — so a slot re-registered or a texture destroyed misses it at once and takes the map's path. **The map stays the authority**; the cache only answers a lookup whose every input is the one it was filled with. `RX_VK_VIEW_FAST=0` restores the old path. Measured back to back, same session, same scene, 75 % internal resolution: | | fps | p50 | p99 | 1 % low | |---|---|---|---|---| | without | 24.8 | 40.32 ms | 42.37 ms | 23.6 | | **with** | **24.9** | 40.16 ms | **41.32 ms** | **24.2** | The average is within noise; the tail is a real millisecond. ## Also - `RX_VK_PROF_FINE` now splits the texture path (storage-pointer test, handle, view, fingerprint), and the frame statistics say which binding moved `bind_gen` — counted: ubo 2159, tex 1794, ssbo 601, program 431, img 84 a frame. That last one is worth knowing: the game genuinely rebinds ~7 things per draw, which is why 0.9 of draws rebuild their descriptor set and why the reuse check cannot be relaxed. - `scripts/chain.py` no longer fails on a thread line without a name. ## Testing Based on current `main` (e0e60920); both commits cherry-picked cleanly. I have not been able to build this branch locally (my build machine is on the 127 commits behind base), so CI is the first build — say the word if you would rather I build it first and reopen. Co-authored-by: RufflezAU <[email protected]>### The bug `upload_inner` (gpu/vk/vk_texture.cpp) uploads a texture in runs of rows. For a sparse texture, each run first queues the binds of the tiles it touches with `texture_resident_queue`: the tiles are marked bound at once, and the binds go into `sparse_binds`, one list shared by every texture. `texture_resident_flush` later binds the whole list with `vkQueueBindSparse`, into one image only: `VkSparseImageMemoryBindInfo` takes a single `image` for the list. Each run takes its staging memory with `staging()`. When that failed part-way, the code was: ```cpp if (!st.buffer) return false; ``` so it returned with the binds of the earlier rows still in `sparse_binds`. Step by step: 1. Texture A (sparse) uploads. Rows 0..N queue the binds of their tiles; those tiles are marked bound. 2. The staging for row N+1 fails, and `upload_inner` returns false. A's binds are still in `sparse_binds` and were never made. 3. Texture B (sparse) uploads. Its `texture_resident_flush` binds the whole list into **B's** image, including A's binds. The memory meant for A's tiles goes to the same coordinates of B's image. 4. A's tiles stay marked bound, so no later write queues them again. A's later copies into those tiles land in unbacked memory, and B gets memory it did not ask for. The other exits of that loop (a new staging arena, the end of the upload) already call `texture_resident_flush(&tex)` first. Only the staging-failure exit did not. ### What changed Only that exit, in `upload_inner`: ```cpp if (!st.buffer) { texture_resident_flush(&tex); return false; } ``` The binds the earlier rows queued are now made into A's own image before giving up, as the other exits do. The success path is unchanged. A texture is only sparse when it is a 2D array over `RX_VK_TEX_MAX_MB` (1.5 GB) on a device with sparse support, which is MKW's course-map case. ### Test `tests/test_sparse_staging_failure.py` is a source check on `upload_inner`. In the part of the loop after the first `staging()` call and before `texture_resident_queue`, every `return false;` must have a `texture_resident_flush(&tex);` in the same block. The old `if (!st.buffer) return false;` must be gone. On main it fails with "a staging failure returns without binding the sparse blocks queued before it"; with the fix it passes. ### How tested - The new test fails on main and passes with the fix. - The tests that reference vk_texture.cpp pass. - Release build OK. - Zelda OoT on Radeon RX 9070 XT (Windows, Adrenalin), nightly with this change, about 40 minutes, log clean. This checks that non-sparse uploads are unaffected; the sparse path itself is not reached there. Assisted-by: GitHub Copilot### The bug Read-backs (texture write-backs to guest memory) use their own command pool `rb_pool`, command buffer `rb_cmd` and fence `rb_fence` (gpu/vk/vk_texture.cpp): - `readback_begin` resets the pool and begins `rb_cmd`. - `readback_submit_wait` resets `rb_fence`, submits `rb_cmd` with it, and waits up to 5 s. When that wait timed out, the function returned false, but nothing remembered that the submission was still running. Step by step: 1. A read-back is submitted. The GPU is slow (heavy load, a long batch before it), and `vkWaitForFences` returns `VK_TIMEOUT` after 5 s. `rb_cmd` is still in the pending state and `rb_fence` is still tied to it. 2. The next read-back calls `readback_begin`: - `vkResetCommandPool(rb_pool)` while `rb_cmd` is pending. This violates **VUID-vkResetCommandPool-commandPool-00040**: "All VkCommandBuffer objects allocated from commandPool must not be in the pending state". - `vkBeginCommandBuffer(rb_cmd)` while it is pending. This violates **VUID-vkBeginCommandBuffer-commandBuffer-00049**: "commandBuffer must not be in the pending state". 3. `readback_submit_wait` then calls `vkResetFences(rb_fence)` while it is still tied to the unfinished submission. This violates **VUID-vkResetFences-pFences-01123**: "Each element of pFences must not be currently associated with any queue command that has not yet completed execution on that queue". This is undefined behaviour: the GPU can be running commands that the CPU is rewriting. There was a second problem in the creation. `readback_begin` makes the pool, then the command buffer, then the fence, all only when `rb_pool` is null. If `vkAllocateCommandBuffers` or `vkCreateFence` failed, `rb_pool` stayed set with `rb_cmd` or `rb_fence` null. Every later call then skipped the creation and used a null command buffer or fence. ### What changed - New `bool rb_pending`. `readback_submit_wait` sets it when its wait does not succeed: ```cpp rb_pending = vkWaitForFences(dev.device, 1, &rb_fence, VK_TRUE, 5000000000ull) != VK_SUCCESS; return !rb_pending; ``` - `readback_begin` first waits for a pending submission, before any reset. If it is still not done, this read-back is skipped (returns false), as a failed read-back already was: ```cpp if (rb_pending) { if (vkWaitForFences(dev.device, 1, &rb_fence, VK_TRUE, 5000000000ull) != VK_SUCCESS) return false; rb_pending = false; } ``` - If the command buffer or the fence cannot be made, the pool is destroyed and all three handles go back to null, so the next call makes them again instead of using them half made. When no wait times out, the behaviour is the same as before. ### Test `tests/test_readback_pending.py` is a source check on `readback_begin` and `readback_submit_wait`. It checks that: - `rb_pending` exists. - In `readback_begin`, `if (rb_pending)` and its `vkWaitForFences` on `rb_fence` come **before** `vkResetCommandPool`. - The failure branch of the creation destroys `rb_pool` and sets it back to null. - `readback_submit_wait` stores the result of its wait in `rb_pending`. On main it fails ("a timed-out read-back is not remembered"); with the fix it passes. ### How tested - The new test fails on main and passes with the fix. - The tests that reference vk_texture.cpp pass. - Release build OK. - Zelda OoT on Radeon RX 9070 XT (Windows, Adrenalin), nightly with this change, about 40 minutes, log clean. Read-backs run normally there, but none timed out, so the timeout path itself was not reached. Assisted-by: GitHub Copilot### The bug `copy_t2t` (gpu/vk/vk_xfer.cpp) makes a texture-to-texture copy in one of three ways: 1. `vkCmdResolveImage`: a multisampled source into a single-sampled destination of the same size. 2. `vkCmdCopyImage`: same size, same block size, same sample count. 3. `vkCmdBlitImage`: everything else, which means scaled or converting copies. Branch 3 called `vkCmdBlitImage` without checking any of the rules the spec puts on it. Step by step, with one example: 1. The game copies a texture into another of a different size or a different format, for example an unsigned-integer texture into a normalized-colour one. 2. Neither the resolve nor the copy branch applies, so the copy goes to `vkCmdBlitImage`. Linear is used when the game asks for it, or when the destination keeps its level at a smaller size than the source (`d.down > s.down`). 4. When one of those rules is broken, the command is invalid usage and the result is undefined. Some drivers tolerate it; others may write garbage or lose the device. The rules, with their exact text from the spec (`validusage.json`): | VUID | Text | |---|---| | VUID-vkCmdBlitImage-srcImage-01999 | The format features of srcImage must contain VK_FORMAT_FEATURE_BLIT_SRC_BIT | | VUID-vkCmdBlitImage-dstImage-02000 | The format features of dstImage must contain VK_FORMAT_FEATURE_BLIT_DST_BIT | | VUID-vkCmdBlitImage-filter-02001 | If filter is VK_FILTER_LINEAR, then the format features of srcImage must contain VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_LINEAR_BIT | | VUID-vkCmdBlitImage-srcImage-00229 | If either of srcImage or dstImage was created with a signed integer VkFormat, the other must also have been created with a signed integer VkFormat | | VUID-vkCmdBlitImage-srcImage-00230 | If either of srcImage or dstImage was created with an unsigned integer color VkFormat, the other must also have been created with an unsigned integer color VkFormat | | VUID-vkCmdBlitImage-srcImage-00231 | If either of srcImage or dstImage was created with a depth/stencil format and the maintenance12 feature is not enabled the other must have exactly the same format | | VUID-vkCmdBlitImage-srcImage-00233 | srcImage must have been created with a samples value of VK_SAMPLE_COUNT_1_BIT | | VUID-vkCmdBlitImage-dstImage-00234 | dstImage must have been created with a samples value of VK_SAMPLE_COUNT_1_BIT | Hookshot does not enable maintenance12, so -00231 applies as written. The images are created with `VK_IMAGE_TILING_OPTIMAL`, so the format features that matter are `optimalTilingFeatures`. ### What changed (gpu/vk/vk_xfer.cpp) Before, branch 3 went straight to the blit: ```cpp } else { // scaled or converting VkImageBlit bl{}; ... const bool linear = ((p.flags & NVN_COPY_FLAGS_LINEAR_FILTER_BIT) || d.down > s.down) && color; vkCmdBlitImage(..., linear ? VK_FILTER_LINEAR : VK_FILTER_NEAREST); } ``` Now it first checks the rules above, and skips a blit that breaks any of them: ```cpp if (!(tiling_features(s.tex->format) & VK_FORMAT_FEATURE_BLIT_SRC_BIT) || // -01999 !(tiling_features(d.tex->format) & VK_FORMAT_FEATURE_BLIT_DST_BIT) || // -02000 format_class(s.tex->format) != format_class(d.tex->format) || // -00229, -00230 (!color && s.tex->format != d.tex->format) || // -00231 s.tex->samples != VK_SAMPLE_COUNT_1_BIT || d.tex->samples != VK_SAMPLE_COUNT_1_BIT) { // -00233, -00234 warn(13, "texture blit the device cannot make: skipped"); full_barrier(cmd); return; } ``` Linear filtering is only used when the source format supports it (-02001), and only for float or normalized formats: ```cpp const bool linear = ((p.flags & NVN_COPY_FLAGS_LINEAR_FILTER_BIT) || d.down > s.down) && color && (tiling_features(s.tex->format) & VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_LINEAR_BIT) && format_class(s.tex->format) == 0; ``` Two new helpers in the same file: - `format_class(VkFormat)` returns: - 1 for the unsigned-integer formats, - 2 for the signed-integer formats, - 0 for everything else (float, normalized, sRGB). - `tiling_features(VkFormat)` returns the format's `optimalTilingFeatures`: - For the core formats (values below 256), it asks the driver once per format and keeps the answer in a lock-free `std::atomic<uint64_t>` table. Bit 63 marks a format as already asked. - Formats from extensions (256 and above) are asked each time. - A scaled copy checks both sides, so the cache avoids querying the driver twice per copy on the replay thread. The depth/stencil condition is already caught earlier in `copy_t2t`, by warn 7 ("texture copy between unlike depth formats: skipped"), so it never fires here. It is kept in the check so the blit's own rules stay in one place. `warn` logs each key at most 3 times. ### Behaviour - A blit that follows the rules goes through exactly as before. - A blit that breaks a rule is skipped, and its destination is left as it was. Before, it was undefined: it may have looked right on some drivers, for example an integer/float mix or a multisampled image on a driver that tolerates it. Now the destination is stale instead. - This matters most on devices with a narrower format list (mobile and integrated GPUs, layered drivers such as MoltenVK), and for games or scenes that blit less common formats. ### Changes after review - The format features come from the `tiling_features` cache, so they are not queried twice per scaled copy. - The first push of that change left one use of the old local variable in the linear-filter check. The vk_xfer tests don't compile that code; the full release build caught it, and it is fixed. ### Test There is no dedicated unit test for these rules. The change is reviewed against the VUIDs above. The existing checks that compile or read vk_xfer.cpp pass (6, including `test_vulkan_copy_span_gpu_compile`). ### How tested - Release build (`nx2/build.py --standalone`, Windows, llvm-mingw). - The 6 vk_xfer.cpp checks pass. - Zelda OoT on Radeon RX 9070 XT (Windows, Adrenalin): - Two full sessions on build g54d01844. - One session of about 40 minutes on nightly g57471723, which includes this change. - No visible regression, no device loss. - "texture blit the device cannot make" does not appear in any of those logs. Every blit OoT makes on this device follows the rules, so the skip path is not reached there. This change guards against invalid usage; it does not fix a visible glitch in OoT. Assisted-by: GitHub Copilot### The bug ASTC textures are kept in one of three ways, depending on the GPU: - **Decoded to RGBA8** on most GPUs. - **Re-encoded as BC3** for some big textures. - **Kept as ASTC blocks** when the device samples ASTC natively (`textureCompressionASTC_LDR`, `astc_native()`). The image then has an ASTC `VkFormat`. `clear_tex` (gpu/vk/vk_xfer.cpp) clears a texture in one of two ways: 1. **Whole level**: when the clear covers the whole level, the image is not 3D, and the format is not BC (`raw`), it uses `vkCmdClearColorImage` for colour, or `vkCmdClearDepthStencilImage`. 2. **Region**: otherwise, a render pass over that level and layer with `vkCmdClearAttachments`. For BC images (`raw`) the rectangle is converted to blocks. `raw` is computed only for the BC range, so an image kept in ASTC blocks was treated as uncompressed. Step by step: 1. On a device with native ASTC, the game clears an ASTC texture. 2. If the clear covers the whole level, `clear_tex` calls `vkCmdClearColorImage` on the ASTC image. This breaks **VUID-vkCmdClearColorImage-image-00007**: "image must not have a compressed or depth/stencil format". 3. If it covers part of the level, the region path below only converts the rectangle to blocks for BC formats, not for ASTC. 4. Either way, a colour written over ASTC blocks would be garbage, since the blocks are compressed data, not texels. ### What changed (gpu/vk/vk_xfer.cpp, clear_tex) Before, no ASTC check: ```cpp full_barrier(cmd); const bool volume = s.tex->type == VK_IMAGE_TYPE_3D; const bool raw = s.tex->format >= VK_FORMAT_BC1_RGB_UNORM_BLOCK && s.tex->format <= VK_FORMAT_BC7_SRGB_BLOCK; ``` After: ```cpp full_barrier(cmd); if (s.tex->format >= VK_FORMAT_ASTC_4x4_UNORM_BLOCK && s.tex->format <= VK_FORMAT_ASTC_12x12_SRGB_BLOCK) { warn(14, "clear of an ASTC texture: skipped"); return; } const bool volume = s.tex->type == VK_IMAGE_TYPE_3D; ``` A clear of an image whose format is ASTC is skipped, and the image is left as it was. `warn` logs it at most 3 times. The early return also skips `texture_written` at the end of `clear_tex`, so the texture is not marked as GPU-written. That is on purpose, because nothing was written to it. ASTC textures decoded to RGBA8, ASTC kept as BC3, and every other format are cleared exactly as before. ### Who reaches this Only devices that expose `textureCompressionASTC_LDR` and sample ASTC natively, such as Intel iGPUs. AMD and NVIDIA desktop drivers don't expose it, so on them the image is never in an ASTC format and the new check never matches. ### Changes after review - The description no longer claims that drivers hang. The rule comes from the spec (the VUID above). - Skipping `texture_written` was confirmed as intended. ### Test There is no dedicated unit test. In the review run, `test_bc_raw_targets.py` and `test_vulkan_clear_mip.py` pass with this change. ### How tested - Release build. - Zelda OoT on Radeon RX 9070 XT (Windows, Adrenalin), including about 40 minutes on nightly g57471723, which includes this change. Log clean, and "clear of an ASTC texture" does not appear. - This GPU has no native ASTC, so the new path was not run. The test only checks that a GPU without native ASTC is unaffected. Assisted-by: GitHub Copilot### The bug `texel_buffer_view` (gpu/vk/vk_texture.cpp) creates the `VkBufferView` for a texture the game binds as a texel buffer: - as a **uniform** texel buffer when it is sampled, - as a **storage** texel buffer when it is written. The view's format is the one the texture maps to (`vk_format(nf).format`). The only check was that the format is not `VK_FORMAT_UNDEFINED`. Nothing checked whether the device supports texel buffers of that format. Step by step: 1. The game binds a texture as a texel buffer, in a format such as `VK_FORMAT_R32G32B32_SFLOAT`. 2. The device does not list `VK_FORMAT_FEATURE_UNIFORM_TEXEL_BUFFER_BIT` (or `STORAGE_TEXEL_BUFFER_BIT`) for it in the format's `bufferFeatures`. Which formats are listed varies by driver. 3. `texel_buffer_view` creates the view anyway. This is invalid usage: | VUID | Text | |---|---| | VUID-VkBufferViewCreateInfo-format-08778 | If the buffer view usage contains VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT, then format features of format must contain VK_FORMAT_FEATURE_UNIFORM_TEXEL_BUFFER_BIT | | VUID-VkBufferViewCreateInfo-format-08779 | If the buffer view usage contains VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT, then format features of format must contain VK_FORMAT_FEATURE_STORAGE_TEXEL_BUFFER_BIT | 4. What the shader then reads or writes through that view is undefined. ### What changed (gpu/vk/vk_texture.cpp, texel_buffer_view) A check right after the format is known, before anything is created, looked up, or marked as written: ```cpp VkFormat f = vk_format(nf).format; if (f == VK_FORMAT_UNDEFINED) return VK_NULL_HANDLE; // new: if (!(format_props(f).bufferFeatures & (storage ? VK_FORMAT_FEATURE_STORAGE_TEXEL_BUFFER_BIT : VK_FORMAT_FEATURE_UNIFORM_TEXEL_BUFFER_BIT))) { static std::atomic<int> logged{0}; if (logged.fetch_add(1, std::memory_order_relaxed) < 8) logf("vk: texel buffer of format %d (%s) not supported by the device: unbound", (int)f, storage ? "storage" : "uniform"); return VK_NULL_HANDLE; } ``` - A format without the needed feature gets no view, and the slot stays unbound. Callers already handle a null view; it is the same result as the existing `VK_FORMAT_UNDEFINED` case. - Because it returns before `texture_gpu_wrote`, a storage bind that is refused does not mark the texture as written by the GPU. - The answer comes from the existing `format_props` cache, which takes one short lock (`fmt_mu`) per texel-buffer bind but does not query the driver again. - The message is logged at most 8 times. - Supported formats follow the same path as before. ### Test `tests/test_texel_buffer_ownership.py` cuts `texel_buffer_view` out of vk_texture.cpp and compiles it against the stubs in `tests/texel_buffer_ownership_test.cpp`: - The fixture gets a `format_props` stub. It reports both texel-buffer features for every format except the one mapped from NVN format 44 (`VK_FORMAT_R32G32B32_SFLOAT`). - A new case, `unsupported`, binds format 44 as uniform and as storage. It checks that: - neither returns a view; - no view is created (`creates == 0`); - no buffer is looked up (`lookups == 0`); - the texture is not marked as written (`writes == 0`). - All 24 cases pass. On main, the case list has 23. ### Changes after review - The first version did not add the stub, so the test failed to compile ("'format_props' was not declared in this scope"). The stub and the `unsupported` case were added. ### How tested - Release build (`nx2/build.py --standalone`, Windows, llvm-mingw). - `tests/test_texel_buffer_ownership.py`: 24 cases pass. - Zelda OoT on Radeon RX 9070 XT (Windows, Adrenalin): - Two full sessions on build g54d01844. - One session of about 40 minutes on nightly g57471723, which includes this change. - No regression. - "texel buffer of format" does not appear in any of those logs. Every texel-buffer format OoT uses is supported on this device, so the new path is not reached there. This change guards against invalid usage; it does not fix a visible glitch in OoT. Assisted-by: GitHub Copilot### The bug At startup, `coopmat_init` (gpu/vk/vk_coopmat.cpp) checks whether native matrix multiplies can be used with cooperative matrices. `measure_order` and `test_multiply` each run a small compute shader through `run_shader`. `run_shader` does the following: 1. Creates a host-visible buffer and its memory, maps it, and copies the input into it. 2. Creates the pipeline, a descriptor pool, a command pool and a fence. 3. Submits one dispatch, waits for it for up to 5 s, and reads the result back. 4. Destroys everything. It had three problems. **1. The mapping was not checked.** ```cpp uint32_t* words = nullptr; vkMapMemory(dev.device, memory, 0, size, 0, (void**)&words); std::memcpy(words, data.data(), size); ``` If `vkMapMemory` failed (for example, out of host memory), `words` stayed null and the `memcpy` crashed the emulator at startup. **2. A dispatch that did not finish was destroyed under the GPU.** ```cpp ok = vkQueueSubmit2(dev.queue, 1, &si, fence) == VK_SUCCESS && vkWaitForFences(dev.device, 1, &fence, VK_TRUE, 5000000000ull) == VK_SUCCESS; ... vkDestroyFence(...); vkDestroyCommandPool(...); vkDestroyDescriptorPool(...); vkDestroyPipeline(...); // and at the end of the function: vkDestroyBuffer(...); vkFreeMemory(...); ``` When the submit succeeded but the wait timed out (a slow or hung driver), the code still destroyed everything while the GPU could still be running the dispatch. Each of those calls breaks a rule: | VUID | Text | |---|---| | VUID-vkDestroyFence-fence-01120 | All queue submission commands that refer to fence must have completed execution | | VUID-vkDestroyCommandPool-commandPool-00041 | All VkCommandBuffer objects allocated from commandPool must not be in the pending state | | VUID-vkDestroyDescriptorPool-descriptorPool-00303 | All submitted commands that refer to descriptorPool (via any allocated descriptor sets) must have completed execution | | VUID-vkDestroyPipeline-pipeline-00765 | All submitted commands that refer to pipeline must have completed execution | | VUID-vkDestroyBuffer-buffer-00922 | All submitted commands that refer to buffer, either directly or via a VkBufferView, must have completed execution | | VUID-vkFreeMemory-memory-00677 | All submitted commands that refer to memory (via images or buffers) must have completed execution | **3. The fence was created unchecked.** If `vkCreateFence` failed, the fence was a null handle, and the code still submitted with it and then waited on it. `vkWaitForFences` requires valid handles (VUID-vkWaitForFences-pFences-parameter: "pFences must be a valid pointer to an array of fenceCount valid VkFence handles"). ### What changed (gpu/vk/vk_coopmat.cpp, run_shader) - A failed mapping now logs "cooperative matrix test: the buffer could not be mapped", frees the buffer and memory, and fails the probe: ```cpp if (vkMapMemory(dev.device, memory, 0, size, 0, (void**)&words) != VK_SUCCESS || !words) { logf("vk: cooperative matrix test: the buffer could not be mapped"); vkDestroyBuffer(dev.device, buffer, nullptr); vkFreeMemory(dev.device, memory, nullptr); return false; } ``` - A fence that cannot be created makes the probe skip the submit. Nothing was queued, so everything is freed as usual and the probe fails: ```cpp if (vkCreateFence(dev.device, &fci, nullptr, &fence) != VK_SUCCESS) fence = VK_NULL_HANDLE; const bool submitted = fence && vkQueueSubmit2(dev.queue, 1, &si, fence) == VK_SUCCESS; ok = submitted && vkWaitForFences(dev.device, 1, &fence, VK_TRUE, 5000000000ull) == VK_SUCCESS; ``` - A dispatch that was submitted but did not complete leaves the fence, command pool, descriptor pool, pipeline, buffer and memory alive, instead of destroying them under the GPU. Only the shader module is destroyed, which is allowed once the pipeline exists. This is a one-time leak at startup; on a device that really hung, later submits would queue behind it anyway. ```cpp if (submitted && !ok) { // (it may still be running: what it uses is left alive rather than destroyed under it) if (module) vkDestroyShaderModule(dev.device, module, nullptr); return false; } ``` The probe's result and every successful path are unchanged. A probe that fails falls back to software matrix multiplies, as before. ### Note for #1975 #1975 (cooperative matrices for the AA networks) changes `run_shader`'s signature. It merges cleanly with this PR, but whichever lands second should check that the new early returns still free what #1975 adds. ### Test There is no dedicated unit test. The failure paths need a driver that fails `vkMapMemory` or `vkCreateFence`, or a dispatch that takes more than 5 s. ### How tested - Release build. - Zelda OoT on Radeon RX 9070 XT (Windows, Adrenalin), including about 40 minutes on nightly g57471723, which includes this change. Log clean. - On this GPU, `coopmat_init` stops before the probe: the log says "matrix multiplies in software (the cooperative matrices lack 16x8x16 f16)". So `run_shader` is not reached there, and none of the changed paths ran. The test only shows that startup is unaffected. Assisted-by: GitHub Copilot### The bug A sparse texture gets device memory per block (tile) as it is written, instead of all at once (gpu/vk/vk_texture.cpp): - `sparse_queue` walks the tiles a write reaches. For each tile that has no memory yet, it takes memory with `sparse_take`, appends a bind to `sparse_binds`, and **marks the tile bound at once** (`bound = 1`). The mip tail works the same way, with `sparse_opaque` and `tail_bound`. - `sparse_flush` then binds the whole list with `vkQueueBindSparse` and waits on a fence. When the bind failed, `sparse_flush` only logged it: ```cpp res = vkQueueBindSparse(dev.queue, 1, &bi, fence); ... if (res == VK_SUCCESS) { vkWaitForFences(...); vkResetFences(...); } else { logf("vk: sparse texture %#llx: binding failed (%d)", ...); } ... sparse_binds.clear(); sparse_opaque.clear(); return res == VK_SUCCESS; ``` Step by step: 1. A write to a sparse texture queues the binds of tiles A, B and C. Their `tiles[...]` entries are set to 1. 2. `vkQueueBindSparse` fails (for example, out of memory). The list is cleared, but the tiles stay marked bound. 3. The upload's copies go ahead and write tiles A, B and C, which have no memory behind them. 4. Every later write to those tiles sees them marked bound, so it never queues them again. Those tiles stay unbacked for the life of the texture, and every write to them is lost. Two more problems were in the same function: - The fence was created without checking. If `vkCreateFence` failed, every bind was submitted and waited on with a null fence. `vkWaitForFences` requires valid handles (VUID-vkWaitForFences-pFences-parameter: "pFences must be a valid pointer to an array of fenceCount valid VkFence handles"). - The memory `sparse_take` gave for the failed binds was simply lost to that texture. ### What changed (gpu/vk/vk_texture.cpp and vk_texture.h) - **New `sparse_unqueue(tex)`.** When the binds are not made, it: - marks every queued tile unbound again (`map[i] = 0`), finding the tile with the same math `sparse_queue` uses; - marks every queued mip tail unbound again (`tail_bound[layer] = 0`); - puts the memory those binds had taken on the texture's free list; - clears the queue, and counts `sparse_added` as `sparse_flush` does. The next write to those tiles queues them again, so the bind is retried. - **`sparse_flush` calls it in both failure cases**, and returns false: ```cpp if (vkCreateFence(dev.device, &fi, nullptr, &fence) != VK_SUCCESS) { fence = VK_NULL_HANDLE; static int logged; if (logged++ < 8) logf("vk: sparse texture %#llx: no fence for its binds", ...); sparse_unqueue(tex); return false; } ... } else { static int failed; if (failed++ < 8) logf("vk: sparse texture %#llx: binding failed (%d)", ...); sparse_unqueue(tex); return false; } ``` - **New per-texture free list** (`Texture::sparse_free`, with memory, offset and size). `sparse_take` gives it out before taking new chunk memory. A bind that keeps failing reuses the same memory instead of taking new memory on every retry. The list only points at the texture's own chunks, which are freed with the texture. - **Both messages are capped at 8.** A failing bind is now retried on later writes, so without the cap it would log on every write to that texture. The success path is unchanged. ### Who reaches this A texture is only sparse when it is a 2D array larger than `RX_VK_TEX_MAX_MB` (1.5 GB) on a device with sparse support. That is MKW's course-map case. OoT likely never makes one. ### Changes after review - The "binding failed" and "no fence for its binds" messages are capped at 8, like the other sparse messages. - The description now says when a texture is sparse, and that the OoT test does not reach this path. ### Test There is no dedicated unit test. The failure needs `vkQueueBindSparse` or `vkCreateFence` to fail on a sparse texture. ### How tested - Release build. - Zelda OoT on Radeon RX 9070 XT (Windows, Adrenalin), including about 40 minutes on nightly g57471723, which includes this change. Log clean, with no "sparse texture" line. - The sparse path itself was not run. The test only shows that non-sparse textures are unaffected. Assisted-by: GitHub Copilot513646e95875ca015d1175ca015d117a479474a47a479474a4c5e19e1c8bc5e19e1c8b7bf35db418WIP: chore: restore 24 Hookshot PRs merged on GitHub after Longshot branchedto WIP: chore: restore 81 Hookshot PRs merged on GitHub after Longshot branchedThanks for this. Bringing back the 81 merged Hookshot fixes Longshot lost when it branched is worth doing, and the PR is careful about it. Each restored PR is its own commit, under its original author (29 of them), with the upstream title and body. The 9 left out are each listed with a reason, and I agree with all of them: Hookshot #2972 and Hookshot #3038 are already in Longshot, the four Builder-window ones went with #181, Hookshot #2946 and Hookshot #2926 are covered on Longshot's side, and Hookshot #2582 needs a real port onto Longshot's cap. Some commits needed a port, and 7 of them say how in a "Ported to Longshot:" note.
I read the PR-only diff against
main@origin(fork point2331f46c), the commit messages, and the renderer, host and setup commits in particular. I didn't build, run or compare with upstream, since GitHub is down.Checked, and fine as far as I can tell:
MKWVK_API_VERSIONgoes 21 → 22 for the two interface changes:MkwVkHost.wide_cover(https://github.com/NotRexed/Hookshot/pull/2939) andHOS_PAD_TOUCHPADmoving insideHOS_PAD_N(https://github.com/NotRexed/Hookshot/pull/2989).pad_name,touchpad_bound,nn_hid.c's range checks andprompts.c'sHOS_PAD_ART_Nfall-back all agree on the new value.HOS_OPT_AUTO_RUN = 55doesn't collide with another option.fg_scale_comp_spv.handfg_scale_pq_comp_spv.hname the SHA-256 offg_scale.compas it is at the PR (6c105b2d…).third_party/isn't touched. Thegpu/vk/siritandgpu/vk/edenedits only drop "(super build)" tags (https://github.com/NotRexed/Hookshot/pull/2782), and those folders aren't under the provenance check.tests/srccut.py(https://github.com/NotRexed/Hookshot/pull/2914) makes the 80 checks it touches stricter: a missing or repeated marker now fails. I sampledtest_vsync_row.py,test_commit_limit.py,test_low_vram_texture_mips.pyand the stand-ins commit7bf35db4, and none of them is weakened. The Windowsunsetenvshim is still there.HOS_FS_OVERLAYandthread_arg's own-thread fast path (https://github.com/NotRexed/Hookshot/pull/2708). The condition-variable early return counts a waiter in beforecv->lk, so it misses no wake-up the old Signal would have given.move_retry(https://github.com/NotRexed/Hookshot/pull/3021):faildoesn't return, andpublish_installrestoresGetLastError.void *return.nx2/oot_patches.py. They don't overlap.Must change
90bfb06a(https://github.com/NotRexed/Hookshot/pull/2732,scripts/dev_packs.py). Main already has it, as #147 (fdc2840b). Main's version is newer: itsCACHES,WRITTEN_IN_PLAYandNO_MODULEcover more (including.forgejo/), and itsprepare()handles a stalled driver. This commit adds the older 695-line file over it, along with a second "Testing a change against a base" section indocs/development.mdand a duplicate line inAGENTS.md. A rebase will conflict on all four files, and taking this side would undo #147.main, which also makes "Rebased on currentmain" true. Right now the box is ticked but false: main has moved five commits since2331f46c(#214, #197, #147, #78, #54). Besides the dev_packs conflict, #197 changedvk_replay.cpp,vk_draw.cpp,vk_main.cpp,vk_internal.handtests/fsr_uniform_ownership_test.cpp, all of which this PR also edits. Re-runrun_all.pyafter the rebase.Should change
Split it before it merges. One PR of 81 commits can't be reviewed or reverted as a unit, and the "One topic" box is honestly unticked. Here's one way to cut it, from safest to riskiest:
scripts/test_requests.py's commands for the whole team, so it should merge separately.If you split it, Hookshot #2939 and Hookshot #2989 each change the interface, so each piece needs its own
MKWVK_API_VERSIONbump (22, then 23), not one shared 22.Each Hookshot reference needs its full URL. Without one,
#NNNNlinks to Longshot's own number:#2949and similar for Hookshot PRs, mixed in with Longshot's real#77,#181and#215. Usehttps://github.com/NotRexed/Hookshot/pull/NNNNfor the Hookshot ones.Fixes #2180,#2182,#2220,#2579,#2954,#2885and#2667(in Hookshot #2766, Hookshot #3044, Hookshot #2939, Hookshot #2625, Hookshot #2978, Hookshot #2933 and Hookshot #2929). Forgejo reads those as closing Longshot issues once Longshot's numbers get there. Reword them, or add aRestored-from: https://github.com/NotRexed/Hookshot/pull/NNNNtrailer to each commit. A trailer would also give the commits-to-Hookshot mapping a home outside this description.docs/test-requests.md's "Hookshot's PR #2731" (should be https://github.com/NotRexed/Hookshot/pull/2731). Others are indocs/tested-configurations.md(9 rows),tests/srccut.py,vk_memory.cppandvk_texture.cpp.The staging prewarm (https://github.com/NotRexed/Hookshot/pull/2960) doesn't keep what it makes. The 4 × 64 MB arenas made in
replay_startgo throughtexture_frame's existing keep and idle rules. On a card with 6.5 GB or less,keepis 64 MB, so three of the four are freed at the first frame. On every card, all of them are freed after 30 s untouched. So it pays 256 MB of host-visible memory at start for little lasting effect, and the title's "keep" isn't implemented. Either mark the prewarmed arenas as kept, or leave this part out of rc.4. Inline comment below.Small description fixes:
HOS_FS_OVERLAYis in the flags table as if it were new, but it isn't.Not checked:
vk_draw.cpp: Hookshot #2625, Hookshot #2896, Hookshot #2983 and Hookshot #2987nn_hid.c's input changes in Hookshot #2944, Hookshot #2962 and Hookshot #2963, beyond reading the difflongshot_bench.pyand the Hookshot #2830 port oftest_requests.pyVerdict: request changes. Items 1 to 3 have to be fixed before this can merge, and it's still WIP.
@ -94,0 +114,4 @@### Which commit?`request` pins the PR's head at that moment. For example, on November 12, Hookshot's PR #2731 moved from `bce3d17e` toThe bare
#2731links to Longshot's own PR 2731 here. Please write it as https://github.com/NotRexed/Hookshot/pull/2731, and do the same for the other Hookshot numbers this PR adds todocs/, comments and the description.@ -16,3 +16,3 @@#endif#define MKWVK_API_VERSION 21#define MKWVK_API_VERSION 2222 covers both interface changes here:
MkwVkHost.wide_cover(https://github.com/NotRexed/Hookshot/pull/2939) andHOS_PAD_TOUCHPADmoving insideHOS_PAD_N(https://github.com/NotRexed/Hookshot/pull/2989). That's right while they stay in one PR. If the PR is split, whichever piece merges second needs its own bump to 23.@ -518,0 +578,4 @@had gone stale and been given back, so the first upload of the next burst made another. On an empty device they are cheap.RX_VK_STAGING_PREWARM=<n>: how many, 0: none. */} // namespace (the prewarm is called from the replay's start: it has to link)void texture_staging_prewarm(int n) {These arenas don't survive long. They go through
texture_frame's existing free loop (if (idle && (it->size > kArena || kept + it->size > keep || stale))). On a card with 6.5 GB or less,keepis 64 MB, so three of the four are freed at the first frame. On every card, all of them are freed 30 s afterused_at, which is set here. So the commit title's "keep the staging arenas" isn't implemented, and on small cards this costs 256 MB of host-visible (BAR) memory at start for nothing. Either exempt the prewarmed ones from the keep and idle rules, for example with a flag onArena, or leave this half of https://github.com/NotRexed/Hookshot/pull/2960 out for rc.4.@ -2347,0 +2561,4 @@bool game_order() {static const bool on = [] {const char* e = getenv("RX_VK_GAME_ORDER");const bool v = e ? !env_value_off(e) : dev.props.vendorID == 0x10de;This is a new default for every NVIDIA player: back-to-back passes and direct dispatches get no barrier of their own (https://github.com/NotRexed/Hookshot/pull/2559). The PR has no Longshot measurement for it. For rc.4, please say what it was measured on and what it costs if the game's barriers miss a hazard, or hold it for after rc.4.
@ -0,0 +1,695 @@"""Development builds as a base and small packs over it: build the game once, then build only what a change touches and run itMain already has this script, as #147 (
fdc2840b), and main's version is newer. ItsCACHESandWRITTEN_IN_PLAYcover the whole cache family,NO_MODULEincludes.forgejo/, andprepare()handles a stalled driver (PREPARE_FAILED). This commit (Hookshot https://github.com/NotRexed/Hookshot/pull/2732) would conflict with it here, intests/test_dev_packs.py, indocs/development.mdand inAGENTS.md, and taking this side would undo #147. Please drop90bfb06afrom the branch.Thanks for restoring these, TrashPanda. Three of the commits here are already on Longshot from me, or in an open PR, and HumanBean's review of #156 asked us to settle which PR carries them:
bd2b56d3(Hookshot #2708, three per-call costs) is the same change as #156 (WIP, same code and tests; #156 adds a docs/development.md row and is waiting for a Longshot A/B). I'm happy for #209 to carry it: I'd close #156 and post the docs row and the Longshot measurement here as a comment for you to take or leave. Or, if you'd rather keep #209 to what has no Longshot PR yet, dropbd2b56d3and #156 carries it. Your call.3a04664c(Hookshot #2896, native shader keys in draw statistics) is #169 (WIP, refreshed onto current main with a fixture that fails on the old source). Same question; I'll follow whichever you prefer.90bfb06a(Hookshot #2732, dev_packs) is already merged as #147 (fdc2840b, with fixes from its review), so it can be dropped here.Thanks for the careful review. I've split this PR as you suggested, so I'm closing it: every commit is accounted for
below, in the five PRs or as dropped.
restore/tools-checks-docs, 20 commitsrestore/test-request-pins, 2 commitsrestore/rc4-player-fixes, 21 commits, test request T-DV9Q, claimable once a maintainer adds thetesting wantedlabelrestore/renderer-fixes, 22 commits, test request T-TQY6, claimable once a maintainer adds thetesting wantedlabelrestore/defaults-after-rc4, 26 commits, test request T-Y5MJ, claimable once a maintainer adds thetesting wantedlabelEach one is based on current
main(514591a3), passes the game-free checks, and has been compiled for both platforms(details in each PR). They are all WIP until they've had their runs in the game.
Must change
Done. It's in none of the split PRs, so main's newer
scripts/dev_packs.py,tests/test_dev_packs.py, thedevelopment.md section and the AGENTS.md line stay as #147 left them. One more commit went the same way:
4fc02edb(the floor web above 60 fps, https://github.com/NotRexed/Hookshot/pull/2929) reached main as #49 after thisPR opened, with that version's comments, so it's dropped too.
Done, twice. I first rebased this branch onto
fbd42024(#214, #197, #147, #78, #54 and the rest). #197's files mergedwithout conflicts. The front cache's timers in
build_descriptorsconflicted with #107's handle tables; I kept #107'scode and put the timers inside it.
run_allthen found one break: #55's upscaler round-trip check cutssave(), whichhttps://github.com/NotRexed/Hookshot/pull/2966 makes call
logfandlstat, and its fixture declared neither. Thatfix is in #286. Main then moved to
d9648863, and then to514591a3(#274, docs only), and the split branches are rebased onto that. Two more things came up:Hookshot's Auto run option number, 55, is now main's Damage taken row (#138), so Auto run takes 56 in #288 (its commit
says so); and the draw statistics'
StatsQuery(https://github.com/NotRexed/Hookshot/pull/2896) now sits beside #260'sOccGuard, with main's line kept, and #260's occlusion check names the newStatsQuery(cmd, p, d)call in#288. Each split PR's run_all is in its Testing section: no failure other than
test_lift_cache, which fails on main the same way here.Partly done. I found tools.lock's llvm-mingw on this PC, so the Windows target was cross-built here, and Linux was built
with clang and lld. I have no game build here, so this covers what can be built without one. For each split branch, and
for all five merged together,
nx2/build.py --standalonewith an empty lift compiled every source in the repository thatthe game links (the host, its libraries and the Vulkan back end: about 400 files per platform) with no failure. The link
stopped only at the symbols the game's lifted code provides, the same list as on main.
RX_VK_ONLYlinkedlongshot_vk.soandlongshot_vk.dll, andbuilder/setup/setup.clinked asLongshotBuilder-Setup.exe. What's stillmissing is a full game build on each platform and a short run in the game. Those are open as test requests: T-DV9Q, claimable once a maintainer adds the
testing wantedlabelfor #286, T-TQY6, claimable once a maintainer adds the
testing wantedlabel for #287 and T-Y5MJ, claimable once a maintainer adds thetesting wantedlabel for #288. The PRs stay WIP until they're done.Should change
Done, with your cut: a #284, b #285, c #286, d #287, e #288. The 11 tested-configuration rows and the README
form-feed fix are in a. My stand-in commit is split by what it stands in for (
ViewFastin e,compile_allin d). EachPR lists the Hookshot PRs it restores, with their authors, and asks for a rebase merge so each author keeps their
commit.
Done, in order. #286 (touchpad binding, https://github.com/NotRexed/Hookshot/pull/2989) takes 22. #287 (ultrawide
loading cover, https://github.com/NotRexed/Hookshot/pull/2939) takes 23 in a commit of its own, assuming c merges first.
Both descriptions say so, and say that if d merges first, it takes 22 and c takes 23. The Auto run option in e changes no
struct, so it doesn't bump.
Done. Every restored commit now has a
Restored-from: https://github.com/NotRexed/Hookshot/pull/NNNNtrailer. Thebare numbers in its message are full URLs, including
Fixes #2180,#2182,#2220,#2579,#2954,#2885and#2667, so Forgejo won't close Longshot issues with those numbers. The(#NNNN)at the end of each title went into thetrailer, and other numbers in titles read "Hookshot PR NNNN" or "Hookshot issue NNNN". In the restored text, each PR
has a commit of mine that writes the bare Hookshot numbers it adds as URLs. That covers
docs/test-requests.md's #2731,the tested-configuration rows,
tests/srccut.py,vk_memory.cpp,vk_texture.cpp,target_overlap.hand thechecks' docstrings, with the comments rewrapped. Longshot's own numbers (#55, #77, #107, #138, #171, #177, #181) are
left as they are. The descriptions use URLs too.
Left out, as you suggested. In #288 the commit is the texture-view front cache and the statistics only, retitled to
match, with a "Ported to Longshot" note saying why the prewarm half is gone. Keeping the arenas would need an exemption
on
Arenaand a check of its own, and that can be its own PR.Done in the new descriptions. juliotec's fixes are eight, listed in #287.
HOS_FS_OVERLAYisn't presented as new.https://github.com/NotRexed/Hookshot/pull/2559's NVIDIA default is in #288, after rc.4, marked as needing a Longshot
measurement, with what it risks and its test request. The new switches with no entry in docs/development.md now have
one (d:
RX_VK_TEMPORAL_CUT,RX_VK_SPLIT_UBFLUSH; e: seven).Line comments
Dropped (point 1).
Done (point 4): c is 22 and d is 23, and both descriptions say what to do if they merge in the other order.
Left out (point 6).
Held out of the rc.4 PRs. It's in #288, after rc.4, and that description says it needs a measurement (main against the
PR in Kokiri Forest and Hyrule Field,
RX_VK_GAME_ORDER=1against=0, two interleaved runs each) and what it risksif the game's barriers miss a hazard: flicker or wrong pixels, with
RX_VK_GAME_ORDER=0to turn it off. Test requestT-Y5MJ, claimable once a maintainer adds the
testing wantedlabel.Done (point 5): https://github.com/NotRexed/Hookshot/pull/2731, in #285, and the rest in each PR.
Not checked
I didn't review the items on your not-checked list beyond building them and running the checks. The pipeline-list
binaries in e are as they merged upstream.
Every commit of #209 and where it went (82: 81 restored and mine)
5300084810d978ebee2692b6033d0dab39fef1dee83a8948e3211468e2a4b9a6ec7d65c7b77f804d40f423ed0d380a2849f75dc8and the la...b33e13df641bba35d2ca5830fc649e08e1927326ce9e6504f424c82052752fcd90bfb06ae7922e447e8e5b06ec01d41e8a8295434fc02edbb67d539e288cb74459c9587f4160ef2b68aea4a5abd42c6efd061e48c9a088941575198fe8391dc5688ae12974baebcf41c2482f8bcab34f839af6ec6a7f3123f4e8fc27140480be1238a5e2ba93d6401fbf8fa475f31e38da3a67f70d1c01789d5f5a43a4a52e420c5b3c012193b94b9bc5496215f0c5947612160da483172faece421205c2c55cd56f1f2f3a04664c7ea7b16b6d4565ebe7565a7fbd2b56d35a99777179c1152e1fa26f07b2e4cea9c20aeb64e099e47549c34ebe585003607fcb7a65bfd36e7b2231454fa10e32a23d0e622bd6b1c6ca998564b97bf35db4ViewFaststand-ins went to #288, thecompile_allone to #287Pull request closed