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

fix(vk): DLSS comes back after a feature that was not made, and frame generation takes the multiplier its sets were made for #2658

Closed
notquiteog wants to merge 1 commit from notquiteog/fix/dlss-retry-and-framegen-multiplier into main
pull from: notquiteog/fix/dlss-retry-and-framegen-multiplier
merge into: MagicBeanDev:main
Conversation 6 Commits 1 Files changed 5 +35 -6
notquiteog commented 2026-11-12 04:33:48 +00:00 (Migrated from github.com)

After one failed DLSS feature creation, DLSS stays off until the player changes the DLSS model or the output size; and stepping the Frame generation multiplier up in the menu can make a frame be copied into an image that does not exist. Both are in code that #2550 and #2574 touched, found by reading the merged result.

What changed

Frame generation: the multiplier is read once (vk_framegen.cpp). framegen_record called multiplier() again after ensure_size had made the sets for fg.mult. The menu runs on the display thread (menu_tick from present_image), the record on the replay thread, with a wait for a free set in between. A step from 2x to 3x in that window gives m = fg.mult + 1, and the last vkCmdCopyImage targets set->frame[fg.mult], a null image, which the display thread then presents. It takes fg.mult now, and reads the Auto test once for the same reason.

DLSS: refused is cleared where a retry should happen (vk_dlss.cpp). Since #2550 a creation that fails returns 0 with s.refused = key, and the key is the output size, preset and HDR. The old feature is already let go, so from then on:

  • the Render resolution row does not retry (same output, same key);
  • DLSS to DLAA does not retry (DLAA's output is the display's, the same key);
  • the Upscaler away and back does not retry (release() left refused set).

#2574's temporal_reset_failure does not help there: the chain's failed flag is not what holds DLSS off. Now the quality mode is part of the key, release() clears refused, and temporal_reset_failure clears it too (dlss_retry, called with the chain's lock held, as prepare has it).

DLSS: a failing evaluation no longer logs on every frame (vk_dlss.cpp). With retry = true (#2574) an evaluation that fails every frame wrote its evaluation failed 60 times a second for the rest of the run. The line is spaced like the chain's own reason (when the result changes, and every 600th time), and after 30 failures in a row the feature is let go once so prepare makes a new one.

How it was tested

  • tests/run_all.py: with #2656 (which makes main's checks pass) and this together, 468 passed, 0 failed, 22 skipped. On this branch alone the 15 checks that are red on main stay red and nothing else fails. test_present_source_crop's fixture sets fg.mult in its ensure_size stand-in, as the real one does.
  • In the game, with this branch and #2656 built into the Vulkan back end: RTX 5060 Ti 16 GB, driver 615.71.09, Linux (COSMIC, Wayland), Kokiri Forest, 2048x1152 output, V-Sync Off.
    • DLSS at Ultra Performance, Performance and Auto: 58 to 60 fps, one feature made per range change.
    • DLSS Frame Generation at 2x and 3x: 598 to 600 of each 600 presents made by DLSS-G.
    • 28 quick steps of the multiplier through 2x to 8x from the control bridge, then 3x: no device lost, no error.
    • Upscaler to AMD FSR 3.1 and back: feature 4 made (again after it was let go ...).

Not tested

  • A real failed feature creation or failed evaluation: neither could be provoked on this card (16 GB), so the retry and the spaced log line are checked by reading, not by a run.
  • The race itself is a timing window; the run shows the fix does no harm under quick steps, not that the old code crashed here.
  • Windows, AMD and Intel.

Review notes that are not fixed here, for whoever owns them: FSR 4's engine still has the permanent latch #2574 removed for DLSS (vk_fsr4.cpp prepare returns -1, no retry); with Native HDR, frame generation's images are RGBA8 (vk_framegen.cpp ensure_size), so the game's 10-bit PQ picture passes through 8 bits; DLSS-G's feature keeps the render size it was created with when the Render resolution changes.

🤖 Generated with Claude Code

After one failed DLSS feature creation, DLSS stays off until the player changes the DLSS model or the output size; and stepping the Frame generation multiplier up in the menu can make a frame be copied into an image that does not exist. Both are in code that #2550 and #2574 touched, found by reading the merged result. ## What changed **Frame generation: the multiplier is read once** (`vk_framegen.cpp`). `framegen_record` called `multiplier()` again after `ensure_size` had made the sets for `fg.mult`. The menu runs on the display thread (`menu_tick` from `present_image`), the record on the replay thread, with a wait for a free set in between. A step from 2x to 3x in that window gives `m = fg.mult + 1`, and the last `vkCmdCopyImage` targets `set->frame[fg.mult]`, a null image, which the display thread then presents. It takes `fg.mult` now, and reads the Auto test once for the same reason. **DLSS: `refused` is cleared where a retry should happen** (`vk_dlss.cpp`). Since #2550 a creation that fails returns 0 with `s.refused = key`, and the key is the output size, preset and HDR. The old feature is already let go, so from then on: - the Render resolution row does not retry (same output, same key); - DLSS to DLAA does not retry (DLAA's output is the display's, the same key); - the Upscaler away and back does not retry (`release()` left `refused` set). #2574's `temporal_reset_failure` does not help there: the chain's `failed` flag is not what holds DLSS off. Now the quality mode is part of the key, `release()` clears `refused`, and `temporal_reset_failure` clears it too (`dlss_retry`, called with the chain's lock held, as `prepare` has it). **DLSS: a failing evaluation no longer logs on every frame** (`vk_dlss.cpp`). With `retry = true` (#2574) an evaluation that fails every frame wrote `its evaluation failed` 60 times a second for the rest of the run. The line is spaced like the chain's own reason (when the result changes, and every 600th time), and after 30 failures in a row the feature is let go once so `prepare` makes a new one. ## How it was tested - `tests/run_all.py`: with #2656 (which makes main's checks pass) and this together, **468 passed, 0 failed, 22 skipped**. On this branch alone the 15 checks that are red on main stay red and nothing else fails. `test_present_source_crop`'s fixture sets `fg.mult` in its `ensure_size` stand-in, as the real one does. - In the game, with this branch and #2656 built into the Vulkan back end: RTX 5060 Ti 16 GB, driver 615.71.09, Linux (COSMIC, Wayland), Kokiri Forest, 2048x1152 output, V-Sync Off. - DLSS at Ultra Performance, Performance and Auto: 58 to 60 fps, one feature made per range change. - DLSS Frame Generation at 2x and 3x: 598 to 600 of each 600 presents made by DLSS-G. - 28 quick steps of the multiplier through 2x to 8x from the control bridge, then 3x: no device lost, no error. - Upscaler to AMD FSR 3.1 and back: `feature 4 made (again after it was let go ...)`. ## Not tested - A real failed feature creation or failed evaluation: neither could be provoked on this card (16 GB), so the retry and the spaced log line are checked by reading, not by a run. - The race itself is a timing window; the run shows the fix does no harm under quick steps, not that the old code crashed here. - Windows, AMD and Intel. Review notes that are not fixed here, for whoever owns them: FSR 4's engine still has the permanent latch #2574 removed for DLSS (`vk_fsr4.cpp` `prepare` returns -1, no `retry`); with Native HDR, frame generation's images are RGBA8 (`vk_framegen.cpp` `ensure_size`), so the game's 10-bit PQ picture passes through 8 bits; DLSS-G's feature keeps the render size it was created with when the Render resolution changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
github-sync commented 2026-11-12 12:37:23 +00:00
Owner

@Jiro131 commented on GitHub · 2026-11-12 12:13 UTC · original

A Windows test of the frame-generation race in the first part of this PR. Current main crashed twice here, and this PR did not crash under the same multiplier steps.

  • Hardware: RTX 4090, NVIDIA 621.01, Windows 11, i9-13900K, 144 Hz display.
  • Builds, both from source with NGX (llvm-mingw, SDK 310.9.16), on the same base:
    • main: hookshot v2.1.0-rc.3-256-g51da0464 (source 18ed87b5);
    • main + this PR (343bee04 cherry-picked): hookshot v2.1.0-rc.3-257-g204757a2 (source 39f17cc9).
  • Settings, the same options file for both: DLSS DLAA (preset K) at 2560x1440, Frame generation by Auto, Frame pacing on, cap 60, V-Sync Auto.
  • The RTX 4090's DLSS-G runtime makes 1 frame, so at 3x and 4x the renderer's own generator makes the frames.

main, two crashes, both the same:

[hos Presentation Thread] CRASH: exception 0xc0000005 (read 0x590, guest 0xfffff00000000590) at nvoglv64.dll+0xea4c1a
[hos Presentation Thread] CRASH: host backtrace: nvoglv64.dll+0xea4c1a nvoglv64.dll+0xe99c39 nvoglv64.dll+0xe35298 hookshot_vk.dll+0x271ef1 hookshot_vk.dll+0x346225 hookshot_vk.dll+0x32f32e ...

The three hookshot_vk.dll frames, symbolized against that build's DLL with llvm-symbolizer, are mkwvk::framegen_record, called from do_present, called from present. The driver reads at 0x590 from a null base while it records framegen_record's commands.

  1. In a window, Reflex off, stepping the multiplier up in the menu from 2x to 3x, then 3x to 4x. The crash came at the step to 4x, and the swapchain was being remade for it at the same moment: 
    [hos host] vk: surface caps images 2..8 extent 2558x1360..2558x1360 usage 0x9f alpha 0x1 transform 0x1
[hos Presentation Thread] CRASH: exception 0xc0000005 (read 0x590, ...) at nvoglv64.dll+0xea4c1a
[hos host] vk: V-Sync Auto with frame generation at 4x, its frames the renderer's own (DLSS-G does not make them): Auto's mode, as without DLSS-G
[hos host] vk: swapchain 2558x1360, 6 images, mailbox (V-Sync Auto)
  2. At 4x all along, no multiplier change, Reflex on. The game started fullscreen (5120x1440) and went to a window before play (2 draws a frame). The swapchain was remade twice at 1280x720, then the same crash came.

With this PR, no crash: three rounds of 2x → 3x → 4x and back in the menu, in a window: 11 swapchain remakes (4, 5 and 6 images), the game ran on and exited normally.

This PR also held up for the second case. At 4x before play, the game went from fullscreen to a window and back 25 times each (display: fullscreen / display: windowed), with 62 swapchain remakes between 5120x1440 and 1280x720. There was no crash, and the game exited normally. On main, one such switch had been enough.

These are small counts: one crash per case on main, and a timing window. Still, both cases end in the same driver read inside framegen_record, and neither came back with this PR.

Not tested: Linux, AMD and Intel; the DLSS retry part of this PR (no failed feature creation here).

🤖 Generated with Claude Code

> [@Jiro131](https://github.com/Jiro131) commented on GitHub · 2026-11-12 12:13 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2658#issuecomment-6823287357) A Windows test of the frame-generation race in the first part of this PR. Current `main` crashed twice here, and this PR did not crash under the same multiplier steps. - Hardware: RTX 4090, NVIDIA 621.01, Windows 11, i9-13900K, 144 Hz display. - Builds, both from source with NGX (llvm-mingw, SDK 310.9.16), on the same base: - `main`: `hookshot v2.1.0-rc.3-256-g51da0464` (source 18ed87b5); - `main` + this PR (343bee04 cherry-picked): `hookshot v2.1.0-rc.3-257-g204757a2` (source 39f17cc9). - Settings, the same options file for both: DLSS DLAA (preset K) at 2560x1440, Frame generation by Auto, Frame pacing on, cap 60, V-Sync Auto. - The RTX 4090's DLSS-G runtime makes 1 frame, so at 3x and 4x the renderer's own generator makes the frames. **`main`, two crashes, both the same:** ``` [hos Presentation Thread] CRASH: exception 0xc0000005 (read 0x590, guest 0xfffff00000000590) at nvoglv64.dll+0xea4c1a [hos Presentation Thread] CRASH: host backtrace: nvoglv64.dll+0xea4c1a nvoglv64.dll+0xe99c39 nvoglv64.dll+0xe35298 hookshot_vk.dll+0x271ef1 hookshot_vk.dll+0x346225 hookshot_vk.dll+0x32f32e ... ``` The three `hookshot_vk.dll` frames, symbolized against that build's DLL with `llvm-symbolizer`, are `mkwvk::framegen_record`, called from `do_present`, called from `present`. The driver reads at 0x590 from a null base while it records `framegen_record`'s commands. 1. In a window, Reflex off, stepping the multiplier up in the menu from 2x to 3x, then 3x to 4x. The crash came at the step to 4x, and the swapchain was being remade for it at the same moment: ``` [hos host] vk: surface caps images 2..8 extent 2558x1360..2558x1360 usage 0x9f alpha 0x1 transform 0x1 [hos Presentation Thread] CRASH: exception 0xc0000005 (read 0x590, ...) at nvoglv64.dll+0xea4c1a [hos host] vk: V-Sync Auto with frame generation at 4x, its frames the renderer's own (DLSS-G does not make them): Auto's mode, as without DLSS-G [hos host] vk: swapchain 2558x1360, 6 images, mailbox (V-Sync Auto) ``` 2. At 4x all along, no multiplier change, Reflex on. The game started fullscreen (5120x1440) and went to a window before play (2 draws a frame). The swapchain was remade twice at 1280x720, then the same crash came. **With this PR, no crash:** three rounds of 2x → 3x → 4x and back in the menu, in a window: 11 swapchain remakes (4, 5 and 6 images), the game ran on and exited normally. This PR also held up for the second case. At 4x before play, the game went from fullscreen to a window and back 25 times each (`display: fullscreen` / `display: windowed`), with 62 swapchain remakes between 5120x1440 and 1280x720. There was no crash, and the game exited normally. On `main`, one such switch had been enough. These are small counts: one crash per case on `main`, and a timing window. Still, both cases end in the same driver read inside `framegen_record`, and neither came back with this PR. Not tested: Linux, AMD and Intel; the DLSS retry part of this PR (no failed feature creation here). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- github-sync:c:6823287357 -->
github-sync commented 2026-11-12 12:37:23 +00:00
Owner

@polackangry commented on GitHub · 2026-11-12 12:28 UTC · original

The race in the first part happens in the game: a crash on Windows when stepping the multiplier up, at the copy this PR fixes.

  • main at 51da046 (v2.1.0-rc.3-260-g62c03b1), without this PR; Windows 11 Pro 25H2, RTX 5060 Ti 16 GB, driver 617.42; 3440x1440, DLAA, DLSS Frame Generation (nvngx_dlssg.dll 310.9.16 in ngx\), V-Sync Auto (FIFO).
  • Frame generation 2x → 3x → 4x worked (DLSS-G made 481 of 600 at 2x). 4x → 6x in the Insert menu: the game closed at once, before any swapchain ... 7 images line for 6x.
CRASH: exception 0xc0000005 (read 0x590, guest 0xfffff00000000590) at nvoglv64.dll+0xe80cba
CRASH: host backtrace: nvoglv64.dll+0xe80cba nvoglv64.dll+0xe760bd nvoglv64.dll+0xe13d68 hookshot_vk.dll+0x271ef1 hookshot_vk.dll+0x346225 hookshot_vk.dll+0x32f32e ...
CRASH: thread Presentation Thread

With the build's hookshot_vk.map: hookshot_vk.dll+0x271ef1 is mkwvk::framegen_record +0x1db1, the return from the last vkCmdCopyImage (set->frame[set->made].image, loaded from the set's frame array); +0x346225 is do_present, +0x32f32e present. So the destination was the null image of a set made for 4x (frame[0..3]) while m was already 6: the driver reads 0x590 past a null handle.

So it isn't only a window seen in reading: one step up was enough here.

🤖 Generated with Claude Code

> [@polackangry](https://github.com/polackangry) commented on GitHub · 2026-11-12 12:28 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2658#issuecomment-6823619313) The race in the first part happens in the game: a crash on Windows when stepping the multiplier up, at the copy this PR fixes. - main at 51da046 (`v2.1.0-rc.3-260-g62c03b1`), without this PR; Windows 11 Pro 25H2, RTX 5060 Ti 16 GB, driver 617.42; 3440x1440, DLAA, DLSS Frame Generation (`nvngx_dlssg.dll` 310.9.16 in `ngx\`), V-Sync Auto (FIFO). - Frame generation 2x → 3x → 4x worked (DLSS-G made 481 of 600 at 2x). 4x → 6x in the Insert menu: the game closed at once, before any `swapchain ... 7 images` line for 6x. ``` CRASH: exception 0xc0000005 (read 0x590, guest 0xfffff00000000590) at nvoglv64.dll+0xe80cba CRASH: host backtrace: nvoglv64.dll+0xe80cba nvoglv64.dll+0xe760bd nvoglv64.dll+0xe13d68 hookshot_vk.dll+0x271ef1 hookshot_vk.dll+0x346225 hookshot_vk.dll+0x32f32e ... CRASH: thread Presentation Thread ``` With the build's `hookshot_vk.map`: `hookshot_vk.dll+0x271ef1` is `mkwvk::framegen_record` +0x1db1, the return from the last `vkCmdCopyImage` (`set->frame[set->made].image`, loaded from the set's frame array); +0x346225 is `do_present`, +0x32f32e `present`. So the destination was the null image of a set made for 4x (frame[0..3]) while `m` was already 6: the driver reads 0x590 past a null handle. So it isn't only a window seen in reading: one step up was enough here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- github-sync:c:6823619313 -->
github-sync commented 2026-11-12 14:43:51 +00:00
Owner

@Joey-Magnus approved on GitHub · 2026-11-12 14:33 UTC · original

I merged this into current main and ran the DLSS/framegen/present/temporal/HDR checks (run_all.py dlss framegen present temporal hdr upscale: 26 passed, 4 skipped for Vulkan headers). The test_hdr_encode failure on this head was main's own (kEngine's retry, fixed by #2648), so a re-run should clear it.

The framegen fix is right and matters. ensure_size builds the sets for fg.mult, and framegen_record re-read multiplier() afterwards, so a menu step on the display thread in between made the last vkCmdCopyImage target a frame that was never created. Taking fg.mult at gpu/vk/vk_framegen.cpp:791 closes that. Jiro131 and polackangry both reproduced the nvoglv64.dll read at 0x590 in framegen_record on main, and neither saw it with this change.

On DLSS: adding the quality mode to the refused key, clearing refused in release() (vk_dlss.cpp:481) and calling dlss_retry from temporal_reset_failure look correct. The key's mode bits (8-10) don't overlap the preset bits. Note that the release after 30 failed evaluations (:469) fires only once per run of the same error code, which I think is what you want. I found no open PR touching these same lines (#2788 and #2665 change other parts of these files).

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2658#pullrequestreview-5876227473) I merged this into current main and ran the DLSS/framegen/present/temporal/HDR checks (`run_all.py dlss framegen present temporal hdr upscale`: 26 passed, 4 skipped for Vulkan headers). The test_hdr_encode failure on this head was main's own (kEngine's `retry`, fixed by #2648), so a re-run should clear it. The framegen fix is right and matters. `ensure_size` builds the sets for `fg.mult`, and `framegen_record` re-read `multiplier()` afterwards, so a menu step on the display thread in between made the last `vkCmdCopyImage` target a frame that was never created. Taking `fg.mult` at gpu/vk/vk_framegen.cpp:791 closes that. Jiro131 and polackangry both reproduced the `nvoglv64.dll` read at 0x590 in `framegen_record` on main, and neither saw it with this change. On DLSS: adding the quality mode to the refused key, clearing `refused` in `release()` (vk_dlss.cpp:481) and calling `dlss_retry` from `temporal_reset_failure` look correct. The key's mode bits (8-10) don't overlap the preset bits. Note that the release after 30 failed evaluations (:469) fires only once per run of the same error code, which I think is what you want. I found no open PR touching these same lines (#2788 and #2665 change other parts of these files). <!-- github-sync:r:5876227473 -->
github-sync closed this pull request 2026-11-12 16:55:31 +00:00
github-sync commented 2026-11-12 16:55:32 +00:00
Owner

@hckrman101 commented on gpu/vk/vk_dlss.cpp line 387 on GitHub · 2026-11-12 16:33 UTC · original

Nit, nothing to change unless you see it in practice: with the mode in the key, a refused feature is also retried whenever the game's dynamic resolution moves the render size into another mode's range. refused holds one key, so a size that goes back and forth across a range boundary tries again on each crossing. That's the same cost a working feature already pays (a new feature on each crossing), so I think it's fine. I'd keep the mode here, because it covers the case where the menu's dlss_retry is used up on a frame that still has the old render size.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_dlss.cpp` line 387 on GitHub · 2026-11-12 16:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2658#discussion_r4553289627) Nit, nothing to change unless you see it in practice: with the mode in the key, a refused feature is also retried whenever the game's dynamic resolution moves the render size into another mode's range. `refused` holds one key, so a size that goes back and forth across a range boundary tries again on each crossing. That's the same cost a working feature already pays (a new feature on each crossing), so I think it's fine. I'd keep the mode here, because it covers the case where the menu's `dlss_retry` is used up on a frame that still has the old render size. <!-- github-sync:rc:4553289627 -->
github-sync commented 2026-11-12 16:55:32 +00:00
Owner

@hckrman101 commented on gpu/vk/vk_dlss.cpp line 465 on GitHub · 2026-11-12 16:33 UTC · original

Nit: if two error codes alternate (A, B, A, B...), r != s.eval_result is true every frame and this logs every frame again, and the 30-in-a-row release never fires. That seems unlikely from NGX, so it's fine as it is. Keeping a separate eval_fails count that only a success resets would cover it, if you want to.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_dlss.cpp` line 465 on GitHub · 2026-11-12 16:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2658#discussion_r4553289643) Nit: if two error codes alternate (A, B, A, B...), `r != s.eval_result` is true every frame and this logs every frame again, and the 30-in-a-row release never fires. That seems unlikely from NGX, so it's fine as it is. Keeping a separate `eval_fails` count that only a success resets would cover it, if you want to. <!-- github-sync:rc:4553289643 -->
github-sync commented 2026-11-12 16:55:32 +00:00
Owner

@hckrman101 approved on GitHub · 2026-11-12 16:33 UTC · original

Thanks, this is a good one. The frame generation half fixes a real crash, and the DLSS half is small and easy to follow.

Frame generation. Taking fg.mult in framegen_record is the right fix. ensure_size made the sets for that count, and nothing on the display thread reads multiplier() to size anything. Reading opts.framegen == 1 once also holds up: both branches are now bounded by fg.mult, so a switch between Auto and a fixed count between ensure_size and the record stays inside the sets that exist. Jiro131 (RTX 4090, Windows 11) and polackangry (RTX 5060 Ti, Windows 11) both crashed main with the same driver read inside framegen_record when stepping the multiplier, and neither crashed with this change. Together with your own Linux runs, that's the in-game evidence this needs.

DLSS retry. I checked that it can't turn into a retry every frame:

  • refused is cleared in three places: when the key changes, in release(), and in dlss_retry(). release() only runs when the Upscaler switches engines, when the chain goes idle (and only once the row has left DLSS), or once after 30 failed evaluations. dlss_retry() only runs from the menu's DLSS, Render resolution and DLSS model rows. So a creation that keeps failing is tried again once per player change, not once per frame. While DLSS is off, prepare returns 0 with the same reason pointer, so the chain's fail() doesn't log each frame either.
  • The 30-in-a-row release happens once for each run of the same error code (as Joey-Magnus noted): one new feature, then at most one log line every 600 frames. If that new creation fails, refused holds it off again. The release() inside evaluate defers the handle through temporal_later, so the evaluation already recorded in cmd keeps a live feature.
  • While DLSS is off, the state stays consistent: s.feature is null, the stale feature_* sizes are only read behind s.feature, hdr() reports false, and the chain falls back to the game's upscale.
  • dlss_retry runs under the chain's lock, as prepare and release do, so refused has a single owner. The non-DLSS stub is there too.

Two small notes, inline. Neither blocks.

You couldn't make a creation or an evaluation fail on a 16 GB card, so the DLSS retry is checked by reading only. That's fine here: the normal DLSS path ran in the game with no regression, and if the new branches misbehaved, the worst they could do is what main already does. If someone with an 8 GB card hits a its feature was not made line, a log showing feature N made (again after it was let go) after a row change would close it.

CI is green and the branch merges cleanly. I found no open PR that touches these lines. #2788 and #2665 edit other parts of vk_dlss.cpp / vk_framegen.cpp, so the merge order doesn't matter.

Approving.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 16:33 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2658#pullrequestreview-5878216202) Thanks, this is a good one. The frame generation half fixes a real crash, and the DLSS half is small and easy to follow. **Frame generation.** Taking `fg.mult` in `framegen_record` is the right fix. `ensure_size` made the sets for that count, and nothing on the display thread reads `multiplier()` to size anything. Reading `opts.framegen == 1` once also holds up: both branches are now bounded by `fg.mult`, so a switch between Auto and a fixed count between `ensure_size` and the record stays inside the sets that exist. Jiro131 (RTX 4090, Windows 11) and polackangry (RTX 5060 Ti, Windows 11) both crashed main with the same driver read inside `framegen_record` when stepping the multiplier, and neither crashed with this change. Together with your own Linux runs, that's the in-game evidence this needs. **DLSS retry.** I checked that it can't turn into a retry every frame: - `refused` is cleared in three places: when the key changes, in `release()`, and in `dlss_retry()`. `release()` only runs when the Upscaler switches engines, when the chain goes idle (and only once the row has left DLSS), or once after 30 failed evaluations. `dlss_retry()` only runs from the menu's DLSS, Render resolution and DLSS model rows. So a creation that keeps failing is tried again once per player change, not once per frame. While DLSS is off, `prepare` returns 0 with the same reason pointer, so the chain's `fail()` doesn't log each frame either. - The 30-in-a-row release happens once for each run of the same error code (as Joey-Magnus noted): one new feature, then at most one log line every 600 frames. If that new creation fails, `refused` holds it off again. The `release()` inside `evaluate` defers the handle through `temporal_later`, so the evaluation already recorded in `cmd` keeps a live feature. - While DLSS is off, the state stays consistent: `s.feature` is null, the stale `feature_*` sizes are only read behind `s.feature`, `hdr()` reports false, and the chain falls back to the game's upscale. - `dlss_retry` runs under the chain's lock, as `prepare` and `release` do, so `refused` has a single owner. The non-DLSS stub is there too. Two small notes, inline. Neither blocks. You couldn't make a creation or an evaluation fail on a 16 GB card, so the DLSS retry is checked by reading only. That's fine here: the normal DLSS path ran in the game with no regression, and if the new branches misbehaved, the worst they could do is what main already does. If someone with an 8 GB card hits a `its feature was not made` line, a log showing `feature N made (again after it was let go)` after a row change would close it. CI is green and the branch merges cleanly. I found no open PR that touches these lines. #2788 and #2665 edit other parts of `vk_dlss.cpp` / `vk_framegen.cpp`, so the merge order doesn't matter. Approving. <!-- github-sync:r:5878216202 -->
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!2658
No description provided.