feat(vk): add experimental DLSS 5 Neural Rendering #1590

Open
HypeZ85 wants to merge 5 commits from HypeZ85/codex/dlss5-neural-rendering into main
HypeZ85 commented 2026-11-10 10:09:22 +00:00 (Migrated from github.com)

Adds optional DLSS 5 Neural Rendering to the existing Vulkan NGX path, with persistent controls and in-game A/B comparison. Missing or incompatible NR runtimes fall back to ordinary rendering and report the reason only inside the NR submenu.

  • Integrates main through ba005ca4, retaining its menu localization, keyboard prompts, DLSS SR/FG recovery, Graphics sections and compact tuning bar, direct NGX Frame Generation, native HDR tone mapper, reactive SR mask, FOV and secondary controller bindings.
  • NR persistence coexists with language preferences: language.txt owns game language, debug stays session-only and Compare keeps transactional save masking. Completes the existing translation tables for 121 NR source strings in all 13 non-English locales, including controls, profiles, tooltips, runtime errors and Compare. Existing non-NR translations remain unchanged. Compare translates components before composition; text fitting clips translated UTF-8 text. No runtime translation service is used. Initial translations were machine-assisted, with Italian and technical terminology review; native-speaker review remains welcome.
  • Adds Graphics > DLSS 5 Neural Rendering, immediately below DLSS model, hidden without NGX. Base/Advanced share one state; Advanced offers General/Quality.
  • Compare, Retry detection and Reset NR settings accept confirm, Right and mouse input. Compare Toolbar position has no full-menu placement preview.
  • Compare Advanced adds a three-row toolbar / parameter carousel / value editor. Up/Down changes rows; Left/Right browses parameters or adjusts values. Mouse wheel and draggable sliders are supported; both General and Quality are accessible. Shortcut mode and Retry/Reset remain in the full menu. The user approved this interface in the running Windows game.
  • Adds profiles, requested/effective state and F8/controller Toggle/Hold comparison with transactional Save/Cancel. Compare skips NR while Off and retains its feature; ordinary Off releases NR resources after two seconds.
  • Reuses maximum SR input allocations with actual per-frame subrects for dynamic resolution. Preserves SR history during unavailable NR and native HDR fallback. Drains the recorder before native NGX calls.
  • Provides linear SDR color composition and real buffer diagnostics. Debug views are session-only; fault injection is developer-only. NR Statistics and timestamp collection are removed, preserving the upstream FPS counter.
  • Addresses both maintainers' follow-ups: descriptive entry/order, UTF-8 repair, documented NR overrides, removal of RX_VK_DLSS_START, nonblocking periodic availability checks and menu-key fixture anchors/stubs. The shortcut fixture now exercises F1 cancelling Compare with queued and polled input; existing assertions remain intact.

This uses NGX feature 18 (Reserved18 in the public SDK) and community-documented DLSSNR.* keys with a user-supplied nvngx_dlssnr.dll. It is not a public NVIDIA NR SDK contract and may change with driver/runtime updates. NR defaults Off; an additional environment shipping gate remains a maintainer policy decision. Linux has no known compatible native NR runtime and offers experimental opt-in probing/fallback only. No Streamline or other intermediate layer is introduced.

No verified NR jitter-offset keys exist in the inspected contract/runtime. Jittered input therefore resets NR history each frame while SR retains its real jitter/history. This conservative fallback is not a claim of temporally stable NR.

Validation:

  • Final Windows NGX renderer link passes. The complete API-19 game build succeeded (3:57 total, 1:31 compilation), and the three-row comparison controls were tested and approved by the user in the running game before merging the last five upstream commits. No performance gains or new temporal-quality/controller/aspect-ratio coverage are claimed. The final upstream V-Sync/Frame Generation fix has compile/source-check coverage, without a new live GPU run.
  • Final complete Windows suite: 464 pass, 3 fail, 34 skip. All NR and affected-menu checks pass, including all-locale coverage, DLL/environment identifier preservation and compiled production translation-lookup tests. Multilingual visual layout/glyph coverage has not been verified in the running game. Remaining failures: Windows symlink privileges, the upstream input-background unused-global warning under clang 23/Werror, and the new upstream prompt-skip fixture writing a manifest into a missing directory. The latter fixture and implementation are identical to main. All six .NET fixtures now pass with SDK 8.0.428. No tests were weakened.
  • Linux checks could not be rerun because Docker's Linux engine is unavailable. Earlier Linux object/CPU checks and RTX 5080 / NR 310.9.16 subrect and lifecycle probes are recorded as historical evidence in the validation document, not validation of the latest merge on Linux.
  • Physical-controller coverage, all aspect ratios and temporal NR image quality remain limited. The earlier intermittent Windows exit/TDR report is unresolved; no general shutdown fix is claimed.

See implementation and contract, Linux limitations and detailed validation. No proprietary runtime, SDK, game data, binary, save or cache is included.

Contributor screenshots of the updated menu. The subsequent descriptive label and placement were approved in the running game before publication. No screenshots or generated previews were captured by the agent.

Adds optional DLSS 5 Neural Rendering to the existing Vulkan NGX path, with persistent controls and in-game A/B comparison. Missing or incompatible NR runtimes fall back to ordinary rendering and report the reason only inside the NR submenu. - Integrates main through `ba005ca4`, retaining its menu localization, keyboard prompts, DLSS SR/FG recovery, Graphics sections and compact tuning bar, direct NGX Frame Generation, native HDR tone mapper, reactive SR mask, FOV and secondary controller bindings. - NR persistence coexists with language preferences: language.txt owns game language, debug stays session-only and Compare keeps transactional save masking. Completes the existing translation tables for 121 NR source strings in all 13 non-English locales, including controls, profiles, tooltips, runtime errors and Compare. Existing non-NR translations remain unchanged. Compare translates components before composition; text fitting clips translated UTF-8 text. No runtime translation service is used. Initial translations were machine-assisted, with Italian and technical terminology review; native-speaker review remains welcome. - Adds **Graphics > DLSS 5 Neural Rendering**, immediately below **DLSS model**, hidden without NGX. Base/Advanced share one state; Advanced offers General/Quality. - Compare, Retry detection and Reset NR settings accept confirm, Right and mouse input. Compare Toolbar position has no full-menu placement preview. - Compare Advanced adds a three-row toolbar / parameter carousel / value editor. Up/Down changes rows; Left/Right browses parameters or adjusts values. Mouse wheel and draggable sliders are supported; both General and Quality are accessible. Shortcut mode and Retry/Reset remain in the full menu. The user approved this interface in the running Windows game. - Adds profiles, requested/effective state and F8/controller Toggle/Hold comparison with transactional Save/Cancel. Compare skips NR while Off and retains its feature; ordinary Off releases NR resources after two seconds. - Reuses maximum SR input allocations with actual per-frame subrects for dynamic resolution. Preserves SR history during unavailable NR and native HDR fallback. Drains the recorder before native NGX calls. - Provides linear SDR color composition and real buffer diagnostics. Debug views are session-only; fault injection is developer-only. NR Statistics and timestamp collection are removed, preserving the upstream FPS counter. - Addresses both maintainers' follow-ups: descriptive entry/order, UTF-8 repair, documented NR overrides, removal of RX_VK_DLSS_START, nonblocking periodic availability checks and menu-key fixture anchors/stubs. The shortcut fixture now exercises F1 cancelling Compare with queued and polled input; existing assertions remain intact. This uses **NGX feature 18 (Reserved18 in the public SDK)** and community-documented DLSSNR.* keys with a user-supplied nvngx_dlssnr.dll. It is not a public NVIDIA NR SDK contract and may change with driver/runtime updates. NR defaults Off; an additional environment shipping gate remains a maintainer policy decision. Linux has no known compatible native NR runtime and offers experimental opt-in probing/fallback only. No Streamline or other intermediate layer is introduced. No verified NR jitter-offset keys exist in the inspected contract/runtime. Jittered input therefore resets NR history each frame while SR retains its real jitter/history. This conservative fallback is not a claim of temporally stable NR. Validation: - Final Windows NGX renderer link passes. The complete API-19 game build succeeded (3:57 total, 1:31 compilation), and the three-row comparison controls were tested and approved by the user in the running game before merging the last five upstream commits. No performance gains or new temporal-quality/controller/aspect-ratio coverage are claimed. The final upstream V-Sync/Frame Generation fix has compile/source-check coverage, without a new live GPU run. - Final complete Windows suite: **464 pass, 3 fail, 34 skip**. All NR and affected-menu checks pass, including all-locale coverage, DLL/environment identifier preservation and compiled production translation-lookup tests. Multilingual visual layout/glyph coverage has not been verified in the running game. Remaining failures: Windows symlink privileges, the upstream input-background unused-global warning under clang 23/Werror, and the new upstream prompt-skip fixture writing a manifest into a missing directory. The latter fixture and implementation are identical to main. All six .NET fixtures now pass with SDK 8.0.428. No tests were weakened. - Linux checks could not be rerun because Docker's Linux engine is unavailable. Earlier Linux object/CPU checks and RTX 5080 / NR 310.9.16 subrect and lifecycle probes are recorded as historical evidence in the validation document, not validation of the latest merge on Linux. - Physical-controller coverage, all aspect ratios and temporal NR image quality remain limited. The earlier intermittent Windows exit/TDR report is unresolved; no general shutdown fix is claimed. See [implementation and contract](https://github.com/HypeZ85/Hookshot/blob/codex/dlss5-neural-rendering/docs/dlss5-neural-rendering.md), [Linux limitations](https://github.com/HypeZ85/Hookshot/blob/codex/dlss5-neural-rendering/docs/dlss5-linux.md) and [detailed validation](https://github.com/HypeZ85/Hookshot/blob/codex/dlss5-neural-rendering/docs/dlss5-review-validation.md). No proprietary runtime, SDK, game data, binary, save or cache is included. [Contributor screenshots of the updated menu](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/1590). The subsequent descriptive label and placement were approved in the running game before publication. No screenshots or generated previews were captured by the agent.
HypeZ85 commented 2026-11-10 15:19:28 +00:00 (Migrated from github.com)

i'll leave a couple screenshot of the result:
with "intense" profile:
image

with "balanced" and using the in-game preview mode:
image

the new tab DLSS 5 into the options:
image

another couple screenshots of the "intense" profile:
image

image
i'll leave a couple screenshot of the result: with "intense" profile: <img width="3440" height="1440" alt="image" src="https://github.com/user-attachments/assets/df19cd04-7464-4121-966e-bd5d418d66d9" /> with "balanced" and using the in-game preview mode: <img width="1722" height="752" alt="image" src="https://github.com/user-attachments/assets/59a04a5e-f07d-4722-9a39-ecae5598f085" /> the new tab DLSS 5 into the options: <img width="1722" height="752" alt="image" src="https://github.com/user-attachments/assets/2168a73e-d24d-4a35-9bce-162c3ef5670f" /> another couple screenshots of the "intense" profile: <img width="3440" height="1440" alt="image" src="https://github.com/user-attachments/assets/53b6c4e9-c37c-4700-8093-e2662dce06a9" /> <img width="3440" height="1440" alt="image" src="https://github.com/user-attachments/assets/9a522ca8-ed3f-4ec8-8fcd-e12a3095ffdb" />
hckrman101 commented 2026-11-10 16:04:43 +00:00 (Migrated from github.com)

Can we have DLSS 5 / neural rendering as a sub-menu within graphics instead of it's own tab?

Can we have DLSS 5 / neural rendering as a sub-menu within graphics instead of it's own tab?
hckrman101 (Migrated from github.com) reviewed 2026-11-10 16:10:41 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for this, it's a lot of careful work: the candidate/rollback around feature creation, the deferred release through later(), the coalesced edits and the non-blocking timestamps all look right to me. I'm leaving COMMENT because a few things in the render path change how DLSS SR behaves even for people who never get NR running.

What "Neural Rendering" means here should be spelled out for maintainers. This is NGX feature 18, which the public SDK still calls Reserved18. The DLSSNR.* parameter names come from community documentation, not from NVIDIA headers, and it needs a user-supplied nvngx_dlssnr.dll. On Linux there's no known native runtime, so that path only probes. The code gates all of this properly at runtime, and the no-SDK stubs look complete. Still, it's an undocumented contract that a driver update can change under us, so whether to ship it outside an env opt-in is a maintainer call.

+1 to the earlier comment about making this a sub-menu under Graphics. Two more data points:

  • The DLSS 5 tab is shown in every build, including ones without HOOKSHOT_NGX and on AMD/Intel. row_shown() has no dlss_built() check for TAB_DLSS5, while the Upscaler row does hide via ur::upscaler_shown(dlss_built(), ...). So most users get a tab of ~19 rows that can only say "not built".
  • There's already a mechanism for this: the Controls tab's Motion row opens its own page (TAB_MOTION). A "DLSS 5" row under the Upscaler, shown only when DLSS is built, could open the same way. That would also drop the TAB_COUNT churn in test_audio_output_menu / test_framegen_preset / test_rumble_menu / test_stick_dead_zone.

Other points:

  • Tests: the four new test_dlss_nr_*.py default to --cxx g++ but aren't in ARGS in tests/run_all.py. run_all only checks for the compiler when it's named there, so a runner without g++ fails these instead of skipping them. Please add them like the other --cxx tests.
  • dlss_nr_composite_comp_spv.h isn't covered by the shader-to-header check in tests/test_dlss_actor.py, which covers the other dlss_*.comp files. Worth adding it to that list.
  • Nit: dlss5_debug is saved to the ini. If someone leaves "Device depth" or "Motion vectors" selected, the next launch starts with SR bypassed and the debug buffer on screen. I'd keep the debug view session-only.
  • #1008: dlss_mode() only returns 1/2, so FSR values on the dlss key don't collide with the NR gate, and the dlss5* ini keys are separate. Both PRs edit vk_dlss.cpp/vk_menu.cpp heavily, though, so whichever lands second needs a careful rebase. main has also moved in vk_dlss.cpp, vk_menu.cpp and vk_main.cpp since your base.
  • CI hasn't run on this PR (no checks reported).
Thanks for this, it's a lot of careful work: the candidate/rollback around feature creation, the deferred release through `later()`, the coalesced edits and the non-blocking timestamps all look right to me. I'm leaving COMMENT because a few things in the render path change how DLSS SR behaves even for people who never get NR running. What "Neural Rendering" means here should be spelled out for maintainers. This is NGX feature 18, which the public SDK still calls `Reserved18`. The `DLSSNR.*` parameter names come from community documentation, not from NVIDIA headers, and it needs a user-supplied `nvngx_dlssnr.dll`. On Linux there's no known native runtime, so that path only probes. The code gates all of this properly at runtime, and the no-SDK stubs look complete. Still, it's an undocumented contract that a driver update can change under us, so whether to ship it outside an env opt-in is a maintainer call. +1 to the earlier comment about making this a sub-menu under Graphics. Two more data points: - The DLSS 5 tab is shown in every build, including ones without `HOOKSHOT_NGX` and on AMD/Intel. `row_shown()` has no `dlss_built()` check for `TAB_DLSS5`, while the Upscaler row does hide via `ur::upscaler_shown(dlss_built(), ...)`. So most users get a tab of ~19 rows that can only say "not built". - There's already a mechanism for this: the Controls tab's Motion row opens its own page (`TAB_MOTION`). A "DLSS 5" row under the Upscaler, shown only when DLSS is built, could open the same way. That would also drop the `TAB_COUNT` churn in test_audio_output_menu / test_framegen_preset / test_rumble_menu / test_stick_dead_zone. Other points: - Tests: the four new `test_dlss_nr_*.py` default to `--cxx g++` but aren't in `ARGS` in tests/run_all.py. run_all only checks for the compiler when it's named there, so a runner without g++ fails these instead of skipping them. Please add them like the other `--cxx` tests. - `dlss_nr_composite_comp_spv.h` isn't covered by the shader-to-header check in tests/test_dlss_actor.py, which covers the other dlss_*.comp files. Worth adding it to that list. - Nit: `dlss5_debug` is saved to the ini. If someone leaves "Device depth" or "Motion vectors" selected, the next launch starts with SR bypassed and the debug buffer on screen. I'd keep the debug view session-only. - #1008: `dlss_mode()` only returns 1/2, so FSR values on the `dlss` key don't collide with the NR gate, and the `dlss5*` ini keys are separate. Both PRs edit vk_dlss.cpp/vk_menu.cpp heavily, though, so whichever lands second needs a careful rebase. main has also moved in vk_dlss.cpp, vk_menu.cpp and vk_main.cpp since your base. - CI hasn't run on this PR (no checks reported).
@ -105,0 +325,4 @@
full_barrier(cmd);
}
if (NVSDK_NGX_FAILED(r) || !candidate) {
temporal_later([candidate, candidate_params] {
hckrman101 (Migrated from github.com) commented 2026-11-10 16:10:41 +00:00

This rebuilds the NR feature, and reallocates nr_output/nr_composed, whenever the render size changes. That bypasses the 32-frame / 2 s rate limit that rebuild has. With Resolution Auto, rw/rh (in->width/height) move with the game's dynamic resolution, so this can be a CreateFeature plus two full_barriers on every size step. If any one of those creations fails, refused() sets nr_failed and NR stays off until Retry.

SR avoids this with a size range: make_feature keeps the feature while min_rw <= rw <= feature_rw, sizes s.color at feature_rw/rh, and passes the real size as a subrect. You already set the DLSSNR.*Subrect* keys, so creating NR at feature_rw/feature_rh (and the images at that size) and passing rw/rh as the subrect should give the same behaviour, assuming the runtime honours subrects.

This rebuilds the NR feature, and reallocates `nr_output`/`nr_composed`, whenever the render size changes. That bypasses the 32-frame / 2 s rate limit that `rebuild` has. With Resolution Auto, `rw`/`rh` (`in->width/height`) move with the game's dynamic resolution, so this can be a CreateFeature plus two `full_barrier`s on every size step. If any one of those creations fails, `refused()` sets `nr_failed` and NR stays off until Retry. SR avoids this with a size range: `make_feature` keeps the feature while `min_rw <= rw <= feature_rw`, sizes `s.color` at `feature_rw/rh`, and passes the real size as a subrect. You already set the `DLSSNR.*Subrect*` keys, so creating NR at `feature_rw/feature_rh` (and the images at that size) and passing `rw/rh` as the subrect should give the same behaviour, assuming the runtime honours subrects.
hckrman101 (Migrated from github.com) commented 2026-11-10 16:10:42 +00:00

While NR is requested and HDR is Native, this branch runs every frame and sets reset = true every frame. That reaches ep.InReset for DLSS SR, so SR's temporal history is wiped on every frame: no accumulation, worse AA, more shimmer. Someone who saved NR On and later switches to Native HDR silently gets a degraded DLSS. Only reset on the transition into this state, e.g. when s.nr_running was true, then clear it.

While NR is requested and HDR is Native, this branch runs every frame and sets `reset = true` every frame. That reaches `ep.InReset` for DLSS SR, so SR's temporal history is wiped on every frame: no accumulation, worse AA, more shimmer. Someone who saved NR On and later switches to Native HDR silently gets a degraded DLSS. Only reset on the transition into this state, e.g. when `s.nr_running` was true, then clear it.
hckrman101 (Migrated from github.com) commented 2026-11-10 16:10:42 +00:00

Same pattern at a lower rate. While NR is requested but blocked (missing DLL, Linux not opted in, no DLSS runtime, ...), this check fails once a second and sets reset = true, so SR's history is reset every second: a visible pulse once a second for anyone with NR saved On but unusable. drop_nr() is also re-run each time. Reset only when NR was actually running (s.nr_running) or the issue changed.

Same pattern at a lower rate. While NR is requested but blocked (missing DLL, Linux not opted in, no DLSS runtime, ...), this check fails once a second and sets `reset = true`, so SR's history is reset every second: a visible pulse once a second for anyone with NR saved On but unusable. `drop_nr()` is also re-run each time. Reset only when NR was actually running (`s.nr_running`) or the issue changed.
hckrman101 (Migrated from github.com) commented 2026-11-10 16:10:42 +00:00

When NR is switched off (and saved Off), the NR feature, its params and nr_output/nr_composed stay allocated for as long as DLSS SR is in use. dlss_idle only frees them after SR itself goes unused for 120 frames. That's fine for Compare A/B, but outside Compare it means the NR model's VRAM is held until restart. Could you release it after a short time off when !menu_comparing()?

When NR is switched off (and saved Off), the NR feature, its params and `nr_output`/`nr_composed` stay allocated for as long as DLSS SR is in use. `dlss_idle` only frees them after SR itself goes unused for 120 frames. That's fine for Compare A/B, but outside Compare it means the NR model's VRAM is held until restart. Could you release it after a short time off when `!menu_comparing()`?
hckrman101 (Migrated from github.com) commented 2026-11-10 16:10:42 +00:00

Question about the inputs. s.color is the jittered render (SR gets ep.InJitterOffsetX/Y), but NR gets no jitter, exposure or camera data here, and the motion has the jitter removed. If NR keeps temporal history (DLSSNR.Reset suggests it does), feeding it sub-pixel-jittered frames without the offsets is a likely source of the flicker the history help text mentions. Does the community contract have jitter keys? If not, it may be worth trying NR on an unjittered input.

Question about the inputs. `s.color` is the jittered render (SR gets `ep.InJitterOffsetX/Y`), but NR gets no jitter, exposure or camera data here, and the motion has the jitter removed. If NR keeps temporal history (`DLSSNR.Reset` suggests it does), feeding it sub-pixel-jittered frames without the offsets is a likely source of the flicker the history help text mentions. Does the community contract have jitter keys? If not, it may be worth trying NR on an unjittered input.
hckrman101 (Migrated from github.com) commented 2026-11-10 16:10:42 +00:00

Nit: a fault-injection env var on the production render path. I'd keep this out of the shipped build (e.g. behind a debug define), or at least note it in the docs.

Nit: a fault-injection env var on the production render path. I'd keep this out of the shipped build (e.g. behind a debug define), or at least note it in the docs.
hckrman101 (Migrated from github.com) commented 2026-11-10 16:10:42 +00:00

Heads-up for #1473 (recorder thread). NGX records into cmd through its own function pointers, so that PR adds rec::boundary(rec::B_DIRECT) before every NGX create/evaluate in this file. This CreateFeature(18) and the EvaluateFeature below (line 526) will need the same, or whatever is queued on the recorder lands after NGX's commands. Whichever PR lands second should add them.

Heads-up for #1473 (recorder thread). NGX records into `cmd` through its own function pointers, so that PR adds `rec::boundary(rec::B_DIRECT)` before every NGX create/evaluate in this file. This `CreateFeature(18)` and the `EvaluateFeature` below (line 526) will need the same, or whatever is queued on the recorder lands after NGX's commands. Whichever PR lands second should add them.
HypeZ85 commented 2026-11-10 22:28:35 +00:00 (Migrated from github.com)

@hckrman101 i merged with the new main and fixed what you requested. I also saw the mention in the #1833 and the fact that claude prefers that PR to this, feel free to choose what is better!

@hckrman101 i merged with the new main and fixed what you requested. I also saw the mention in the #1833 and the fact that claude prefers that PR to this, feel free to choose what is better!
hckrman101 commented 2026-11-11 15:51:02 +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!
HypeZ85 commented 2026-11-11 19:08:38 +00:00 (Migrated from github.com)

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

@hckrman101 done! i removed the my fps stats addition since it was extended by other PRs

> Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks! @hckrman101 done! i removed the my fps stats addition since it was extended by other PRs
hckrman101 (Migrated from github.com) reviewed 2026-11-11 19:53:13 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the quick turnaround. I went through the new head against my last review (3c7b713) patch by patch.

Fixed from last time

  • NR is now a "DLSS 5" row under Upscaler that opens its own page (TAB_DLSS5, like Motion), hidden when !dlss_built(). The TAB_COUNT churn in the four menu tests is gone.
  • Dynamic resolution: NR is created at feature_rw/feature_rh and the real size goes in as the subrect, so a resolution step no longer recreates the feature. The subrect probe in the description is a good check of that.
  • Native HDR and blocked NR now only reset SR when NR was actually running (reset |= s.nr_running). No more reset every frame or once a second.
  • Off releases NR's feature and images after 2 s outside Compare.
  • rec::boundary(B_DIRECT) comes before NR's CreateFeature/EvaluateFeature, and test_recorder_sync now covers both calls.
  • The fault injection is behind HOOKSHOT_NR_TESTING. The debug view is session-only (not saved or loaded).
  • The test_dlss_nr_*.py tests declare RUN_ALL_ARGS, and dlss_nr_composite.comp is in test_dlss_actor's shader list. I regenerated the header with glslang and it matches the source.
  • Jitter question: answered. There are no known NR jitter keys, so NR history is reset while SR jitters, and the docs and the Temporal history hint say so. That's a reasonable conservative choice.

New

  • The last merge mangled three non-ASCII characters into mojibake (inline comments): the Inventory page's range text, the teleport list's separator, and a comment in vk_dlss.cpp. The first two show up on screen. This is the only thing I'd want fixed before merging.
  • Nit: RX_VK_DLSS_START (setting the Upscaler to DLSS) is a new switch that isn't related to NR. RX_VK_DLSS_NR, _NR_DEFAULT and _NR_INTENSITY aren't documented either. I'd drop the first and add the others to docs/dlss5-neural-rendering.md.
  • Nit: prepare/evaluate now hold State::mu for the whole DLSS pass, including NR's CreateFeature. The menu's once-a-second dlss_nr_check() takes the same lock on the present thread, so a rebuild can stall the menu for a frame. A try_lock in that periodic check would avoid it. Lock order with #2118's frame generation (g.mu -> st().mu via ngx_init) is fine: nothing takes g.mu under st().mu.
  • Nit: with an NGX build on AMD/Intel the DLSS 5 row still shows. It just explains why it can't run, so that's fine either way.

Composite pass: the sync and memory look right to me. The new images go UNDEFINED -> GENERAL once in the same command buffer, there are full barriers around the dispatch and NGX calls, destruction goes through temporal_later, the storage format matches the RGBA16F outputs, and the push constant block is 24 bytes on both sides. No new settings option ids, no vk_api.h change. The touched menu tests' stubs match the new source. It merges cleanly with current main, which already includes #2118.

CI: the build and test workflows on this head are waiting for maintainer approval ("action_required"). Only the title check has run.

On the bigger picture, this now avoids the SR-history and VRAM issues I raised when comparing it with #1833, so the two are closer than when I suggested #1833 as the base. Which of them to take, and whether an undocumented feature-18 path ships without an env gate, is still @NotRexed's call. I'm leaving this as a comment for that reason and for the mojibake fix.

Thanks for the quick turnaround. I went through the new head against my last review (3c7b713) patch by patch. **Fixed from last time** - NR is now a "DLSS 5" row under Upscaler that opens its own page (`TAB_DLSS5`, like Motion), hidden when `!dlss_built()`. The `TAB_COUNT` churn in the four menu tests is gone. - Dynamic resolution: NR is created at `feature_rw/feature_rh` and the real size goes in as the subrect, so a resolution step no longer recreates the feature. The subrect probe in the description is a good check of that. - Native HDR and blocked NR now only reset SR when NR was actually running (`reset |= s.nr_running`). No more reset every frame or once a second. - Off releases NR's feature and images after 2 s outside Compare. - `rec::boundary(B_DIRECT)` comes before NR's CreateFeature/EvaluateFeature, and test_recorder_sync now covers both calls. - The fault injection is behind `HOOKSHOT_NR_TESTING`. The debug view is session-only (not saved or loaded). - The `test_dlss_nr_*.py` tests declare `RUN_ALL_ARGS`, and `dlss_nr_composite.comp` is in test_dlss_actor's shader list. I regenerated the header with glslang and it matches the source. - Jitter question: answered. There are no known NR jitter keys, so NR history is reset while SR jitters, and the docs and the Temporal history hint say so. That's a reasonable conservative choice. **New** - The last merge mangled three non-ASCII characters into mojibake (inline comments): the Inventory page's range text, the teleport list's separator, and a comment in vk_dlss.cpp. The first two show up on screen. This is the only thing I'd want fixed before merging. - Nit: `RX_VK_DLSS_START` (setting the Upscaler to DLSS) is a new switch that isn't related to NR. `RX_VK_DLSS_NR`, `_NR_DEFAULT` and `_NR_INTENSITY` aren't documented either. I'd drop the first and add the others to docs/dlss5-neural-rendering.md. - Nit: `prepare`/`evaluate` now hold `State::mu` for the whole DLSS pass, including NR's CreateFeature. The menu's once-a-second `dlss_nr_check()` takes the same lock on the present thread, so a rebuild can stall the menu for a frame. A `try_lock` in that periodic check would avoid it. Lock order with #2118's frame generation (`g.mu` -> `st().mu` via `ngx_init`) is fine: nothing takes `g.mu` under `st().mu`. - Nit: with an NGX build on AMD/Intel the DLSS 5 row still shows. It just explains why it can't run, so that's fine either way. Composite pass: the sync and memory look right to me. The new images go UNDEFINED -> GENERAL once in the same command buffer, there are full barriers around the dispatch and NGX calls, destruction goes through `temporal_later`, the storage format matches the RGBA16F outputs, and the push constant block is 24 bytes on both sides. No new settings option ids, no `vk_api.h` change. The touched menu tests' stubs match the new source. It merges cleanly with current main, which already includes #2118. CI: the build and test workflows on this head are waiting for maintainer approval ("action_required"). Only the title check has run. On the bigger picture, this now avoids the SR-history and VRAM issues I raised when comparing it with #1833, so the two are closer than when I suggested #1833 as the base. Which of them to take, and whether an undocumented feature-18 path ships without an env gate, is still @NotRexed's call. I'm leaving this as a comment for that reason and for the mojibake fix.
hckrman101 (Migrated from github.com) commented 2026-11-11 19:53:13 +00:00

This was an en dash (%d–%d of %d) on main. The merge turned it into mojibake, so the Inventory page now shows – between the numbers. Same thing at the teleport · below, and in the "Café" comment in vk_dlss.cpp (around line 454). Restoring those three lines from main fixes it. Your editor or merge tool probably re-read the UTF-8 file as Windows-1252, so it's worth checking it's set to UTF-8.

This was an en dash (`%d–%d of %d`) on main. The merge turned it into mojibake, so the Inventory page now shows `–` between the numbers. Same thing at the teleport `·` below, and in the `"Café"` comment in vk_dlss.cpp (around line 454). Restoring those three lines from main fixes it. Your editor or merge tool probably re-read the UTF-8 file as Windows-1252, so it's worth checking it's set to UTF-8.
@ -2523,3 +2787,3 @@
ui::text_mid(c, c.f.heading, 23 * s, x + 28 * s, ty, shown ? ui::kInkBright : ui::kBannerInk, tab_titles[i]);
ui::text_mid(c, c.f.heading, 23 * ts, x + 28 * ts, ty, shown ? ui::kInkBright : ui::kBannerInk, tab_titles[i]);
hits.add(x, 0, x + tw[i], hh, menu_hit::kTab, i);
x += tw[i];
hckrman101 (Migrated from github.com) commented 2026-11-11 19:53:13 +00:00

Same mojibake: this was "%s · %s" on main. The teleport list now shows ·.

Same mojibake: this was `"%s · %s"` on main. The teleport list now shows `·`.
hckrman101 (Migrated from github.com) commented 2026-11-11 19:53:13 +00:00

Nit: this sets the Upscaler to DLSS from a new RX_VK_DLSS_START switch, which doesn't seem related to NR (a test helper?). I'd drop it, or document it with the other RX_VK_DLSS_NR* switches above.

Nit: this sets the Upscaler to DLSS from a new `RX_VK_DLSS_START` switch, which doesn't seem related to NR (a test helper?). I'd drop it, or document it with the other `RX_VK_DLSS_NR*` switches above.
hckrman101 commented 2026-11-11 19:55:35 +00:00 (Migrated from github.com)

Can you update screenshots too please?

Can you update screenshots too please?
HypeZ85 commented 2026-11-11 20:20:36 +00:00 (Migrated from github.com)

Can you update screenshots too please?

Ok thanks for the review @hckrman101 , i updated the code.
I also merged with the new main and solved the new conflicts!

Here the new screenshots:

Screenshot (4) Screenshot (8) Screenshot (6) Screenshot (9) Screenshot (10)
> Can you update screenshots too please? Ok thanks for the review @hckrman101 , i updated the code. I also merged with the new main and solved the new conflicts! Here the new screenshots: <img width="3440" height="1440" alt="Screenshot (4)" src="https://github.com/user-attachments/assets/af3960cd-2200-477e-96a1-e48398fa9cb4" /> <img width="3440" height="1440" alt="Screenshot (8)" src="https://github.com/user-attachments/assets/d3f83b78-7fd2-42a4-9e51-0cfc2ab6b234" /> <img width="3440" height="1440" alt="Screenshot (6)" src="https://github.com/user-attachments/assets/d91e55e9-82f9-493e-8216-37458294726f" /> <img width="3440" height="1440" alt="Screenshot (9)" src="https://github.com/user-attachments/assets/d0d3428a-a6db-4087-a2aa-e9d60d576a22" /> <img width="3440" height="1440" alt="Screenshot (10)" src="https://github.com/user-attachments/assets/8e3eb8af-aa11-4646-838b-616a6cb8a08f" />
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 21:46:01 +00:00
Joey-Magnus (Migrated from github.com) left a comment

I checked the head against the open points from the last review. The three mojibake strings are gone, RX_VK_DLSS_START is removed, and dlss_nr_check() now uses try_lock (vk_dlss.cpp:961). Nothing else on main uses F8.

One new problem. The text merges cleanly with current main, but tests/test_menu_keys.py then fails with ValueError: substring not found at line 249. That check came in with #2197 (merged 16:12 today) and cuts menu_tick at if (open_now.load() && page == 5) {. This PR turns that line into } else if (...) (vk_menu.cpp:3960) and puts the NR Compare branch in front of it. The test passes on this head alone and on main alone. Please merge main and update the test's anchor and stubs for the new branch, rather than reshaping the code to fit the test. The other dlss/menu/recorder/fps/rumble/stick/framegen checks pass on the merge.

The bigger call is still the maintainer's: this or #1833 (+49.6k lines, also open and mergeable). There's also whether an undocumented NGX feature-18 path ships with only the menu opt-in.

I checked the head against the open points from the last review. The three mojibake strings are gone, `RX_VK_DLSS_START` is removed, and `dlss_nr_check()` now uses `try_lock` (vk_dlss.cpp:961). Nothing else on main uses F8. One new problem. The text merges cleanly with current main, but `tests/test_menu_keys.py` then fails with `ValueError: substring not found` at line 249. That check came in with #2197 (merged 16:12 today) and cuts `menu_tick` at ` if (open_now.load() && page == 5) {`. This PR turns that line into `} else if (...)` (vk_menu.cpp:3960) and puts the NR Compare branch in front of it. The test passes on this head alone and on main alone. Please merge main and update the test's anchor and stubs for the new branch, rather than reshaping the code to fit the test. The other dlss/menu/recorder/fps/rumble/stick/framegen checks pass on the merge. The bigger call is still the maintainer's: this or #1833 (+49.6k lines, also open and mergeable). There's also whether an undocumented NGX feature-18 path ships with only the menu opt-in.
hckrman101 commented 2026-11-11 21:59:19 +00:00 (Migrated from github.com)

Fix the comment above, but also can we maybe rename 'DLSS 5' to something a bit more descriptive? eg 'DLSS 5 Neural Rendering'. also the row should be under the upscaler model

Fix the comment above, but also can we maybe rename 'DLSS 5' to something a bit more descriptive? eg 'DLSS 5 Neural Rendering'. also the row should be under the upscaler model
hckrman101 commented 2026-11-11 22:55:33 +00:00 (Migrated from github.com)

This now conflicts with main in gpu/vk/vk_menu.cpp, gpu/vk/vk_menu.h, tests/test_audio_output_menu.py, tests/test_stick_dead_zone.py. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.

This now conflicts with main in `gpu/vk/vk_menu.cpp`, `gpu/vk/vk_menu.h`, `tests/test_audio_output_menu.py`, `tests/test_stick_dead_zone.py`. Could you merge main into the branch (or rebase) when you get a chance? I'll take another look once it's up to date.
HypeZ85 commented 2026-11-12 00:00:40 +00:00 (Migrated from github.com)

@hckrman101 requested edits done, merged with all the new commits on main, solved the conflits, updated the screenshots above

@hckrman101 requested edits done, merged with all the new commits on main, solved the conflits, updated the screenshots above
Owner

@HypeZ85 commented on GitHub · 2026-11-12 10:37 UTC · original

The Compare menu is now extended to all the DLSS 5 features using the "advanced" option.
I also added all the localization needed for the 13 supported languages

> [@HypeZ85](https://github.com/HypeZ85) commented on GitHub · 2026-11-12 10:37 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1590#issuecomment-6821309737) The Compare menu is now extended to all the DLSS 5 features using the "advanced" option. I also added all the localization needed for the 13 supported languages <!-- github-sync:c:6821309737 -->
Owner

@Joey-Magnus reviewed on GitHub · 2026-11-12 14:32 UTC · original

The test_menu_keys anchor problem is fixed. I merged b5a7474 into current main (clean) and ran the dlss, menu, recorder, fps_overlay, rumble, stick, framegen, audio_output, env_on, utf8 and prompt_skip checks: 49 passed, and 3 were skipped because this PC lacks the Vulkan headers and dotnet. hckrman101's mojibake, RX_VK_DLSS_START and try_lock points are handled too. CI's build workflows are still waiting for approval.

What's left isn't code. #1833 was rebuilt today on current main as a standalone NGX feature-18 path that runs before the HUD, and its CI is green. So the two are now real alternatives, and only one of them can land. This one has the Graphics submenu hckrman101 asked for, hides on builds without NGX, has its strings in all 13 languages, and has Compare. #1833 runs inside the temporal chain, before the HUD. The other open question for @NotRexed is whether an undocumented feature-18 path ships with nothing but a menu opt-in.

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 14:32 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1590#pullrequestreview-5876199502) The test_menu_keys anchor problem is fixed. I merged b5a7474 into current main (clean) and ran the dlss, menu, recorder, fps_overlay, rumble, stick, framegen, audio_output, env_on, utf8 and prompt_skip checks: 49 passed, and 3 were skipped because this PC lacks the Vulkan headers and dotnet. hckrman101's mojibake, RX_VK_DLSS_START and try_lock points are handled too. CI's build workflows are still waiting for approval. What's left isn't code. #1833 was rebuilt today on current main as a standalone NGX feature-18 path that runs before the HUD, and its CI is green. So the two are now real alternatives, and only one of them can land. This one has the Graphics submenu hckrman101 asked for, hides on builds without NGX, has its strings in all 13 languages, and has Compare. #1833 runs inside the temporal chain, before the HUD. The other open question for @NotRexed is whether an undocumented feature-18 path ships with nothing but a menu opt-in. <!-- github-sync:r:5876199502 -->
Commenting is not possible because the repository is archived.
No description provided.