MagicBeanDev/Hookshot

Archived

Watch

Fork 0

feat(Menu): a row that turns the preset to Custom keeps the preset's memory budgets #1358

Merged

DarkzGG merged 2 commits from fix/custom-keeps-memory-budget into main

2026-11-11 02:29:42 +00:00

DarkzGG commented

2026-11-09 23:00:50 +00:00

(Migrated from github.com)

Problem

Pick Quality preset → Low (6GB), then step Resolution (or any other non-exempt row). The preset becomes Custom, and set_row calls sync_preset_budgets(0). That sets vram_mb and mirror_mb back to 0, so the card loses the texture and mirror caps the preset gave it. Nothing in the menu says so.

On an RTX 4050 Laptop (6 GB) this is exactly what players do: pick Low, then tune Resolution. Without the cap the game went past its VRAM: vk: VRAM: 5896 MB used of 5080 MB allowed, VRAM full: ... in system memory. GPU time in Kakariko rose to ~37 ms/frame, and with DLSS on, Streamline logged Exceeded VRAM budget and refused frames. With the same settings and the Low budget restored (via RX_VK_VRAM_MB=3600), usage stayed under the limit and GPU time fell to ~13.5 ms in a similar scene.

Change

opts.budget is the preset whose memory budgets hold.
- The preset row sets it.
- Other rows leave it alone when they turn the preset to Custom.
- Choosing Custom on the preset row itself still clears it, as before.
It is saved as memory_budget=<n> in hookshot_options.ini. It is not a menu row, so it is written next to microphone_device. It is read back at start.
An options file from before this change has no memory_budget. It gets its preset's budgets, as today.
preset_caps_memory() checks the card size against opts.budget instead of opts.preset. A smaller preset on a bigger card therefore keeps the card's own budget after a row is tuned too, the same as it does while the preset is shown.

3 files, +12 / −4: gpu/vk/vk_menu.cpp, gpu/vk/vk_menu.h, gpu/vk/vk_memory.cpp.

Tested

hookshot_vk.dll builds (llvm-mingw, RX_VK_ONLY=1).
tests/test_rumble_menu.py still applies: it stubs sync_preset_budgets, and set_row no longer calls it.
Not yet run in game on this branch. The bug and the effect of restoring the cap were measured on v2.0.5 and v2.0.11 with an equivalent RX_VK_VRAM_MB setting.

🤖 Generated with Claude Code

**Problem** Pick **Quality preset → Low (6GB)**, then step **Resolution** (or any other non-exempt row). The preset becomes Custom, and `set_row` calls `sync_preset_budgets(0)`. That sets `vram_mb` and `mirror_mb` back to 0, so the card loses the texture and mirror caps the preset gave it. Nothing in the menu says so. On an RTX 4050 Laptop (6 GB) this is exactly what players do: pick Low, then tune Resolution. Without the cap the game went past its VRAM: `vk: VRAM: 5896 MB used of 5080 MB allowed`, `VRAM full: ... in system memory`. GPU time in Kakariko rose to ~37 ms/frame, and with DLSS on, Streamline logged `Exceeded VRAM budget` and refused frames. With the same settings and the Low budget restored (via `RX_VK_VRAM_MB=3600`), usage stayed under the limit and GPU time fell to ~13.5 ms in a similar scene. **Change** - `opts.budget` is the preset whose memory budgets hold. - The preset row sets it. - Other rows leave it alone when they turn the preset to Custom. - Choosing Custom on the preset row itself still clears it, as before. - It is saved as `memory_budget=<n>` in `hookshot_options.ini`. It is not a menu row, so it is written next to `microphone_device`. It is read back at start. - An options file from before this change has no `memory_budget`. It gets its preset's budgets, as today. - `preset_caps_memory()` checks the card size against `opts.budget` instead of `opts.preset`. A smaller preset on a bigger card therefore keeps the card's own budget after a row is tuned too, the same as it does while the preset is shown. 3 files, +12 / −4: `gpu/vk/vk_menu.cpp`, `gpu/vk/vk_menu.h`, `gpu/vk/vk_memory.cpp`. **Tested** - `hookshot_vk.dll` builds (llvm-mingw, `RX_VK_ONLY=1`). - `tests/test_rumble_menu.py` still applies: it stubs `sync_preset_budgets`, and `set_row` no longer calls it. - Not yet run in game on this branch. The bug and the effect of restoring the cap were measured on v2.0.5 and v2.0.11 with an equivalent `RX_VK_VRAM_MB` setting. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) reviewed

2026-11-09 23:24:24 +00:00

hckrman101 (Migrated from github.com) left a comment

The change reads right to me. opts.budget is set only by apply_preset (and from the file at start), set_row no longer zeroes vram_mb/mirror_mb, and picking Custom on the preset row itself still clears it. The old-file path (no memory_budget, preset > 0) gives the preset's budgets as before. preset_caps_memory() is the only consumer that looked at opts.preset, and it now follows opts.budget.

The PR conflicts with main, in one place: menu_init() in gpu/vk/vk_menu.cpp, next to sync_preset_budgets(...). #1121 (frame generation Auto) deleted the line if (opts.framegen == 1) opts.framegen = 2; that sits right under your edit, so the hunks touch adjacent lines. The other hunks in vk_menu.cpp, vk_menu.h and vk_memory.cpp merge cleanly (I merged them locally against current main). To resolve it, rebase and keep your two budget lines without the framegen == 1 line:

if (opts.preset > 0) opts.budget = opts.preset;   // (a file from before memory_budget: its preset's)
sync_preset_budgets(opts.budget);

Small things, none blocking:

nit: the hint on the Quality preset row still says "Changing most rows yourself comes back to Custom", which stays true. Nothing to change, though it might be worth a word that the memory caps stay.
No checks have run on the branch, so the rebase will be the first CI run. The game test is still open, since you only measured the effect with RX_VK_VRAM_MB. It would be good to confirm in game that Low, then a Resolution step, logs the 3600 MB budget.

Good to go after the rebase.

The change reads right to me. `opts.budget` is set only by `apply_preset` (and from the file at start), `set_row` no longer zeroes `vram_mb`/`mirror_mb`, and picking Custom on the preset row itself still clears it. The old-file path (no `memory_budget`, preset > 0) gives the preset's budgets as before. `preset_caps_memory()` is the only consumer that looked at `opts.preset`, and it now follows `opts.budget`. The PR conflicts with main, in one place: `menu_init()` in `gpu/vk/vk_menu.cpp`, next to `sync_preset_budgets(...)`. #1121 (frame generation Auto) deleted the line `if (opts.framegen == 1) opts.framegen = 2;` that sits right under your edit, so the hunks touch adjacent lines. The other hunks in `vk_menu.cpp`, `vk_menu.h` and `vk_memory.cpp` merge cleanly (I merged them locally against current main). To resolve it, rebase and keep your two budget lines without the `framegen == 1` line: ``` if (opts.preset > 0) opts.budget = opts.preset; // (a file from before memory_budget: its preset's) sync_preset_budgets(opts.budget); ``` Small things, none blocking: - nit: the hint on the Quality preset row still says "Changing most rows yourself comes back to Custom", which stays true. Nothing to change, though it might be worth a word that the memory caps stay. - No checks have run on the branch, so the rebase will be the first CI run. The game test is still open, since you only measured the effect with `RX_VK_VRAM_MB`. It would be good to confirm in game that Low, then a Resolution step, logs the 3600 MB budget. Good to go after the rebase.

DarkzGG commented

2026-11-09 23:56:40 +00:00

(Migrated from github.com)

Thanks for the review! I rebased on current main and resolved the conflict the way you described: the two budget lines are kept, without the framegen == 1 line. I also took the nit. The Quality preset hint now reads "…comes back to Custom, with the preset's memory caps kept."

In-game test: RTX 4050 Laptop, 6 GB. Built on v2.0.11 with the same change applied, since the branch itself is ahead of my game's version. hookshot_env.txt had RX_VK_VRAM_MB=3000, so the log shows which budget is in effect:

At start, preset Custom with no memory_budget: textures 94 MB (budget 3000 MB), the env value.
Quality preset → Low (6GB), then Resolution → 67 %. The preset shows Custom, and the log reads textures 2482 MB (budget 3600 MB). Before the fix this went back to 3000.
Saved file: quality_preset=0 with memory_budget=2.
After a restart: budget 3600 MB from the first VRAM line on. It later reads 3555 MB, which is the existing 75 %-of-allowed safety clamp (4741 MB allowed at that point). There were no Exceeded VRAM budget warnings from Streamline this run.

The menu tests that touch these rows pass locally: test_upscaler_rows.py, test_rumble_menu.py and test_audio_output_menu.py.

Thanks for the review! I rebased on current main and resolved the conflict the way you described: the two budget lines are kept, without the `framegen == 1` line. I also took the nit. The Quality preset hint now reads "…comes back to Custom, with the preset's memory caps kept." **In-game test:** RTX 4050 Laptop, 6 GB. Built on v2.0.11 with the same change applied, since the branch itself is ahead of my game's version. `hookshot_env.txt` had `RX_VK_VRAM_MB=3000`, so the log shows which budget is in effect: 1. **At start, preset Custom with no `memory_budget`:** `textures 94 MB (budget 3000 MB)`, the env value. 2. **Quality preset → Low (6GB), then Resolution → 67 %.** The preset shows Custom, and the log reads `textures 2482 MB (budget 3600 MB)`. Before the fix this went back to 3000. 3. **Saved file:** `quality_preset=0` with `memory_budget=2`. 4. **After a restart:** `budget 3600 MB` from the first VRAM line on. It later reads 3555 MB, which is the existing 75 %-of-allowed safety clamp (4741 MB allowed at that point). There were no `Exceeded VRAM budget` warnings from Streamline this run. The menu tests that touch these rows pass locally: `test_upscaler_rows.py`, `test_rumble_menu.py` and `test_audio_output_menu.py`.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 03:37:34 +00:00

hckrman101 (Migrated from github.com) left a comment

Re-checked 4fde5eb..d6cbbee. The rebase is clean: the two budget lines in menu_init() are kept without the framegen == 1 line, and GitHub no longer reports a conflict. The hint nit is taken, and the in-game run (Low, then Resolution to 67%, shows 3600 MB, memory_budget=2 saved, still 3600 after restart, no Streamline warnings) covers the open test point.

The diff is otherwise the same as before, and I still find nothing wrong with it. No CI has run on the branch yet (the checks list is empty), so the first run will be after this lands in the queue. Good to merge.

Re-checked 4fde5eb..d6cbbee. The rebase is clean: the two budget lines in `menu_init()` are kept without the `framegen == 1` line, and GitHub no longer reports a conflict. The hint nit is taken, and the in-game run (Low, then Resolution to 67%, shows 3600 MB, `memory_budget=2` saved, still 3600 after restart, no Streamline warnings) covers the open test point. The diff is otherwise the same as before, and I still find nothing wrong with it. No CI has run on the branch yet (the checks list is empty), so the first run will be after this lands in the queue. Good to merge.

rakefire-impala commented

2026-11-10 06:34:52 +00:00

(Migrated from github.com)

Independent confirmation from a 4 GB card, and since #1485 this bug hits harder.

RX 5500M laptop (RDNA1, 4 GB; the driver allows ~3.4 GB), main 0c412ad. I had picked Minimum (4GB), then turned frame generation back on, so the preset became Custom and its 2200 MB texture budget was dropped, exactly as described here. Before #1485 the fallback was 70% of what the driver allows (2412 MB on this card), so it went unnoticed. Since #1485 it's 45%: 1551 MB, below the ~2.1-2.4 GB that Kokiri Forest's textures take.

Same place, same settings (Custom, frame generation 2x):

	Custom, so #1485's 45% default (1551 MB)	`RX_VK_VRAM_MB=2412`
Median fps in play	25.9	30.2
Textures across a long pause	993 → 413 (580 evicted)	1052 → 800 (~250 evicted)
fps right after resuming	58.7, 53.5, then 8.6	60.1, then normal play
VRAM used / allowed	3213 / 3235 MB	3231 / 3231 MB

One caveat: the second session also had more free system RAM, which helps the alt-tab case. The pause numbers don't depend on it.

With this PR, picking Minimum and then changing a row would have kept the 2200 MB budget, which is about what the second column shows.

Independent confirmation from a 4 GB card, and since #1485 this bug hits harder. RX 5500M laptop (RDNA1, 4 GB; the driver allows ~3.4 GB), main 0c412ad. I had picked Minimum (4GB), then turned frame generation back on, so the preset became Custom and its 2200 MB texture budget was dropped, exactly as described here. Before #1485 the fallback was 70% of what the driver allows (2412 MB on this card), so it went unnoticed. Since #1485 it's 45%: 1551 MB, below the ~2.1-2.4 GB that Kokiri Forest's textures take. Same place, same settings (Custom, frame generation 2x): | | Custom, so #1485's 45% default (1551 MB) | `RX_VK_VRAM_MB=2412` | |---|---|---| | Median fps in play | 25.9 | 30.2 | | Textures across a long pause | 993 → 413 (580 evicted) | 1052 → 800 (~250 evicted) | | fps right after resuming | 58.7, 53.5, then 8.6 | 60.1, then normal play | | VRAM used / allowed | 3213 / 3235 MB | 3231 / 3231 MB | One caveat: the second session also had more free system RAM, which helps the alt-tab case. The pause numbers don't depend on it. With this PR, picking Minimum and then changing a row would have kept the 2200 MB budget, which is about what the second column shows.

hckrman101 commented

2026-11-10 17:49:10 +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!

DarkzGG commented

2026-11-10 21:35:45 +00:00

(Migrated from github.com)

Rebased on current main (7b81752). The only conflict was the Quality preset hint, whose preset lines changed on main: I kept main's text and added only "…with the preset's memory caps kept." The budget changes applied cleanly, and hookshot_vk.dll builds.

@rakefire-impala thanks for the 4 GB numbers. They show the same thing on a smaller card, and that the 45% default from #1485 makes this bug easier to hit.

Rebased on current main (7b81752). The only conflict was the Quality preset hint, whose preset lines changed on main: I kept main's text and added only "…with the preset's memory caps kept." The budget changes applied cleanly, and `hookshot_vk.dll` builds. @rakefire-impala thanks for the 4 GB numbers. They show the same thing on a smaller card, and that the 45% default from #1485 makes this bug easier to hit.

DarkzGG commented

2026-11-11 00:43:52 +00:00

(Migrated from github.com)

Rebased again on current main (e187859). Two conflicts, both next to my lines, and both resolved by keeping main's line beside mine:

menu_init()'s options reading: main's new menu_size line.
apply_preset(): #2000's ur::snap_render(...), with opts.budget = p; after it.

hookshot_vk.dll builds.

Rebased again on current main (e187859). Two conflicts, both next to my lines, and both resolved by keeping main's line beside mine: - `menu_init()`'s options reading: main's new `menu_size` line. - `apply_preset()`: #2000's `ur::snap_render(...)`, with `opts.budget = p;` after it. `hookshot_vk.dll` builds.

hckrman101 (Migrated from github.com) approved these changes

2026-11-11 02:15:24 +00:00

hckrman101 (Migrated from github.com) left a comment

One thing to fix before merge: the Conventional Commit title check fails. The PR title is Menu: a row that turns the preset to Custom keeps the preset's memory budgets, so it needs a type prefix, e.g. fix(menu): a row that turns the preset to Custom keeps the preset's memory budgets. No other checks have run on the branch.

Re-checked against the new head. The author rebased twice on main since my approval. The diff is still the same three files, and the two conflict spots are resolved the way they describe: `menu_init()` keeps main's `menu_size` line beside `memory_budget`, and `apply_preset()` keeps #2000's `ur::snap_render(...)` line with `opts.budget = p;` after it. `set_row` still no longer zeroes the budgets, Custom on the preset row still clears `budget`, and a file without `memory_budget` still gets its preset's budgets. I found nothing new in the code, and the in-game run from the last round still covers it. The GitHub conflict state is clear. One thing to fix before merge: the `Conventional Commit title` check fails. The PR title is `Menu: a row that turns the preset to Custom keeps the preset's memory budgets`, so it needs a type prefix, e.g. `fix(menu): a row that turns the preset to Custom keeps the preset's memory budgets`. No other checks have run on the branch.