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

fix(vk): keep ultrawide loading fades full width #2851

Open
github-sync wants to merge 3 commits from github/pr/2851 into main
pull from: github/pr/2851
merge into: MagicBeanDev:main
Conversation 6 Commits 3 Files changed 5 +161 -6
github-sync commented 2026-11-12 16:54:38 +00:00
Owner

@mooki000 opened this pull request on GitHub · 2026-11-12 16:36 UTC · original

What this is for: centred ultrawide menus currently leave the scene visible in the side strips during a loading fade. This keeps the fade full-width, but it also moves the loading icon into the side treatment, so this is a draft proposal, not a complete visual fix. What is measured: real windowless game captures on Linux/RTX 4090 at the reporter's 4432×1856 game-picture size, plus compiled regression checks. Not measured: a physical ultrawide display, Windows gameplay, other GPU vendors or performance.

Refs #2220. Credit to @EjianYT for identifying bit 3 and testing the 0xa → 0x2 correction.

Problem

  • nx2/oot_wide.py has one keep bit per UI canvas. Bit 3 belongs to the loading fade, not the map.
  • wide_sides() in gpu/vk/vk_framegen.cpp removed bits 1 and 3 in either centred mode. The HUD patch therefore squeezed the entire loading canvas into the middle 16:9.
  • Keeping bit 3 fixes the uncovered sides. However, the icon shares that canvas: it becomes wider and moves towards the full picture's right edge. The existing blurred-side pass then blurs it. This is visible in the comparison below, not just a theoretical risk.

What changed

  • Keep only the pause/system background's bit 1 out of the full-width mask in centred modes. Full width still uses 0xf; centred modes now use 0xd instead of 0x5.
  • Correct the canvas descriptions in vk_framegen.cpp and oot_wide.py.
  • Add test_wide_loading_fade.py, compiling the real renderer policy and generated HUD patch with authored state, in the existing wide-shape fixture style.
  • Document the icon trade-off and add six rows to docs/tested-configurations.md, including the unsuccessful input routes. No new shader, runtime switch, host interface, pane hook or generated game code is included.

The pause/map centring, wheel background, event fades and inactive 16:9 projection are deliberately unchanged.

How it was tested

Environment: Arch Linux, kernel 7.2.15-1-omarchy; Intel Core i9-13900K; NVIDIA RTX 4090, driver 615.78.08. Base 51da0464, mask-only change 7dc0773b, 2026-11-12. The same host executable was used before and after. Its base build includes dormant readback diagnostics (c372e16e, not enabled); the changed renderer was built from this branch. No Xbox UI mod or overlay was used.

Rebase: final head d2e37d47 is on e2057b2b. The one end-of-task rebase was conflict-free. The fade regression was rerun with GCC and Clang and passes; its generated unit is byte-identical to the Windows-syntax-checked unit. Upstream added AMD Anti-Lag frame-generation tagging, not a change to the fade policy. The game build/captures and full-suite result below are from before that rebase; the rebased game was not rebuilt or run.

The reporter's log distinguishes 3840×2160 output setting, 4432×1856 game picture, 3328×1392 scene, and a 3440×1440 swapchain. The reproduction uses the first three, with HOS_ASPECT=2.3889 and HOS_WIDE_MAX_PX=8294400; it does not pretend that a windowless run has the reporter's swapchain. Both 21:9 captures are 2216×928, preserving the game picture's shape.

Game-free checks

  • GCC 16.2.1 and Clang 23.1.3: the new test compiles and fails on the pre-fix sources with 12 FAIL lines each: four keep-mask checks and eight loading-projection components. With the change: 
    PASS: loading/event fades and wheel stay full width; pause/HUD centring, sides and 16:9 unchanged
  • Existing wide-shape, save-picture and independent-scaler tests pass with both compiler families.
  • An initial test-only chained volatile assignment was rejected by both C++20 compilers. It was split into separate stores before collecting the runtime negative control; no warning was suppressed.
  • Full Linux suite, pinned Python 3.12.16: 
    476 passed, 0 failed, 20 skipped in 164 s (linux, 8 at once)
  • The final generated C++ fixture passes Windows-target syntax compilation with its own -std=c++20 -O2 -Wall -Wextra -Werror flags. The local preflight had an incorrect C/libc++ include order; its captured source was compiled with that order corrected. This is not Windows execution.
  • Renderer builds and links. A separate full-file -Wall -Wextra check reports 11 missing-initializer warnings at unchanged Vulkan structure initializers, none at the mask change. No fixture core dumps were produced.

Game captures

All captures used the real scale pass (RX_VK_SHOT_FX=1). The bridge's named raw screenshots bypass that pass and were not used to judge the blur. These were ordinary runs on a busy PC, not exclusive timing runs; no fps, latency or performance claim is made.

Run Picture Place and result Driver faults
Before, centred blurred sides 4432×1856 Load into Hyrule Field: only the middle fades, with bright side strips. Pause and actual Map centred. 0
Before, 16:9 3840×2160 Loading fade fills the frame; sharp icon; pause checked. The attempted Map capture was the first-use tapestry, so it is not counted as a Map result. 0
Mask only, centred blurred sides 4432×1856 Hyrule Field fade fills the width; pause/Map remain centred. Transition to Kokiri Forest completed. Loading icon becomes a dim blur at the side. 0
Before, corrected 16:9 route 3840×2160 Loading fade, sharp icon, pause and actual Map checked; transition to Kokiri Forest completed. 0
Mask only, 16:9 3840×2160 Same fade, icon proportions/position, pause and Map; transition completed. 0
Mask only, black bars then Full width 4432×1856 Both fades cover the width. Black bars hide the loading icon; Full width shows it sharply but stretched. Black-bar pause/Map remain centred. Both requested transitions completed. 0

Retained input failures: one initial capture helper used a relative count with an absolute-frame wait; the first 16:9 route dismissed the tapestry but did not leave its menu; the extended mode check used an unrecognized setting key. Each stopped on its error, and no unreached step is counted as evidence. None was a game crash or device loss.

Six panels at the reporter's picture size: the loading fade before (only the middle fades) and with the mask change (the whole picture), the pause menu and the Map unchanged

six inspected panels: before/mask-only loading fade, pause and Map. Same save, picture size and settings; the loading frames compare coverage, not identical fade opacity or frame numbers.

The loading icon, 2x crops: sharp inside the middle 16:9 before; a dim blur at the outer edge with the mask change

identical bottom-right crops, enlarged 2×: the original sharp icon inside the central 16:9 versus the mask-only icon blurred near the outer edge. This is why the proposal remains a draft.

16:9 before and after: the fade, the sharp icon, the pause menu and the Map are unchanged

six inspected panels at the same 3840×2160 picture size: before/after loading fade and sharp icon, pause and actual Map. Captures are 1920×1080.

The icon in each mode: before; mask change with blurred sides (a dim blur); with black bars (hidden); Full width (sharp but stretched)

equal 2× crops showing the original centred icon, the mask-only blurred icon, the hidden black-bars icon and the sharp-but-stretched Full-width reference. A separate raw image immediately before the black-bars capture still shows the icon: the side pass, not a missing draw, removes it.

All six runs reported zero fatal/crash/device-loss/kernel-driver-fault lines. The first 16:9 run recorded one virtual-core escape at startup; the other five recorded none. The last three capture routes passed their checks and quit normally. Full width's unchanged mask is covered by the compiled before/after checks; its runtime image is a reference from the changed binary, not a separate pre-fix Full-width run.

Not tested / not shown

  • Windows gameplay, AMD/Intel/Steam Deck, physical ultrawide presentation, HDR and frame-generated output. (The new test itself ran on Windows: both windows-2022 source-check parts passed on a run on my fork at this head before the pull request was opened.)
  • A complete fade-pane-only fix that preserves the icon's original position and shape has not been implemented or demonstrated.

Why not simply keep the icon centred too?

The existing HUD hook changes one projection for the whole canvas, not an individual pane. Clearing bit 3 centres both the icon and fade; keeping it widens both. The side pass receives only margins and its mode, not a pane identity or loading opacity.

A complete solution needs a new game-side hook that widens only the fade pane, or a new loading-state/opacity hand-off to the side-fill path and its shader. Exempting the outer pixels alone would still leave a stretched, displaced icon. This proposal does not add an unproved pane patch or a dark-pixel heuristic.

Risks

The loading icon regression is real in both centred modes: it is a dim blur with blurred sides and completely hidden by black bars. Full width shows it sharply but stretched; 16:9 keeps its original position and proportions. The mask-only change should not be merged as if it preserves the loading indicator.

Overlaps

The pre-rebase open-PR comparison found 57 PRs touching one of the initial four changed files. The shared-code PRs #1488, #1979, #2015, #2017, #2172, #2261, #2262, #2531, #2658 and #2665 merged cleanly in that comparison; no conflict hunk was reported. No competing loading-fade repair was found. The final branch gate passes against e2057b2b.

Questions for the maintainers and reporter

  1. Do you prefer the full-width fade with this icon trade-off, or the current behaviour until the fade pane can be handled separately?
  2. @EjianYT, does your real-window build of the mask correction also blur or hide the icon in the centred modes, or does it differ from these captures without the UI mod?

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

> [@mooki000](https://github.com/mooki000) opened this pull request on GitHub · 2026-11-12 16:36 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2851) **What this is for:** centred ultrawide menus currently leave the scene visible in the side strips during a loading fade. This keeps the fade full-width, but it also moves the loading icon into the side treatment, so this is a **draft proposal, not a complete visual fix**. **What is measured:** real windowless game captures on Linux/RTX 4090 at the reporter's 4432×1856 game-picture size, plus compiled regression checks. **Not measured:** a physical ultrawide display, Windows gameplay, other GPU vendors or performance. Refs #2220. Credit to @EjianYT for identifying bit 3 and testing the `0xa` → `0x2` correction. ## Problem - `nx2/oot_wide.py` has one keep bit per UI canvas. Bit 3 belongs to the loading fade, not the map. - `wide_sides()` in `gpu/vk/vk_framegen.cpp` removed bits 1 and 3 in either centred mode. The HUD patch therefore squeezed the entire loading canvas into the middle 16:9. - Keeping bit 3 fixes the uncovered sides. However, the icon shares that canvas: it becomes wider and moves towards the full picture's right edge. The existing blurred-side pass then blurs it. This is visible in the comparison below, not just a theoretical risk. ## What changed - Keep only the pause/system background's bit 1 out of the full-width mask in centred modes. Full width still uses `0xf`; centred modes now use `0xd` instead of `0x5`. - Correct the canvas descriptions in `vk_framegen.cpp` and `oot_wide.py`. - Add `test_wide_loading_fade.py`, compiling the real renderer policy and generated HUD patch with authored state, in the existing wide-shape fixture style. - Document the icon trade-off and add six rows to `docs/tested-configurations.md`, including the unsuccessful input routes. No new shader, runtime switch, host interface, pane hook or generated game code is included. The pause/map centring, wheel background, event fades and inactive 16:9 projection are deliberately unchanged. ## How it was tested **Environment:** Arch Linux, kernel 7.2.15-1-omarchy; Intel Core i9-13900K; NVIDIA RTX 4090, driver 615.78.08. Base `51da0464`, mask-only change `7dc0773b`, 2026-11-12. The same host executable was used before and after. Its base build includes dormant readback diagnostics (`c372e16e`, not enabled); the changed renderer was built from this branch. No Xbox UI mod or overlay was used. **Rebase:** final head `d2e37d47` is on `e2057b2b`. The one end-of-task rebase was conflict-free. The fade regression was rerun with GCC and Clang and passes; its generated unit is byte-identical to the Windows-syntax-checked unit. Upstream added AMD Anti-Lag frame-generation tagging, not a change to the fade policy. **The game build/captures and full-suite result below are from before that rebase; the rebased game was not rebuilt or run.** The reporter's log distinguishes **3840×2160 output setting**, **4432×1856 game picture**, **3328×1392 scene**, and a **3440×1440 swapchain**. The reproduction uses the first three, with `HOS_ASPECT=2.3889` and `HOS_WIDE_MAX_PX=8294400`; it does not pretend that a windowless run has the reporter's swapchain. Both 21:9 captures are 2216×928, preserving the game picture's shape. ### Game-free checks - GCC 16.2.1 and Clang 23.1.3: the new test compiles and fails on the pre-fix sources with **12 FAIL lines each**: four keep-mask checks and eight loading-projection components. With the change: ```text PASS: loading/event fades and wheel stay full width; pause/HUD centring, sides and 16:9 unchanged ``` - Existing wide-shape, save-picture and independent-scaler tests pass with both compiler families. - An initial test-only chained volatile assignment was rejected by both C++20 compilers. It was split into separate stores before collecting the runtime negative control; no warning was suppressed. - Full Linux suite, pinned Python 3.12.16: ```text 476 passed, 0 failed, 20 skipped in 164 s (linux, 8 at once) ``` - The final generated C++ fixture passes Windows-target syntax compilation with its own `-std=c++20 -O2 -Wall -Wextra -Werror` flags. The local preflight had an incorrect C/libc++ include order; its captured source was compiled with that order corrected. **This is not Windows execution.** - Renderer builds and links. A separate full-file `-Wall -Wextra` check reports 11 missing-initializer warnings at unchanged Vulkan structure initializers, none at the mask change. No fixture core dumps were produced. ### Game captures All captures used the real scale pass (`RX_VK_SHOT_FX=1`). The bridge's named raw screenshots bypass that pass and were not used to judge the blur. These were ordinary runs on a busy PC, not exclusive timing runs; **no fps, latency or performance claim is made**. | Run | Picture | Place and result | Driver faults | |---|---|---|---| | Before, centred blurred sides | 4432×1856 | Load into Hyrule Field: only the middle fades, with bright side strips. Pause and actual Map centred. | 0 | | Before, 16:9 | 3840×2160 | Loading fade fills the frame; sharp icon; pause checked. The attempted Map capture was the first-use tapestry, so it is not counted as a Map result. | 0 | | Mask only, centred blurred sides | 4432×1856 | Hyrule Field fade fills the width; pause/Map remain centred. Transition to Kokiri Forest completed. Loading icon becomes a dim blur at the side. | 0 | | Before, corrected 16:9 route | 3840×2160 | Loading fade, sharp icon, pause and actual Map checked; transition to Kokiri Forest completed. | 0 | | Mask only, 16:9 | 3840×2160 | Same fade, icon proportions/position, pause and Map; transition completed. | 0 | | Mask only, black bars then Full width | 4432×1856 | Both fades cover the width. Black bars hide the loading icon; Full width shows it sharply but stretched. Black-bar pause/Map remain centred. Both requested transitions completed. | 0 | Retained input failures: one initial capture helper used a relative count with an absolute-frame wait; the first 16:9 route dismissed the tapestry but did not leave its menu; the extended mode check used an unrecognized setting key. Each stopped on its error, and no unreached step is counted as evidence. None was a game crash or device loss. ![Six panels at the reporter's picture size: the loading fade before (only the middle fades) and with the mask change (the whole picture), the pause menu and the Map unchanged](https://gist.githubusercontent.com/mooki000/b86bd3f35969803fb032f71f035844f8/raw/d4ca78fce40093c2b2404f8082505ce6d7fbe195/hookshot-wide-loading-fade-21x9.jpg) six inspected panels: before/mask-only loading fade, pause and Map. Same save, picture size and settings; the loading frames compare coverage, not identical fade opacity or frame numbers. ![The loading icon, 2x crops: sharp inside the middle 16:9 before; a dim blur at the outer edge with the mask change](https://gist.githubusercontent.com/mooki000/b86bd3f35969803fb032f71f035844f8/raw/d4ca78fce40093c2b2404f8082505ce6d7fbe195/hookshot-wide-loading-fade-icon-risk.jpg) identical bottom-right crops, enlarged 2×: the original sharp icon inside the central 16:9 versus the mask-only icon blurred near the outer edge. This is why the proposal remains a draft. ![16:9 before and after: the fade, the sharp icon, the pause menu and the Map are unchanged](https://gist.githubusercontent.com/mooki000/b86bd3f35969803fb032f71f035844f8/raw/d4ca78fce40093c2b2404f8082505ce6d7fbe195/hookshot-wide-loading-fade-16x9.jpg) six inspected panels at the same 3840×2160 picture size: before/after loading fade and sharp icon, pause and actual Map. Captures are 1920×1080. ![The icon in each mode: before; mask change with blurred sides (a dim blur); with black bars (hidden); Full width (sharp but stretched)](https://gist.githubusercontent.com/mooki000/b86bd3f35969803fb032f71f035844f8/raw/d4ca78fce40093c2b2404f8082505ce6d7fbe195/hookshot-wide-loading-fade-icon-modes.jpg) equal 2× crops showing the original centred icon, the mask-only blurred icon, the hidden black-bars icon and the sharp-but-stretched Full-width reference. A separate raw image immediately before the black-bars capture still shows the icon: the side pass, not a missing draw, removes it. All six runs reported zero fatal/crash/device-loss/kernel-driver-fault lines. The first 16:9 run recorded one virtual-core escape at startup; the other five recorded none. The last three capture routes passed their checks and quit normally. Full width's unchanged mask is covered by the compiled before/after checks; its runtime image is a reference from the changed binary, not a separate pre-fix Full-width run. ## Not tested / not shown - Windows gameplay, AMD/Intel/Steam Deck, physical ultrawide presentation, HDR and frame-generated output. (The new test itself ran on Windows: both `windows-2022` source-check parts passed on [a run on my fork](https://github.com/mooki000/Hookshot/actions/runs/43258766902) at this head before the pull request was opened.) - A complete fade-pane-only fix that preserves the icon's original position and shape has not been implemented or demonstrated. ## Why not simply keep the icon centred too? The existing HUD hook changes one projection for the whole canvas, not an individual pane. Clearing bit 3 centres both the icon and fade; keeping it widens both. The side pass receives only margins and its mode, not a pane identity or loading opacity. A complete solution needs a new game-side hook that widens only the fade pane, or a new loading-state/opacity hand-off to the side-fill path and its shader. Exempting the outer pixels alone would still leave a stretched, displaced icon. This proposal does not add an unproved pane patch or a dark-pixel heuristic. ## Risks The loading icon regression is real in both centred modes: it is a dim blur with blurred sides and completely hidden by black bars. Full width shows it sharply but stretched; 16:9 keeps its original position and proportions. The mask-only change should not be merged as if it preserves the loading indicator. ## Overlaps The pre-rebase open-PR comparison found 57 PRs touching one of the initial four changed files. The shared-code PRs #1488, #1979, #2015, #2017, #2172, #2261, #2262, #2531, #2658 and #2665 merged cleanly in that comparison; no conflict hunk was reported. No competing loading-fade repair was found. The final branch gate passes against `e2057b2b`. ## Questions for the maintainers and reporter 1. Do you prefer the full-width fade with this icon trade-off, or the current behaviour until the fade pane can be handled separately? 2. @EjianYT, does your real-window build of the mask correction also blur or hide the icon in the centred modes, or does it differ from these captures without the UI mod? Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]> <!-- github-sync:item:2851 -->
Keep the loading-fade canvas in the full-width mask in both centred menu modes. The pause background still centres, and Full width and 16:9 projections stay unchanged. Credit EjianYT for identifying the loading-fade bit in issue #2220.

Compile the real mask policy and HUD patch in a game-free regression fixture. Document the shared loading-icon layout trade-off.

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>
Keep the mask-only proposal a draft: real scale-pass captures show that centred blurred sides blur the loading icon and black bars hide it. Record the six runs, including unsuccessful input helpers and the unchanged 16:9 controls.

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>
github-sync commented 2026-11-12 17:59:58 +00:00
Author
Owner

@hckrman101 commented on gpu/vk/vk_framegen.cpp line 544 on GitHub · 2026-11-12 17:19 UTC · original

The corrected meaning of bit 3 looks right. #2843 reads it the same way. The problem is that this change widens everything on that screen, not just the fade (see the review body). If the fade pane is widened on its own, this mask can stay at 0xa. Then it's only the comment that needs fixing.

> [@hckrman101](https://github.com/hckrman101) commented on `gpu/vk/vk_framegen.cpp` line 544 on GitHub · 2026-11-12 17:19 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2851#discussion_r4553843660) The corrected meaning of bit 3 looks right. #2843 reads it the same way. The problem is that this change widens everything on that screen, not just the fade (see the review body). If the fade pane is widened on its own, this mask can stay at `0xa`. Then it's only the comment that needs fixing. <!-- github-sync:rc:4553843660 -->
github-sync commented 2026-11-12 17:59:58 +00:00
Author
Owner

@hckrman101 commented on readme.md line 445 on GitHub · 2026-11-12 17:19 UTC · original

Main now has an Ultrawide menus bullet in the option list, and a clean merge would put this one beside it. Please merge the two. If the icon gets fixed, this paragraph mostly goes away: a known regression reads oddly in user docs.

> [@hckrman101](https://github.com/hckrman101) commented on `readme.md` line 445 on GitHub · 2026-11-12 17:19 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2851#discussion_r4553843682) Main now has an **Ultrawide menus** bullet in the option list, and a clean merge would put this one beside it. Please merge the two. If the icon gets fixed, this paragraph mostly goes away: a known regression reads oddly in user docs. <!-- github-sync:rc:4553843682 -->
github-sync commented 2026-11-12 17:59:59 +00:00
Author
Owner

@hckrman101 commented on tests/test_wide_loading_fade.py line 63 on GitHub · 2026-11-12 17:19 UTC · original

Nit: take these from wide.KEEP_VTABLES (you already load the module) rather than copying the addresses. That way the fixture can't drift from oot_wide.py. A second nit: the grace-period and wheel checks near the end bump failures without printing anything, so a failure there exits 1 silently. A FAIL: line like the others would help.

> [@hckrman101](https://github.com/hckrman101) commented on `tests/test_wide_loading_fade.py` line 63 on GitHub · 2026-11-12 17:19 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2851#discussion_r4553843692) Nit: take these from `wide.KEEP_VTABLES` (you already load the module) rather than copying the addresses. That way the fixture can't drift from `oot_wide.py`. A second nit: the grace-period and wheel checks near the end bump `failures` without printing anything, so a failure there exits 1 silently. A `FAIL:` line like the others would help. <!-- github-sync:rc:4553843692 -->
github-sync commented 2026-11-12 17:59:59 +00:00
Author
Owner

@hckrman101 reviewed on GitHub · 2026-11-12 17:19 UTC · original

Thanks for this, and for the careful write-up. The before/after pictures at the reporter's 4432x1856 picture are exactly what I wanted to see: the bug is shown, and shown gone. The icon crops make the cost just as clear.

Root cause and layer. I agree with the diagnosis, and #2843 independently names keep bit 3 the same way. The centred modes cleared bits 1 and 3, so the HUD site squeezed the whole loading screen into the middle 16:9. While the fade runs, the world is still drawn, so no_world is false and the side pass is off. That leaves the scene bright in the strips. wide_sides() only lives in vk_framegen.cpp because the scale pass does. It runs on every present with frame generation Off too (that path goes through wide_sides_possible() in vk_replay.cpp), so your FG-off captures cover the normal case. The mask is the right switch for the fade. The trouble is that it is whole-canvas.

Side effects. I don't see any beyond the icon. 16:9 is untouched because the squeeze is 0 there, and your 16:9 pair confirms it. The HUD, text boxes, the cursor and the map are ordinary canvases and stay squeezed. The wheel background (bit 0), event fades (bit 2) and movies keep their current behaviour. The only thing that stops being pillarboxed is the loading screen itself, which takes its icon with it.

The icon trade-off. This is the reason I wouldn't take the mask-only change as it stands. In the default mode (blurred sides) the loading indicator turns into a smudge at the far edge, and with black bars it vanishes completely. I'd rather fix it per pane than ship that. #2843 (Extended) adds per-pane hooks, and one of its rules already draws a pane that spans the canvas at the screen's width. Here's the version I'd like: keep the loading screen squeezed in the centred modes, and widen only its spanning fade pane through that path. The icon then stays sharp in the middle 16:9, and the fade covers the strips. Please check how Extended already treats this screen once #2843 is in, because it may need very little. If that turns out heavy, say so here and we can talk about the mask change as a stopgap.

Test. It's a good check. It compiles the real wide_sides() and the real HUD patch around authored state, so it tests behaviour rather than matching text. It fails on the old mask (four keep-mask checks plus the loading-canvas projection), and it runs and passes in all four source-check parts at this head. Two nits are inline.

Conflicts and ordering.

  • docs/tested-configurations.md conflicts with main: rows landed at the top of the table today. Keep both sets.
  • readme.md merges cleanly but would end up with two Ultrawide menus bullets. Main now has one in the option list, which already says loading screens get the same sides. Please fold your note into that one.
  • vk_framegen.cpp merges cleanly with today's #2262/#2658.
  • #2843 changes the same lines (the kMenus line and its comment, and the keep-bit comment in oot_wide.py). It also renumbers the modes: Extended becomes 1 and the centred modes 2/3 with side = mode - 1. This test's side == mode and mode ? 0xd : 0xf expectations would break once it lands. Since the pane-level route would build on #2843 anyway, I'd land #2843 first and rebase this onto it.

To take it out of draft: a fix that keeps the icon (or an agreed stopgap), rebased on main and on #2843, the readme note folded in, and before/after crops of the icon and fade at 21:9 again. A run from @EjianYT on their real 3440x1440 window would be a welcome extra.

> [@hckrman101](https://github.com/hckrman101) reviewed on GitHub · 2026-11-12 17:19 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2851#pullrequestreview-5878911166) Thanks for this, and for the careful write-up. The before/after pictures at the reporter's 4432x1856 picture are exactly what I wanted to see: the bug is shown, and shown gone. The icon crops make the cost just as clear. **Root cause and layer.** I agree with the diagnosis, and #2843 independently names keep bit 3 the same way. The centred modes cleared bits 1 and 3, so the HUD site squeezed the whole loading screen into the middle 16:9. While the fade runs, the world is still drawn, so `no_world` is false and the side pass is off. That leaves the scene bright in the strips. `wide_sides()` only lives in `vk_framegen.cpp` because the scale pass does. It runs on every present with frame generation Off too (that path goes through `wide_sides_possible()` in `vk_replay.cpp`), so your FG-off captures cover the normal case. The mask is the right switch for the fade. The trouble is that it is whole-canvas. **Side effects.** I don't see any beyond the icon. 16:9 is untouched because the squeeze is 0 there, and your 16:9 pair confirms it. The HUD, text boxes, the cursor and the map are ordinary canvases and stay squeezed. The wheel background (bit 0), event fades (bit 2) and movies keep their current behaviour. The only thing that stops being pillarboxed is the loading screen itself, which takes its icon with it. **The icon trade-off.** This is the reason I wouldn't take the mask-only change as it stands. In the default mode (blurred sides) the loading indicator turns into a smudge at the far edge, and with black bars it vanishes completely. I'd rather fix it per pane than ship that. #2843 (Extended) adds per-pane hooks, and one of its rules already draws a pane that spans the canvas at the screen's width. Here's the version I'd like: keep the loading screen squeezed in the centred modes, and widen only its spanning fade pane through that path. The icon then stays sharp in the middle 16:9, and the fade covers the strips. Please check how Extended already treats this screen once #2843 is in, because it may need very little. If that turns out heavy, say so here and we can talk about the mask change as a stopgap. **Test.** It's a good check. It compiles the real `wide_sides()` and the real HUD patch around authored state, so it tests behaviour rather than matching text. It fails on the old mask (four keep-mask checks plus the loading-canvas projection), and it runs and passes in all four source-check parts at this head. Two nits are inline. **Conflicts and ordering.** - `docs/tested-configurations.md` conflicts with main: rows landed at the top of the table today. Keep both sets. - `readme.md` merges cleanly but would end up with two **Ultrawide menus** bullets. Main now has one in the option list, which already says loading screens get the same sides. Please fold your note into that one. - `vk_framegen.cpp` merges cleanly with today's #2262/#2658. - #2843 changes the same lines (the `kMenus` line and its comment, and the keep-bit comment in `oot_wide.py`). It also renumbers the modes: Extended becomes 1 and the centred modes 2/3 with `side = mode - 1`. This test's `side == mode` and `mode ? 0xd : 0xf` expectations would break once it lands. Since the pane-level route would build on #2843 anyway, I'd land #2843 first and rebase this onto it. **To take it out of draft:** a fix that keeps the icon (or an agreed stopgap), rebased on main and on #2843, the readme note folded in, and before/after crops of the icon and fade at 21:9 again. A run from @EjianYT on their real 3440x1440 window would be a welcome extra. <!-- github-sync:r:5878911166 -->
github-sync commented 2026-11-12 17:59:59 +00:00
Author
Owner

@mooki000 commented on GitHub · 2026-11-12 17:21 UTC · original

Thanks for the thorough review, and for the merge of main. Agreed on all of it: the mask is the wrong grain, and a loading indicator that smudges or vanishes is not something to ship.

What I will do:

  • Pane level, on #2843. Keep the loading screen squeezed in the centred modes and widen only its spanning fade pane through #2843's per-pane path, so the mask stays 0xa and only its comment is corrected. I am reading #2843's head now to see how Extended already treats this screen and will say here whether it needs little or turns out heavy, before writing more than a prototype.
  • Order. #2843 first, then this rebased onto it, with the test's side == mode and mode ? 0xd : 0xf expectations moved to the renumbered modes.
  • The two test nits: the addresses from wide.KEEP_VTABLES, and a FAIL: line for the grace-period and wheel checks.
  • The readme note folded into main's Ultrawide menus bullet; if the icon stays sharp the "known cost" paragraph goes.
  • The table rows: both sets kept.
  • Pictures again at 21:9, icon and fade crops before and after, for the new version.

It stays a draft until then.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 17:21 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2851#issuecomment-6830774047) Thanks for the thorough review, and for the merge of main. Agreed on all of it: the mask is the wrong grain, and a loading indicator that smudges or vanishes is not something to ship. What I will do: - **Pane level, on #2843.** Keep the loading screen squeezed in the centred modes and widen only its spanning fade pane through #2843's per-pane path, so the mask stays `0xa` and only its comment is corrected. I am reading #2843's head now to see how Extended already treats this screen and will say here whether it needs little or turns out heavy, before writing more than a prototype. - **Order.** #2843 first, then this rebased onto it, with the test's `side == mode` and `mode ? 0xd : 0xf` expectations moved to the renumbered modes. - **The two test nits**: the addresses from `wide.KEEP_VTABLES`, and a `FAIL:` line for the grace-period and wheel checks. - **The readme note** folded into main's **Ultrawide menus** bullet; if the icon stays sharp the "known cost" paragraph goes. - **The table rows**: both sets kept. - **Pictures again** at 21:9, icon and fade crops before and after, for the new version. It stays a draft until then. <!-- github-sync:c:6830774047 -->
github-sync commented 2026-11-12 17:59:59 +00:00
Author
Owner

@ozwaldorf commented on GitHub · 2026-11-12 17:40 UTC · original

@hckrman101 #2843 should correctly handle these transitions already in extended mode! It doesn't fix them on the centered layouts though, but that should be fairly easy to bridge over support for

> [@ozwaldorf](https://github.com/ozwaldorf) commented on GitHub · 2026-11-12 17:40 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2851#issuecomment-6831234951) @hckrman101 #2843 should correctly handle these transitions already in extended mode! It doesn't fix them on the centered layouts though, but that should be fairly easy to bridge over support for <!-- github-sync:c:6831234951 -->
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
1 participant
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!2851
No description provided.