Show the middle 16:9 in the save picture on ultrawide #1183

Merged
janvitos merged 3 commits from fix/ultrawide-save-picture into main 2026-11-09 19:24:07 +00:00
janvitos commented 2026-11-09 16:33:18 +00:00 (Migrated from github.com)

Fixes #952.

What

The pause menu draws the whole picture into the save's 543×306 JPEG through the game's viewport helper. When the window is wider than 16:9, the whole wide picture was squeezed into that texture.

nx2/oot_wide.py gets one more site, main+0x284824. That is the helper's SetViewport call, right after its address is loaded and before the blr. At that site the viewport is widened by the picture's ratio over 16:9 and centred, so the middle 16:9 fills the texture, the same view a 16:9 window would save. The scissor still clips to the texture.

  • Only one viewport changes: a 0, 0, 543, 306 viewport, and only past 16:9.
  • 16:9 is untouched: there hos_wide_squeeze is 0, so the game runs as shipped.
  • Other game versions: the site's instruction word is checked like the others, so a different version leaves the site out.

How it was tested

  • Machine: Linux (CachyOS), RTX 4070 SUPER, NVIDIA 620.70.06.
  • Build: a full game build with this change on 8b2e1d3.
  • Ultrawide: at 21:9 (3440×1440, HOS_ASPECT=2.3889), saving from the pause menu now gives a picture that matches a 16:9 save from the same spot. Before, it was the whole 21:9 picture squeezed into 16:9.
  • 16:9: saves are unchanged.
  • New test: tests/test_oot_wide_save_picture.py compiles the site's C exactly as the lifter places it. It checks 21:9, 32:9 and 2560×1080 against the expected width (within 1 px) and centring. It also checks that a 16:9 window and every other viewport stay as the game set them.
  • Suite: on this branch (rebased on 1f6fbdb), python -I -B tests/run_all.py gives 132 passed, 0 failed, 17 skipped on Linux.

🤖 Generated with Claude Code

Fixes #952. ## What The pause menu draws the whole picture into the save's 543×306 JPEG through the game's viewport helper. When the window is wider than 16:9, the whole wide picture was squeezed into that texture. `nx2/oot_wide.py` gets one more site, main+0x284824. That is the helper's `SetViewport` call, right after its address is loaded and before the `blr`. At that site the viewport is widened by the picture's ratio over 16:9 and centred, so the middle 16:9 fills the texture, the same view a 16:9 window would save. The scissor still clips to the texture. - **Only one viewport changes:** a `0, 0, 543, 306` viewport, and only past 16:9. - **16:9 is untouched:** there `hos_wide_squeeze` is 0, so the game runs as shipped. - **Other game versions:** the site's instruction word is checked like the others, so a different version leaves the site out. ## How it was tested - **Machine:** Linux (CachyOS), RTX 4070 SUPER, NVIDIA 620.70.06. - **Build:** a full game build with this change on 8b2e1d3. - **Ultrawide:** at 21:9 (3440×1440, `HOS_ASPECT=2.3889`), saving from the pause menu now gives a picture that matches a 16:9 save from the same spot. Before, it was the whole 21:9 picture squeezed into 16:9. - **16:9:** saves are unchanged. - **New test:** `tests/test_oot_wide_save_picture.py` compiles the site's C exactly as the lifter places it. It checks 21:9, 32:9 and 2560×1080 against the expected width (within 1 px) and centring. It also checks that a 16:9 window and every other viewport stay as the game set them. - **Suite:** on this branch (rebased on 1f6fbdb), `python -I -B tests/run_all.py` gives 132 passed, 0 failed, 17 skipped on Linux. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 (Migrated from github.com) reviewed 2026-11-09 17:58:23 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks, this is a tidy fix for #952. What I checked:

  • hos_wide_squeeze is set in host/hos_main.c as (16/9) / (ow/oh) from the real picture size, and stays 0 for 16:9 or anything within 1% of it, so the site is a no-op there, as described.
  • Negative x is fine downstream. nvnCommandBufferSetViewport reads (int)c->x[1] into a float. The Vulkan replay passes the viewport through unclamped (negative x is valid within viewportBoundsRange), and the scissor is still clamped to the 543x306 target. The rx/WebGPU path does the viewport in its rescale uniform, so it's fine there too.
  • Writing w1/w3 as zero-extended 32-bit values matches what a mov w1, ... would leave. x8 isn't touched, and the instruction word check keeps other game versions out.
  • At 3440x1440 the width comes out as 730 with x = -93, so the left and right margins are 93 and 94 px. That's centred.
  • The test compiles the exact THUMB string from the module, not a copy, so it really exercises what the lifter places. No overlap with other open PRs that I could find.

One thing to fix:

  • The test isn't registered in tests/run_all.py's ARGS, so it runs with --cc cc. CI runs run_all.py --strict on windows-2022 as well, and there's normally no cc there, so this would likely fail with FileNotFoundError on the Windows job. The other C fixture tests are listed as "tests/test_...": ["--cc", "gcc"]. With that entry the runner also skips the test cleanly (or fails it under --strict) when the compiler is missing. Adding the entry, and/or defaulting to gcc like test_text_speed.py and test_input_background.py, should be enough.

Nit: the reflowed docstring line in nx2/oot_wide.py (line 6) is about 170 chars now. It might be worth rewrapping it like the lines around it.

Thanks, this is a tidy fix for #952. What I checked: - `hos_wide_squeeze` is set in host/hos_main.c as `(16/9) / (ow/oh)` from the real picture size, and stays 0 for 16:9 or anything within 1% of it, so the site is a no-op there, as described. - Negative x is fine downstream. `nvnCommandBufferSetViewport` reads `(int)c->x[1]` into a float. The Vulkan replay passes the viewport through unclamped (negative x is valid within `viewportBoundsRange`), and the scissor is still clamped to the 543x306 target. The rx/WebGPU path does the viewport in its rescale uniform, so it's fine there too. - Writing w1/w3 as zero-extended 32-bit values matches what a `mov w1, ...` would leave. x8 isn't touched, and the instruction word check keeps other game versions out. - At 3440x1440 the width comes out as 730 with x = -93, so the left and right margins are 93 and 94 px. That's centred. - The test compiles the exact `THUMB` string from the module, not a copy, so it really exercises what the lifter places. No overlap with other open PRs that I could find. One thing to fix: - **The test isn't registered in `tests/run_all.py`'s `ARGS`, so it runs with `--cc cc`.** CI runs `run_all.py --strict` on windows-2022 as well, and there's normally no `cc` there, so this would likely fail with FileNotFoundError on the Windows job. The other C fixture tests are listed as `"tests/test_...": ["--cc", "gcc"]`. With that entry the runner also skips the test cleanly (or fails it under --strict) when the compiler is missing. Adding the entry, and/or defaulting to `gcc` like test_text_speed.py and test_input_background.py, should be enough. Nit: the reflowed docstring line in nx2/oot_wide.py (line 6) is about 170 chars now. It might be worth rewrapping it like the lines around it.
@ -0,0 +1,101 @@
"""Compile nx2/oot_wide.py's save-picture patch (issue 952) around authored registers.
hckrman101 (Migrated from github.com) commented 2026-11-09 17:58:24 +00:00

This needs an ARGS entry in tests/run_all.py (e.g. ["--cc", "gcc"]). Otherwise run_all.py --strict on the windows-2022 job calls cc, which normally isn't on PATH there.

This needs an `ARGS` entry in tests/run_all.py (e.g. `["--cc", "gcc"]`). Otherwise `run_all.py --strict` on the windows-2022 job calls `cc`, which normally isn't on PATH there.
janvitos commented 2026-11-09 18:16:18 +00:00 (Migrated from github.com)

Thanks for the review. Pushed in ef362de:

  • tests/run_all.py now lists tests/test_oot_wide_save_picture.py with ["--cc", "gcc"], and the test's own default is gcc, like test_text_speed.py. Without gcc on PATH the runner now skips it ("not found here: gcc"), or fails it under --strict.
  • Rewrapped the long docstring line in nx2/oot_wide.py.
Thanks for the review. Pushed in ef362de: - `tests/run_all.py` now lists `tests/test_oot_wide_save_picture.py` with `["--cc", "gcc"]`, and the test's own default is `gcc`, like `test_text_speed.py`. Without gcc on PATH the runner now skips it ("not found here: gcc"), or fails it under `--strict`. - Rewrapped the long docstring line in `nx2/oot_wide.py`.
hckrman101 (Migrated from github.com) approved these changes 2026-11-09 18:18:52 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks, ef362de covers it: the test is listed in run_all.py with ["--cc", "gcc"] like the other C fixture tests and defaults to gcc itself, and the docstring is rewrapped. LGTM.

Thanks, ef362de covers it: the test is listed in run_all.py with `["--cc", "gcc"]` like the other C fixture tests and defaults to gcc itself, and the docstring is rewrapped. LGTM.
janvitos commented 2026-11-09 19:06:52 +00:00 (Migrated from github.com)

Moved the run_all.py entry in 5a03e8b: upstream's new test_motion.py line landed where it was, so this PR had a conflict. It merges cleanly with main again.

Moved the `run_all.py` entry in 5a03e8b: upstream's new `test_motion.py` line landed where it was, so this PR had a conflict. It merges cleanly with main again.
hckrman101 (Migrated from github.com) approved these changes 2026-11-09 19:18:03 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for sorting the conflict. Just the run_all.py entry moved, still --cc gcc. LGTM.

Thanks for sorting the conflict. Just the `run_all.py` entry moved, still `--cc gcc`. LGTM.
Commenting is not possible because the repository is archived.
No description provided.