fix(vk): capture switches follow the on/off rule #2651

Merged

mooki000 merged 1 commit from fix/capture-switch-values into main

2026-11-12 04:30:16 +00:00

mooki000 commented

2026-11-12 04:23:54 +00:00

(Migrated from github.com)

What this is for: make the two capture-buffer switches follow the documented on/off rule, and restore the existing switch check (current main CI). What is measured: the affected game-free check before and after, with GCC 16.2.1 and Clang 23.1.3 selected, plus one strict full-suite run, as recorded below. Not measured: a game run or Windows execution.

Cause

#2437 added two presence-only reads of RX_VK_CAPTURE_KEEP_OLD and one of RX_VK_CAP_BLOCKS in gpu/vk/vk_memory.cpp; setting them to 0 therefore enabled the alternatives, contrary to hookshot_env.h and the existing test_env_on.py rule.

The relevant failure in main's CI log is:

FAIL: on/off switches still read as set-means-on (set to 0 they are on): use env_on:
  RX_VK_CAPTURE_KEEP_OLD (gpu/vk/vk_memory.cpp:937, gpu/vk/vk_memory.cpp:960)
  RX_VK_CAP_BLOCKS (gpu/vk/vk_memory.cpp:995)

What changed

Use the existing env_on helper at those three sites and document the switches in docs/development.md; defaults, enabled behavior, allocation policy and the test remain unchanged.

How it was tested

Recorded on 2026-11-11, Linux kernel 7.2.15-1-omarchy, base de866017, head 8d7f7d47 (version and test records).

Untouched main a016b7a6: the affected check fails independently in both compiler environments with the diagnostic above; the branch passes independently in both (python -I -B tests/run_all.py --strict --jobs 1 tests/test_env_on.py records).
The unchanged check rejects the three original reads on main with both compilers; its C/C++/Python truth table passes on the branch, including unset, empty, 0, false, off and no (standalone records).
One full python -I -B tests/run_all.py --strict: 456 passed, 14 failed, 20 skipped in 139 s (linux, 32 at once), load 17.65 → 26.92 (complete suite output).
That run's remaining failures are: test_wayland_libdecor_hint.py, test_display_startup_failure.py, test_texel_buffer_ownership.py, test_audio_aux.py, test_audio_biquad.py, test_dlss_checks.py, test_framegen_preset.py, test_hdr_encode.py, test_linux_docker_packages.py, test_menu_i18n.py, test_mods_size_table.py, test_mouse_native.py, test_readback_part.py, test_rumble_menu.py (complete suite output). The companion fixture, PPA-parser, camera-handover and hint-catalog branches address their separate causes; #2614 owns the HDR check and is not duplicated here.

Not tested / risks

The deliberate behavior change is for values that explicitly mean off; no Vulkan allocation or game run was performed, so allocation timing and hardware behavior are not measured.

Overlaps

The inspected same-file code changes merge cleanly; #2338 and #2618 add different documentation rows at the same insertion point, so those rows and this one must be retained together (recorded overlap hunks).

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

**What this is for:** make the two capture-buffer switches follow the documented on/off rule, and restore the existing switch check ([current main CI](https://github.com/NotRexed/Hookshot/actions/runs/43151254256)). **What is measured:** the affected game-free check before and after, with GCC 16.2.1 and Clang 23.1.3 selected, plus one strict full-suite run, as recorded below. **Not measured:** a game run or Windows execution. ## Cause #2437 added two presence-only reads of `RX_VK_CAPTURE_KEEP_OLD` and one of `RX_VK_CAP_BLOCKS` in `gpu/vk/vk_memory.cpp`; setting them to `0` therefore enabled the alternatives, contrary to `hookshot_env.h` and the existing `test_env_on.py` rule. The relevant failure in [main's CI log](https://github.com/NotRexed/Hookshot/actions/runs/43151254256) is: ```text FAIL: on/off switches still read as set-means-on (set to 0 they are on): use env_on: RX_VK_CAPTURE_KEEP_OLD (gpu/vk/vk_memory.cpp:937, gpu/vk/vk_memory.cpp:960) RX_VK_CAP_BLOCKS (gpu/vk/vk_memory.cpp:995) ``` ## What changed Use the existing `env_on` helper at those three sites and document the switches in `docs/development.md`; defaults, enabled behavior, allocation policy and the test remain unchanged. ## How it was tested Recorded on 2026-11-11, Linux kernel `7.2.15-1-omarchy`, base `de866017`, head `8d7f7d47` (version and test records). - Untouched main `a016b7a6`: the affected check fails independently in both compiler environments with the diagnostic above; the branch passes independently in both (`python -I -B tests/run_all.py --strict --jobs 1 tests/test_env_on.py` records). - The unchanged check rejects the three original reads on main with both compilers; its C/C++/Python truth table passes on the branch, including unset, empty, `0`, `false`, `off` and `no` (standalone records). - One full `python -I -B tests/run_all.py --strict`: **`456 passed, 14 failed, 20 skipped in 139 s (linux, 32 at once)`**, load 17.65 → 26.92 (complete suite output). - That run's remaining failures are: `test_wayland_libdecor_hint.py`, `test_display_startup_failure.py`, `test_texel_buffer_ownership.py`, `test_audio_aux.py`, `test_audio_biquad.py`, `test_dlss_checks.py`, `test_framegen_preset.py`, `test_hdr_encode.py`, `test_linux_docker_packages.py`, `test_menu_i18n.py`, `test_mods_size_table.py`, `test_mouse_native.py`, `test_readback_part.py`, `test_rumble_menu.py` (complete suite output). The companion fixture, PPA-parser, camera-handover and hint-catalog branches address their separate causes; #2614 owns the HDR check and is not duplicated here. ## Not tested / risks The deliberate behavior change is for values that explicitly mean off; no Vulkan allocation or game run was performed, so allocation timing and hardware behavior are not measured. ## Overlaps The inspected same-file code changes merge cleanly; #2338 and #2618 add different documentation rows at the same insertion point, so those rows and this one must be retained together (recorded overlap hunks). Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]>

hckrman101 (Migrated from github.com) approved these changes

2026-11-12 04:27:27 +00:00

hckrman101 (Migrated from github.com) left a comment

Looks good. I checked all three reads against hookshot_env.h:

RX_VK_CAPTURE_KEEP_OLD (both reads, cap_buf_free and cap_buf_size) and RX_VK_CAP_BLOCKS (cap_spare_make) now go through env_on. env_on is getenv non-null and not env_value_off, so unset, empty, 0, false, off and no (any case, spaces ignored) are off, and anything else is on.
Unset is unchanged: both were off by default before, and !env_on("RX_VK_CAP_BLOCKS") is still true when unset, so dedicated memory stays the default. Only values that explicitly mean off change, which is the point of the fix.
RX_VK_CAP_BLOCKS at the other call site (the create_buffer_cached call further down) already used env_on, so the two sites now agree.
hookshot_env.h is already included in vk_memory.cpp, so nothing new to include. The static const bool reads are once-only, as before.
These are exactly the three lines test_env_on flagged on main, so it should pass again; I didn't run it. The docs row matches the code.

CI: the Python source checks were still pending when I looked. The failures you list from the full run are the known main ones (test_menu_i18n, test_hdr_encode, test_linux_docker_packages, test_mouse_native, with the others covered by sibling PRs), not from this change.

Looks good. I checked all three reads against `hookshot_env.h`: - `RX_VK_CAPTURE_KEEP_OLD` (both reads, `cap_buf_free` and `cap_buf_size`) and `RX_VK_CAP_BLOCKS` (`cap_spare_make`) now go through `env_on`. `env_on` is `getenv` non-null and not `env_value_off`, so unset, empty, `0`, `false`, `off` and `no` (any case, spaces ignored) are off, and anything else is on. - Unset is unchanged: both were off by default before, and `!env_on("RX_VK_CAP_BLOCKS")` is still true when unset, so dedicated memory stays the default. Only values that explicitly mean off change, which is the point of the fix. - `RX_VK_CAP_BLOCKS` at the other call site (the `create_buffer_cached` call further down) already used `env_on`, so the two sites now agree. - `hookshot_env.h` is already included in `vk_memory.cpp`, so nothing new to include. The `static const bool` reads are once-only, as before. - These are exactly the three lines `test_env_on` flagged on main, so it should pass again; I didn't run it. The docs row matches the code. CI: the Python source checks were still pending when I looked. The failures you list from the full run are the known main ones (`test_menu_i18n`, `test_hdr_encode`, `test_linux_docker_packages`, `test_mouse_native`, with the others covered by sibling PRs), not from this change.