MagicBeanDev/Hookshot
Archived
Watch
6
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

feat: menu, renderer and build plumbing for #1831/#1833; upload hitches; lock diagnostics (builds with #1831, #1833) #1834

Closed
FIocker wants to merge 1 commit from FIocker/super-build/core into main
pull from: FIocker/super-build/core
merge into: MagicBeanDev:main
Conversation 6 Commits 1 Files changed 25 +1060 -38
FIocker commented 2026-11-10 17:46:35 +00:00 (Migrated from github.com)

The upscaling (#1831) and neural rendering (#1833) work needs menu rows, renderer hooks and build changes; this PR is those. It also fixes two upload hitches and adds the lock diagnostics docs/performance.md is built on.

#1831, #1833 and this PR build only together. This PR includes vk_dlssg.h, vk_ffx.h, framegen_methods.h, vk_dlssnr*.h and vk_opennr.h, and the other two use this one's rows and hooks. Please review and merge the three as one unit; FIocker:super-build/all is the three together, and it's what was built and tested. #1830 is separate and builds alone.

Part Branch Builds
Shader fixes super-build/shaders (#1830) on its own
Upscaling and frame generation super-build/upscaling-framegen (#1831) with #1833 and this PR
DLSS 5 neural rendering super-build/neural (#1833) with #1831 and this PR
Menu, renderer and build plumbing (this PR) super-build/core (#1834) with #1831 and #1833

What it does

Insert menu (vk_menu.*):

  • The Neural tab is TAB_NEURAL = 5, after main's Audio tab (TAB_COUNT = 6), with its rows, including Stage (after or before the upscaler).
  • Saved keys: the tab's rows are saved as nr_<name> (nr_model, nr_style, nr_preset, …) by one rule in ini_key. The switch itself keeps neural_rendering.
  • Frame generation method: Auto, NVIDIA DLSS or Built-in, of what runs here (framegen_methods.h).
  • Upscaler row: AMD's FidelityFX FSR 4 counts as FSR 4 where main's FSR 4 engine isn't built (ffx::upscale_supported).

Renderer:

  • vk_main.cpp: device features and extensions for NGX and OpenDLSS-NR.
  • vk_replay.cpp: the present's NR pass is skipped when the chain already ran it. The headless present now uses the same call and guard (nr::apply behind !dlss_nr_before_hud()), so a headless run with Stage = Before upscaler no longer applies NR twice.
  • vk_draw.cpp: the scene's view-projection capture, and skinned-draw detection for the new actor formats.
  • The scene depth handed to NR is now the largest depth target of this frame, not the largest ever (scene_depth.frame). scene_frame_inputs answers only for this frame's or the last one's. scene_depth and scene_vp are written under a lock, since RX_VK_PARALLEL's workers resolve targets too.

Upload hitches (vk_texture.cpp, vk_memory.cpp):

  • No import for one upload: upload_unswizzle_gpu uses the GPU path only from a pool that is already imported (memory_guest_contained). An import made for one upload took 22-25 ms on the replay thread, and three of them, when the camera turns to the Deku Tree, made a 60 ms hitch. RX_VK_UNSWIZZLE_IMPORT=1 brings back the import.
  • Threaded CPU unswizzle from 4 MB instead of 16 MB. The scene-size targets uploaded at that moment (2779 × 1318 RGBA8, 15 MB) took about 20 ms each on one thread.

Lock diagnostics (vk_internal.h, vk_memory.cpp, vk_texture.*): ParLock counts by file and line, and memory_pool_lock_reasons and texture_touch_misses count why the two busiest sites take the lock. These cost nothing while parallel replay is off. docs/performance.md has the numbers; it no longer contradicts itself and names functions instead of line numbers.

NVIDIA's shader disk cache is opt-in: nvidia_shader_cache_gb=<GB> in hookshot_options.ini, with no menu row. It sets __GL_SHADER_DISK_CACHE_SIZE and turns trimming off, only when asked and only where the environment doesn't set them. It's documented in docs/performance.md.

Host: nn_vi.c's HOS_DISPLAY_VRR=1 (opt-in) tells the game the display does variable refresh, which lifts its 60 fps limiter, and SetDisplay120HzRefreshRateCapability is answered.

Build (nx2/build.py): NGX flags for every file that sees NVIDIA's headers (vk_dlss, vk_ngx, vk_dlssg, vk_dlssnr_ngx), OpenDLSS-NR's PTX kernels, and Streamline's SDK path for the NR backend.

Tests: main's fixtures that cut our changed functions out get the stubs they now need (direct_buffer_writes, dlss_sdk_cache, framegen_preset, present_geometry_trace, present_source_crop, stick_dead_zone, temporal_jitter, upscale_independent, audio_output_menu, rumble_menu, shader_stamp_cache). None of their checks are weakened.

Taken out since the first review

Everything below is gone from the series:

  • Fork release and update plumbing: semantic-release and --repository-url, package.json, repository.txt / HOOKSHOT_REPOSITORY, the prebuilt-renderer path, and the Linux workflow's NGX SDK fetch. Main has moved to release-please, and the rest was inactive on Windows anyway.
  • The old Upscaling-row migration (old_upscaling 3/4) and the extra tx_upscale labels. tx_upscale is main's.
  • hos_sdl_refresh_hz() and the HOS_VSYNC_HZ mention: neither was used.
  • The duplicate includes in vk_main.cpp and vk_menu.cpp.
  • Streamline frame generation: its row and vk_slfg.cpp, nx2_sl.h and its hooks in Reflex, the device and the present (see #1831).

Testing

  • Tests: tests/run_all.py on the three together (super-build/all), locally (clang targeting MSVC, the CPM Vulkan headers, glslang 11.12): 328 passed. The only failures are the same 18 that main at d7a73062 has on this machine, all down to this PC's toolchain.
  • Build and game: Windows 11, NVIDIA RTX, OoT v131072, MSVC build with the NGX SDK; results in #1831 and #1833.
  • Screenshots of the Neural tab and the Frame generation method row will follow in a comment.

🤖 Generated with Claude Code

The upscaling (#1831) and neural rendering (#1833) work needs menu rows, renderer hooks and build changes; this PR is those. It also fixes two upload hitches and adds the lock diagnostics `docs/performance.md` is built on. **#1831, #1833 and this PR build only together.** This PR includes `vk_dlssg.h`, `vk_ffx.h`, `framegen_methods.h`, `vk_dlssnr*.h` and `vk_opennr.h`, and the other two use this one's rows and hooks. Please review and merge the three as one unit; [`FIocker:super-build/all`](https://github.com/FIocker/Hookshot/tree/super-build/all) is the three together, and it's what was built and tested. #1830 is separate and builds alone. | Part | Branch | Builds | |---|---|---| | Shader fixes | `super-build/shaders` (#1830) | on its own | | Upscaling and frame generation | `super-build/upscaling-framegen` (#1831) | with #1833 and this PR | | DLSS 5 neural rendering | `super-build/neural` (#1833) | with #1831 and this PR | | Menu, renderer and build plumbing (this PR) | `super-build/core` (#1834) | with #1831 and #1833 | ## What it does **Insert menu** (`vk_menu.*`): - **The Neural tab** is `TAB_NEURAL = 5`, after main's Audio tab (`TAB_COUNT = 6`), with its rows, including **Stage** (after or before the upscaler). - **Saved keys**: the tab's rows are saved as `nr_<name>` (`nr_model`, `nr_style`, `nr_preset`, …) by one rule in `ini_key`. The switch itself keeps `neural_rendering`. - **Frame generation method**: Auto, NVIDIA DLSS or Built-in, of what runs here (`framegen_methods.h`). - **Upscaler row**: AMD's FidelityFX FSR 4 counts as FSR 4 where main's FSR 4 engine isn't built (`ffx::upscale_supported`). **Renderer:** - **`vk_main.cpp`**: device features and extensions for NGX and OpenDLSS-NR. - **`vk_replay.cpp`**: the present's NR pass is skipped when the chain already ran it. The headless present now uses the same call and guard (`nr::apply` behind `!dlss_nr_before_hud()`), so a headless run with Stage = Before upscaler no longer applies NR twice. - **`vk_draw.cpp`**: the scene's view-projection capture, and skinned-draw detection for the new actor formats. - **The scene depth handed to NR** is now the largest depth target of *this* frame, not the largest ever (`scene_depth.frame`). `scene_frame_inputs` answers only for this frame's or the last one's. `scene_depth` and `scene_vp` are written under a lock, since `RX_VK_PARALLEL`'s workers resolve targets too. **Upload hitches** (`vk_texture.cpp`, `vk_memory.cpp`): - **No import for one upload**: `upload_unswizzle_gpu` uses the GPU path only from a pool that is already imported (`memory_guest_contained`). An import made for one upload took 22-25 ms on the replay thread, and three of them, when the camera turns to the Deku Tree, made a 60 ms hitch. `RX_VK_UNSWIZZLE_IMPORT=1` brings back the import. - **Threaded CPU unswizzle from 4 MB** instead of 16 MB. The scene-size targets uploaded at that moment (2779 × 1318 RGBA8, 15 MB) took about 20 ms each on one thread. **Lock diagnostics** (`vk_internal.h`, `vk_memory.cpp`, `vk_texture.*`): `ParLock` counts by file and line, and `memory_pool_lock_reasons` and `texture_touch_misses` count why the two busiest sites take the lock. These cost nothing while parallel replay is off. `docs/performance.md` has the numbers; it no longer contradicts itself and names functions instead of line numbers. **NVIDIA's shader disk cache is opt-in**: `nvidia_shader_cache_gb=<GB>` in `hookshot_options.ini`, with no menu row. It sets `__GL_SHADER_DISK_CACHE_SIZE` and turns trimming off, only when asked and only where the environment doesn't set them. It's documented in `docs/performance.md`. **Host**: `nn_vi.c`'s `HOS_DISPLAY_VRR=1` (opt-in) tells the game the display does variable refresh, which lifts its 60 fps limiter, and `SetDisplay120HzRefreshRateCapability` is answered. **Build** (`nx2/build.py`): NGX flags for every file that sees NVIDIA's headers (`vk_dlss`, `vk_ngx`, `vk_dlssg`, `vk_dlssnr_ngx`), OpenDLSS-NR's PTX kernels, and Streamline's SDK path for the NR backend. **Tests**: main's fixtures that cut our changed functions out get the stubs they now need (`direct_buffer_writes`, `dlss_sdk_cache`, `framegen_preset`, `present_geometry_trace`, `present_source_crop`, `stick_dead_zone`, `temporal_jitter`, `upscale_independent`, `audio_output_menu`, `rumble_menu`, `shader_stamp_cache`). None of their checks are weakened. ## Taken out since the first review Everything below is gone from the series: - **Fork release and update plumbing**: semantic-release and `--repository-url`, `package.json`, `repository.txt` / `HOOKSHOT_REPOSITORY`, the prebuilt-renderer path, and the Linux workflow's NGX SDK fetch. Main has moved to release-please, and the rest was inactive on Windows anyway. - **The old Upscaling-row migration** (`old_upscaling` 3/4) and the extra `tx_upscale` labels. `tx_upscale` is main's. - **`hos_sdl_refresh_hz()`** and the `HOS_VSYNC_HZ` mention: neither was used. - **The duplicate includes** in `vk_main.cpp` and `vk_menu.cpp`. - **Streamline frame generation**: its row and `vk_slfg.cpp`, `nx2_sl.h` and its hooks in Reflex, the device and the present (see #1831). ## Testing - **Tests**: `tests/run_all.py` on the three together (`super-build/all`), locally (clang targeting MSVC, the CPM Vulkan headers, glslang 11.12): 328 passed. The only failures are the same 18 that main at d7a73062 has on this machine, all down to this PC's toolchain. - **Build and game**: Windows 11, NVIDIA RTX, OoT v131072, MSVC build with the NGX SDK; results in #1831 and #1833. - **Screenshots** of the Neural tab and the Frame generation method row will follow in a comment. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 commented 2026-11-10 17:51:07 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
hckrman101 (Migrated from github.com) reviewed 2026-11-11 02:21:48 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for splitting the super build up, it makes it a lot easier to read. I went through this one alongside #1830, #1831 and #1833 (and #1892, which touches the same vk_draw.cpp / vk_menu.cpp / vk_menu.h). I'm leaving a comment rather than an approval: the branch is 321 commits behind main and conflicting (see my earlier note), so no CI has run on it. A few things also need fixing before it's worth rebasing.

How the series fits together. This PR alone doesn't compile. vk_main.cpp, vk_menu.cpp and vk_replay.cpp include vk_dlssg.h, vk_ffx.h, vk_slfg.h and framegen_methods.h (from #1831), and vk_streamline.h, vk_dlssnr*.h and vk_opennr.h (from #1833). They also call dlss_nr_before_hud() and use the dlss_actor::kSingle/kPair/kNoWeight flags, both from #1831. That much the description says. But #1830 isn't independent either: its vk_texture.cpp calls memory_guest_contained(), which is added here, and this PR's vk_replay.cpp calls texture_touch_misses(), which only #1830 adds. (The description lists vk_texture.h under this PR, but it isn't in the diff.) So all four have to land together, and none of them can go green in CI alone. Could you either make each one build on its own (move the shared declarations into the PR that uses them first, or stack the PRs) or say plainly in all four descriptions that they only build together? #1892 also edits vk_draw.cpp, vk_menu.cpp and vk_menu.h, so expect conflicts there too.

What the rebase will hit on main:

  • Main now has an Audio tab: TAB_AUDIO = 1 … TAB_DEBUG = 4, TAB_COUNT = 5. TAB_NEURAL = 4 lands on Debug, and tab_titles drops Audio.
  • Releases moved to release-please. package.json is gone, and release.yml no longer runs semantic-release, so the --repository-url change and the conventional-changelog-conventionalcommits dependency have nothing to apply to.
  • .github/workflows/linux-appimage.yml isn't on main any more.
  • tests/run_all.py reads per-test RUN_ALL_ARGS / RUN_ALL_SKIP declarations since #1822. The new entries belong in the tests themselves, not in the central table. (Right now the table also has tests/test_fsr3_shaders.py twice.)

Bugs:

  • tests/test_rumble_menu.py raises a NameError: "wheel": block(tick, loop) uses two names that menu_fixture() never defines, and the template has no %(wheel)s anyway. It looks like a merge leftover (inline).
  • scene_depth only ever grows (inline). It keeps the largest depth target ever seen and is never reset. Lower the render resolution, or have the texture cache drop that depth image, and scene_frame_inputs() keeps handing a destroyed image and view to the open NR backend. Under RX_VK_PARALLEL, targets() and capture_view_projection() also write scene_depth / scene_vp from worker threads without a lock.
  • On the headless/offscreen path, do_present calls dlssnr::apply without the !dlss_nr_before_hud() guard that present_image has, and through a different entry point (nr::apply is used in the window path). With Stage = Before upscaler, a headless run applies NR twice (inline).
  • The description says nn_vi.c gets HOS_VSYNC_HZ and hos_sdl.* gets hos_sdl_refresh_hz() for it. Only the comment mentions HOS_VSYNC_HZ, there's no code that reads it, and nothing in any of the four PRs calls hos_sdl_refresh_hz().

Things NotRexed should decide (not mine to call):

  • The menu side of retiring AMD FSR 3.1 / FSR 4 frame generation. framegen_available() never sets the FSR bits, so with #1831's vk_framegen.cpp, FSR 3.1's 2x frame generation, which runs on any GPU on main today, stops for everyone, AMD and Intel included. The menu keeps dead "AMD FSR 4" / "AMD FSR 3.1" entries and hints. If FSR FG misbehaved on your setup, that's worth an issue with details, not a silent removal.
  • The Streamline frame generation row is added and then hidden for good in row_shown(), but it still takes a restart and saves streamline_frame_generation. If it's retired, I'd leave the row out entirely.
  • Linux CI fetching NVIDIA's NGX SDK with --accept-nvidia-license and linking it into the published AppImage is a release and licence decision.
  • vk_main.cpp sets __GL_SHADER_DISK_CACHE_SIZE=10GB and SKIP_CLEANUP=1 for every Windows player who hasn't set them. That's a big default on someone else's disk, and the description doesn't mention it.
  • The fork-aware updates (repository.txt, HOOKSHOT_REPOSITORY) and the prebuilt-renderer path (tools/prebuilt, tools/amd-fsr) have nothing to do with the menu or renderer, and they'd be easier to judge as their own PR. On Windows they're also inert as written: windows-builder.yml passes neither --vk, --amd-fsr, --repository nor a HOOKSHOT_REPOSITORY define, so the setup's and Builder's repository and the prebuilt DLL are never produced.

Nits:

  • Fork-specific migration code in an upstream PR: old_upscaling 3/4 → opts.dlss, plus tx_upscale gaining "FSR 3.1"/"DLSS" and window_filter(). Upstream's Upscaling row never had 3 or 4.
  • The Neural rows' ini keys are the generic model, style, preset, intensity, local_tone… You already special-cased nr_backend / nr_stage for exactly this reason. Prefixing the rest with nr_ too would avoid a clash with a future row.
  • The vk_menu.h comment on nr_backend lists 0–2, but the row has a 3 (Open).
  • docs/performance.md contradicts itself. "It is not the lock, and tuning does not rescue it" is followed by "It is serialised on one lock … splitting that lock … is the work that would make it pay". It also quotes vk_draw.cpp:1820-style line numbers that will drift.
  • vk_main.cpp includes vk_dlss.h twice, and vk_menu.cpp includes vk_fsr3.h / vk_fsr3fi.h twice.
  • AGENTS.md asks for screenshots when a change shows on screen. The new Neural tab and the Frame generation method row qualify.

Happy to look again once it's rebased and the series builds piece by piece (or is clearly marked as one unit).

Thanks for splitting the super build up, it makes it a lot easier to read. I went through this one alongside #1830, #1831 and #1833 (and #1892, which touches the same `vk_draw.cpp` / `vk_menu.cpp` / `vk_menu.h`). I'm leaving a comment rather than an approval: the branch is 321 commits behind main and conflicting (see my earlier note), so no CI has run on it. A few things also need fixing before it's worth rebasing. **How the series fits together.** This PR alone doesn't compile. `vk_main.cpp`, `vk_menu.cpp` and `vk_replay.cpp` include `vk_dlssg.h`, `vk_ffx.h`, `vk_slfg.h` and `framegen_methods.h` (from #1831), and `vk_streamline.h`, `vk_dlssnr*.h` and `vk_opennr.h` (from #1833). They also call `dlss_nr_before_hud()` and use the `dlss_actor::kSingle/kPair/kNoWeight` flags, both from #1831. That much the description says. But #1830 isn't independent either: its `vk_texture.cpp` calls `memory_guest_contained()`, which is added here, and this PR's `vk_replay.cpp` calls `texture_touch_misses()`, which only #1830 adds. (The description lists `vk_texture.h` under this PR, but it isn't in the diff.) So all four have to land together, and none of them can go green in CI alone. Could you either make each one build on its own (move the shared declarations into the PR that uses them first, or stack the PRs) or say plainly in all four descriptions that they only build together? #1892 also edits `vk_draw.cpp`, `vk_menu.cpp` and `vk_menu.h`, so expect conflicts there too. **What the rebase will hit on main:** - Main now has an Audio tab: `TAB_AUDIO = 1` … `TAB_DEBUG = 4`, `TAB_COUNT = 5`. `TAB_NEURAL = 4` lands on Debug, and `tab_titles` drops Audio. - Releases moved to release-please. `package.json` is gone, and `release.yml` no longer runs semantic-release, so the `--repository-url` change and the `conventional-changelog-conventionalcommits` dependency have nothing to apply to. - `.github/workflows/linux-appimage.yml` isn't on main any more. - `tests/run_all.py` reads per-test `RUN_ALL_ARGS` / `RUN_ALL_SKIP` declarations since #1822. The new entries belong in the tests themselves, not in the central table. (Right now the table also has `tests/test_fsr3_shaders.py` twice.) **Bugs:** - `tests/test_rumble_menu.py` raises a NameError: `"wheel": block(tick, loop)` uses two names that `menu_fixture()` never defines, and the template has no `%(wheel)s` anyway. It looks like a merge leftover (inline). - `scene_depth` only ever grows (inline). It keeps the largest depth target ever seen and is never reset. Lower the render resolution, or have the texture cache drop that depth image, and `scene_frame_inputs()` keeps handing a destroyed image and view to the open NR backend. Under `RX_VK_PARALLEL`, `targets()` and `capture_view_projection()` also write `scene_depth` / `scene_vp` from worker threads without a lock. - On the headless/offscreen path, `do_present` calls `dlssnr::apply` without the `!dlss_nr_before_hud()` guard that `present_image` has, and through a different entry point (`nr::apply` is used in the window path). With Stage = Before upscaler, a headless run applies NR twice (inline). - The description says `nn_vi.c` gets `HOS_VSYNC_HZ` and `hos_sdl.*` gets `hos_sdl_refresh_hz()` for it. Only the comment mentions `HOS_VSYNC_HZ`, there's no code that reads it, and nothing in any of the four PRs calls `hos_sdl_refresh_hz()`. **Things NotRexed should decide** (not mine to call): - The menu side of retiring AMD FSR 3.1 / FSR 4 frame generation. `framegen_available()` never sets the FSR bits, so with #1831's `vk_framegen.cpp`, FSR 3.1's 2x frame generation, which runs on any GPU on main today, stops for everyone, AMD and Intel included. The menu keeps dead "AMD FSR 4" / "AMD FSR 3.1" entries and hints. If FSR FG misbehaved on your setup, that's worth an issue with details, not a silent removal. - The Streamline frame generation row is added and then hidden for good in `row_shown()`, but it still takes a restart and saves `streamline_frame_generation`. If it's retired, I'd leave the row out entirely. - Linux CI fetching NVIDIA's NGX SDK with `--accept-nvidia-license` and linking it into the published AppImage is a release and licence decision. - `vk_main.cpp` sets `__GL_SHADER_DISK_CACHE_SIZE=10GB` and `SKIP_CLEANUP=1` for every Windows player who hasn't set them. That's a big default on someone else's disk, and the description doesn't mention it. - The fork-aware updates (`repository.txt`, `HOOKSHOT_REPOSITORY`) and the prebuilt-renderer path (`tools/prebuilt`, `tools/amd-fsr`) have nothing to do with the menu or renderer, and they'd be easier to judge as their own PR. On Windows they're also inert as written: `windows-builder.yml` passes neither `--vk`, `--amd-fsr`, `--repository` nor a `HOOKSHOT_REPOSITORY` define, so the setup's and Builder's repository and the prebuilt DLL are never produced. **Nits:** - Fork-specific migration code in an upstream PR: `old_upscaling` 3/4 → `opts.dlss`, plus `tx_upscale` gaining "FSR 3.1"/"DLSS" and `window_filter()`. Upstream's Upscaling row never had 3 or 4. - The Neural rows' ini keys are the generic `model`, `style`, `preset`, `intensity`, `local_tone`… You already special-cased `nr_backend` / `nr_stage` for exactly this reason. Prefixing the rest with `nr_` too would avoid a clash with a future row. - The `vk_menu.h` comment on `nr_backend` lists 0–2, but the row has a 3 (Open). - `docs/performance.md` contradicts itself. "It is not the lock, and tuning does not rescue it" is followed by "It is serialised on one lock … splitting that lock … is the work that would make it pay". It also quotes `vk_draw.cpp:1820`-style line numbers that will drift. - `vk_main.cpp` includes `vk_dlss.h` twice, and `vk_menu.cpp` includes `vk_fsr3.h` / `vk_fsr3fi.h` twice. - AGENTS.md asks for screenshots when a change shows on screen. The new Neural tab and the Frame generation method row qualify. Happy to look again once it's rebased and the series builds piece by piece (or is clearly marked as one unit).
gpu/vk/vk_draw.cpp
hckrman101 (Migrated from github.com) commented 2026-11-11 02:21:48 +00:00

This is a high-water mark that's never reset. If the render resolution drops, or the cache drops this depth texture, nothing smaller replaces it, and scene_frame_inputs() keeps returning a destroyed VkImage / VkImageView to the open NR backend. Could it be refreshed per frame instead, e.g. the largest one seen this frame, cleared at present? It's also written from worker threads under RX_VK_PARALLEL with no lock (same for scene_vp).

This is a high-water mark that's never reset. If the render resolution drops, or the cache drops this depth texture, nothing smaller replaces it, and `scene_frame_inputs()` keeps returning a destroyed `VkImage` / `VkImageView` to the open NR backend. Could it be refreshed per frame instead, e.g. the largest one seen *this* frame, cleared at present? It's also written from worker threads under `RX_VK_PARALLEL` with no lock (same for `scene_vp`).
gpu/vk/vk_main.cpp
@ -37,7 +43,42 @@ VKX_DEVICE3(VKX_DEFINE)
static PFN_vkGetInstanceProcAddr get_instance_proc;
hckrman101 (Migrated from github.com) commented 2026-11-11 02:21:49 +00:00

10 GB with no cleanup is a big default to put on every Windows player's disk, and the description doesn't mention it. I'd make it opt-in, or at least call it out in the description and docs.

10 GB with no cleanup is a big default to put on every Windows player's disk, and the description doesn't mention it. I'd make it opt-in, or at least call it out in the description and docs.
gpu/vk/vk_menu.cpp
@ -62,7 +69,10 @@ enum MenuTab {
TAB_GAMEPLAY = 2,
TAB_CONTROLS = 3,
hckrman101 (Migrated from github.com) commented 2026-11-11 02:21:49 +00:00

On main, tab 4 is TAB_DEBUG now (an Audio tab went in at 1), so the rebase needs TAB_NEURAL = 5, TAB_COUNT = 6, and Audio kept in tab_titles.

On main, tab 4 is `TAB_DEBUG` now (an Audio tab went in at 1), so the rebase needs `TAB_NEURAL = 5`, `TAB_COUNT = 6`, and Audio kept in `tab_titles`.
gpu/vk/vk_menu.cpp
@ -629,0 +710,4 @@
static const char* t[] = {"Auto", "NVIDIA DLSS", "AMD FSR 4", "AMD FSR 3.1", "Built-in"};
return t[fgm::effective(v, framegen_available())];
}
const char* hn_framegen_method(int v, int variant) {
hckrman101 (Migrated from github.com) commented 2026-11-11 02:21:49 +00:00

With framegen_available() never setting the FSR3 bit, this is never true, and with #1831 FSR 3.1's 2x frame generation no longer runs for anyone, though it works on main on any GPU. That's a decision for NotRexed, but it should be one on purpose, with the reason written down.

With `framegen_available()` never setting the FSR3 bit, this is never true, and with #1831 FSR 3.1's 2x frame generation no longer runs for anyone, though it works on main on any GPU. That's a decision for NotRexed, but it should be one on purpose, with the reason written down.
gpu/vk/vk_replay.cpp
hckrman101 (Migrated from github.com) commented 2026-11-11 02:21:49 +00:00

The window path (present_image) uses nr::apply behind !dlss_nr_before_hud(). This path calls dlssnr::apply (Streamline only?) without the guard, so headless runs with Stage = Before upscaler apply NR twice, and the NGX/Open backends are skipped here. Same call and guard as present_image?

The window path (`present_image`) uses `nr::apply` behind `!dlss_nr_before_hud()`. This path calls `dlssnr::apply` (Streamline only?) without the guard, so headless runs with Stage = Before upscaler apply NR twice, and the NGX/Open backends are skipped here. Same call and guard as `present_image`?
host/nn_vi.c
hckrman101 (Migrated from github.com) commented 2026-11-11 02:21:49 +00:00

HOS_VSYNC_HZ is mentioned here and in the description, but nothing reads it, and hos_sdl_refresh_hz() isn't called anywhere in the series. Either the code is missing or the comment and description are ahead of it.

`HOS_VSYNC_HZ` is mentioned here and in the description, but nothing reads it, and `hos_sdl_refresh_hz()` isn't called anywhere in the series. Either the code is missing or the comment and description are ahead of it.
tests/test_rumble_menu.py
hckrman101 (Migrated from github.com) commented 2026-11-11 02:21:48 +00:00

tick and loop aren't defined in menu_fixture() (or at module level), so this raises a NameError and the whole check fails. FIXTURE has no %(wheel)s either. Looks like a leftover from a merge: I think the line can just go.

`tick` and `loop` aren't defined in `menu_fixture()` (or at module level), so this raises a NameError and the whole check fails. `FIXTURE` has no `%(wheel)s` either. Looks like a leftover from a merge: I think the line can just go.
FIocker commented 2026-11-11 10:45:40 +00:00 (Migrated from github.com)

Thanks for going through the whole series; it changed a lot. I rebuilt it on main at d7a73062. The descriptions are rewritten.

How the series fits together: #1830 now builds on its own. Its vk_texture.cpp changes, memory_guest_contained and texture_touch_misses moved here, and I checked it with the full test suite locally. #1831, #1833 and this one still need each other, and all three descriptions now say so plainly, at the top, with a table. Splitting them along file lines would mean shipping half-wired rows, so I'd rather they're reviewed as one unit; super-build/all is the three together. The vk_texture.h line in the old description was wrong; it's fixed.

What the rebase hit:

  • Tabs: TAB_NEURAL = 5 after main's Audio tab, TAB_COUNT = 6, and Audio kept in tab_titles.
  • Release plumbing gone: semantic-release, --repository-url, the npm dependency, package.json and .releaserc.json are all gone (main uses release-please). So are linux-appimage.yml's changes, including the NGX SDK fetch.
  • run_all.py: main's per-test RUN_ALL_ARGS / RUN_ALL_SKIP. The opennr tests and test_framegen_methods.py declare theirs; nothing goes in the retired tables, and the duplicate test_fsr3_shaders.py entry went with them.

Bugs:

  • test_rumble_menu.py: the leftover wheel line is gone.
  • scene_depth: now the largest depth of this frame (scene_depth.frame), and scene_frame_inputs answers only for this frame's or the last one's. Both scene_depth and scene_vp are written under a lock.
  • Headless present: nr::apply behind !dlss_nr_before_hud(), the same call and guard as present_image.
  • HOS_VSYNC_HZ and hos_sdl_refresh_hz(): both removed, since they were unused.

Your list for NotRexed:

  • FSR frame generation: main itself made FSR 3.1's interpolation opt-in on 2026-11-10 (RX_VK_FSR_FG=1, #1635), so the series follows main there. #1831 says why FidelityFX FSR 4's isn't offered either, and the method enum keeps both values so a saved choice reads as Auto.
  • Streamline frame generation: removed completely (vk_slfg.cpp, nx2_sl.h, its row, and its hooks in Reflex, the device and the present).
  • Linux NGX SDK in CI: removed.
  • The 10 GB NVIDIA cache: now opt-in, nvidia_shader_cache_gb=<GB> in the options file, documented in docs/performance.md.
  • Fork-aware updates and the prebuilt renderer: removed from the series.

Nits:

  • Upscaling migration: old_upscaling and the extra tx_upscale labels are gone.
  • ini keys: the Neural rows are saved as nr_<name> by one rule in ini_key.
  • Comments: the nr_backend comment lists 0-3.
  • Docs: docs/performance.md no longer contradicts itself and names functions, not line numbers.
  • Includes: the duplicates in vk_main.cpp and vk_menu.cpp are gone.
  • Screenshots of the Neural tab and the Frame generation method row will follow from an in-game run.

Tests: tests/run_all.py on the three together, locally (clang targeting MSVC, the CPM Vulkan headers, glslang 11.12), gives 328 passed. The failures are only the same 18 main has on this machine. Main's fixtures that cut our functions out now get stubs for them, with no checks weakened.

Thanks for going through the whole series; it changed a lot. I rebuilt it on main at d7a73062. The descriptions are rewritten. **How the series fits together:** #1830 now builds on its own. Its `vk_texture.cpp` changes, `memory_guest_contained` and `texture_touch_misses` moved here, and I checked it with the full test suite locally. #1831, #1833 and this one still need each other, and all three descriptions now say so plainly, at the top, with a table. Splitting them along file lines would mean shipping half-wired rows, so I'd rather they're reviewed as one unit; `super-build/all` is the three together. The `vk_texture.h` line in the old description was wrong; it's fixed. **What the rebase hit:** - **Tabs**: `TAB_NEURAL = 5` after main's Audio tab, `TAB_COUNT = 6`, and Audio kept in `tab_titles`. - **Release plumbing gone**: semantic-release, `--repository-url`, the npm dependency, `package.json` and `.releaserc.json` are all gone (main uses release-please). So are `linux-appimage.yml`'s changes, including the NGX SDK fetch. - **`run_all.py`**: main's per-test `RUN_ALL_ARGS` / `RUN_ALL_SKIP`. The opennr tests and `test_framegen_methods.py` declare theirs; nothing goes in the retired tables, and the duplicate `test_fsr3_shaders.py` entry went with them. **Bugs:** - **`test_rumble_menu.py`**: the leftover `wheel` line is gone. - **`scene_depth`**: now the largest depth of *this* frame (`scene_depth.frame`), and `scene_frame_inputs` answers only for this frame's or the last one's. Both `scene_depth` and `scene_vp` are written under a lock. - **Headless present**: `nr::apply` behind `!dlss_nr_before_hud()`, the same call and guard as `present_image`. - **`HOS_VSYNC_HZ` and `hos_sdl_refresh_hz()`**: both removed, since they were unused. **Your list for NotRexed:** - **FSR frame generation**: main itself made FSR 3.1's interpolation opt-in on 2026-11-10 (`RX_VK_FSR_FG=1`, #1635), so the series follows main there. #1831 says why FidelityFX FSR 4's isn't offered either, and the method enum keeps both values so a saved choice reads as Auto. - **Streamline frame generation**: removed completely (`vk_slfg.cpp`, `nx2_sl.h`, its row, and its hooks in Reflex, the device and the present). - **Linux NGX SDK in CI**: removed. - **The 10 GB NVIDIA cache**: now opt-in, `nvidia_shader_cache_gb=<GB>` in the options file, documented in `docs/performance.md`. - **Fork-aware updates and the prebuilt renderer**: removed from the series. **Nits:** - **Upscaling migration**: `old_upscaling` and the extra `tx_upscale` labels are gone. - **ini keys**: the Neural rows are saved as `nr_<name>` by one rule in `ini_key`. - **Comments**: the `nr_backend` comment lists 0-3. - **Docs**: `docs/performance.md` no longer contradicts itself and names functions, not line numbers. - **Includes**: the duplicates in `vk_main.cpp` and `vk_menu.cpp` are gone. - **Screenshots** of the Neural tab and the Frame generation method row will follow from an in-game run. **Tests**: `tests/run_all.py` on the three together, locally (clang targeting MSVC, the CPM Vulkan headers, glslang 11.12), gives 328 passed. The failures are only the same 18 main has on this machine. Main's fixtures that cut our functions out now get stubs for them, with no checks weakened.
hckrman101 commented 2026-11-11 15:51:01 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 21:46:10 +00:00
Joey-Magnus (Migrated from github.com) left a comment

With #1831 closed in favour of #2118, this plumbing no longer has the series it was built for, so I'd close it. It includes vk_dlssg.h, vk_ffx.h and framegen_methods.h from #1831 and vk_dlssnr*.h / vk_opennr.h from #1833. It also conflicts with main in seven files: vk_menu.cpp, vk_menu.h, vk_replay.cpp, nx2/build.py, two present tests and test_shader_stamp_cache.py. Two pieces stand on their own and would be welcome as small PRs on main. One is the upload hitch fix: no pool import for a single upload_unswizzle_gpu, and the threaded CPU unswizzle from 4 MB. The other is the lock diagnostics. If the vk_main.cpp default for __GL_SHADER_DISK_CACHE_SIZE / SKIP_CLEANUP comes back, it needs its own PR and a maintainer decision, as hckrman101 noted, since it changes disk use for every Windows NVIDIA player.

With #1831 closed in favour of #2118, this plumbing no longer has the series it was built for, so I'd close it. It includes vk_dlssg.h, vk_ffx.h and framegen_methods.h from #1831 and vk_dlssnr*.h / vk_opennr.h from #1833. It also conflicts with main in seven files: vk_menu.cpp, vk_menu.h, vk_replay.cpp, nx2/build.py, two present tests and test_shader_stamp_cache.py. Two pieces stand on their own and would be welcome as small PRs on main. One is the upload hitch fix: no pool import for a single upload_unswizzle_gpu, and the threaded CPU unswizzle from 4 MB. The other is the lock diagnostics. If the vk_main.cpp default for __GL_SHADER_DISK_CACHE_SIZE / SKIP_CLEANUP comes back, it needs its own PR and a maintainer decision, as hckrman101 noted, since it changes disk use for every Windows NVIDIA player.
github-sync closed this pull request 2026-11-12 13:39:02 +00:00
github-sync commented 2026-11-12 13:39:04 +00:00
Owner

@FIocker commented on GitHub · 2026-11-12 12:54 UTC · original

Closing, as suggested. With #1831 gone this plumbing has no series left, so its pieces went where they belong:

  • The upload hitch fix is #2785: no pool import for one GPU unswizzle (now read with env_on), and the CPU unswizzle threaded from 4 MB.
  • The lock diagnostics and docs/performance.md are #2786.
  • The menu rows, nr_settings() and the chain and present hooks that neural rendering needs are being folded into #1833, rebuilt on main on top of #2118's NGX session.
  • The NVIDIA shader-cache default isn't coming back in any of these. If it ever does, it'll be its own PR for a maintainer decision.

Thanks for the reviews.

> [@FIocker](https://github.com/FIocker) commented on GitHub · 2026-11-12 12:54 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1834#issuecomment-6824179980) Closing, as suggested. With #1831 gone this plumbing has no series left, so its pieces went where they belong: - **The upload hitch fix** is #2785: no pool import for one GPU unswizzle (now read with `env_on`), and the CPU unswizzle threaded from 4 MB. - **The lock diagnostics and `docs/performance.md`** are #2786. - **The menu rows, `nr_settings()` and the chain and present hooks** that neural rendering needs are being folded into #1833, rebuilt on main on top of #2118's NGX session. - **The NVIDIA shader-cache default** isn't coming back in any of these. If it ever does, it'll be its own PR for a maintainer decision. Thanks for the reviews. <!-- github-sync:c:6824179980 -->
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Hookshot!1834
No description provided.