Repository navigation
test: test_native_astc_texture_count builds on Windows, where the CRT marks scanf deprecated - #2397
Merged
hckrman101 merged 1 commit intoNov 11, 2026
Conversation
… marks scanf deprecated The test from NotRexed#2288 compiles its fixture with -Wall -Wextra -Werror and reads its cases with std::scanf. With clang and the Windows CRT's headers scanf is marked deprecated, so the fixture does not compile there and the game-free checks fail on windows-2022 (part 1/2): fixture.cpp:13:17: error: 'scanf' is deprecated: This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. NotRexed#2288's own workflows had not run before it was merged (they waited for an approval), so this was first seen on a later pull request's run. The fixture now defines _CRT_SECURE_NO_WARNINGS ahead of its includes, as the other test sources that use these functions do. Nothing else changes. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hckrman101
approved these changes
Nov 11, 2026
hckrman101
left a comment
hckrman101
left a comment
Collaborator
There was a problem hiding this comment.
Looks good. One line added to the fixture, no assertion, case or source change, so the test is not weakened (182 cases still run). The define sits before the includes, the same way tests/test_slow_frame_limit.py does it in its inline fixture and the .cpp tests (png_publication_test.cpp, capture_dimensions_test.cpp, ...). Python source checks (windows-2022, part 1/2) now passes on this head, which is the proof the description asked for. The appimage failure is in the System packages step, unrelated to a test-only change. Part 2/2 and the Linux jobs were still pending when I looked.
Contributor
Author
|
Thanks for the review. Confirmed from this head’s Windows part 1/2 log: |
This was referenced Nov 11, 2026
Contributor
|
Testing pass on
|
This was referenced Nov 11, 2026
Merged
Merged
hckrman101
pushed a commit
that referenced
this pull request
Nov 11, 2026
…#2354) **What this is for:** frame times from headless runs, the kind scripted tests and measurements use. `RX_VK_FRAME_LOG=<csv>` ("A CSV line per present: frame, seconds, frame time in ms", docs/development.md) writes nothing in a headless run (`HOS_HEADLESS`): the file is never even created. Fixes #2353. **What is measured:** on Linux (RTX 4090, NVIDIA 615.78.08) a headless run with the change wrote 6480 lines over 108.9 s, and its first interval equals the game's own present trace of the same run; none of 23 headless runs of main left a file; a game-free test that fails on main; the whole of the checks. **Not measured:** a windowed run with the change (that path calls the same code at the same place as before), Windows game runtime, AMD, the Steam Deck under gamescope. ## What changed - `gpu/vk/vk_replay.cpp`: the writer was only in `present_out`, after `vkQueuePresentKHR`. Without a window a present copies the picture into the offscreen image and `do_present` returns before `present_image`, so it never got there. The writer is now `frame_log_present()`, unchanged (same file, header, columns and flush every 60 lines), called where it was and at the end of a headless `do_present`, once the copy into the offscreen image is submitted. - Frame generation is off without a window (`framegen_on`), so a headless run writes one line per game frame, as a window does without frame generation. - The times are the same steady clock as before. A headless run has no swapchain to wait on, so they show the game's and the replay's pace without the display's (V-Sync, the compositor). - `tests/test_frame_log_headless.py` (game-free): compiles the actual `frame_log_present` and checks its CSV header, sequence numbers, elapsed/interval times, and disabled output. It does not infer execution from call-site spelling. - `docs/development.md`: documents headless CPU intervals and the absence of swapchain/display waits. ## How it was tested Latest refresh, 2026-11-11: head `e3119ce0` on `47ed9f14`, including the merged #2397 Windows ASTC fixture repair. The `present_native_pq` conflict was resolved by retaining main’s moved declaration and this PR’s logger. The compiled CSV writer passes with GCC and Clang. No new game run or full local suite was made for this refresh; [Source-regression CI](https://github.com/NotRexed/Hookshot/actions/runs/43093270209) now passes on this head: both Windows and both Ubuntu shards. RTX 4090, NVIDIA 615.78.08, Arch Linux; headless (`HOS_HEADLESS=1`, the log says `vk: no window: presents render offscreen (1920x1080)`), from a fresh profile, no input, 120 s, muted, `RX_VK_FRAME_LOG` and `HOS_PRESENT_TRACE` set. - Main: none of 23 such headless runs left a frame log (the switch was set in every one). - With this change: `frames.csv` holds the header and 6480 lines, frames 0 to 6479 over 108.9 s, times rising from 0; the frame times average 16.80 ms (59.5 fps; the last 30 s 59.9 fps) with the game at its 60 cap through the intro movie and the title screen (pictures of the run looked at). The game's own present trace of the same run has the same first interval (43.28 ms) and 6432 presents over 108.1 s. (Not a frame-rate claim: other work ran beside it.) - That run was of this change on main bdfe09c (commit 7da36252). Rebased onto 49f75dc the change is the same apart from one blank line. Added 2026-11-11 16:27 UTC: the rebased build (cc36308 on 49f75dc, `hookshot v2.1.0-rc.3-54-gcc363089`) ran the same way, 150 s: `frames.csv` holds the header and 5880 lines, frames 0 to 5879 over 98.4 s, times rising from 0; its first interval (32.909 ms) equals the game's own present trace of that run (32,908,677 ns); no FATAL or device-lost line, no driver line in the kernel log. (Load average 22 falling to 9: not a frame-rate claim.) - Updated 2026-11-11: rebased onto `7759a6f3`, head `5ce5970f`. #1995 changed `present_out` to return `bool`; the old test pinned its full signature. The source-text call-site assertions are removed rather than repinned. The compiled writer check passes with GCC 16.2.1 and Clang 23.1.3: `PASS: frame log CSV header, sequence, elapsed/interval times, and disabled output`. Against main it fails because main has no extracted `frame_log_present` helper; that is not a headless integration test. Headless integration evidence is the two older game runs above. - On `5ce5970f`, `tests/run_all.py`: `385 passed, 2 failed, 10 skipped in 141 s`. `test_updater_asset_digest.py` cannot resolve the .NET 8 reference packs on this machine. `nx2/test_texture_phi_dominance.py` had a compiler killed by SIGKILL; its narrow rerun alone passed (`1 passed, 0 failed, 0 skipped in 20 s`). No second full suite was run. - Earlier suite on `cc363089` (load 57 to 113), `python -I -B tests/run_all.py`: `362 passed, 3 failed, 10 skipped in 193 s (linux, 32 at once)`. Two of the failures are this machine's and fail on main as well, each with a pull request of its own: `tests/test_updater_asset_digest.py` (only a .NET 10 SDK here, #2294) and `tests/test_wayland_libdecor_hint.py` (Clang 23, #2290). The third, `tests/test_display_pointer.py`, failed under that load at `reads > 0` (its reader thread had made no read yet when the check ended) and passes when run alone (3 of 3, at load 55). Added 2026-11-11: [Windows part 2/2](https://github.com/NotRexed/Hookshot/actions/runs/43072060527/job/130702262513) passes `tests/test_frame_log_headless.py`. The job fails only on the unrelated ASTC fixture’s `scanf` deprecation, fixed by #2397, which has now merged and is included in the latest refresh. This is Windows coverage of the compiled writer, not a game run. ## Not tested - No new game build/run after the subsequent rebases, including `47ed9f14`; the game runs above name their earlier heads. The renderer patch remains the same; #1995's surface-loss return stays before the existing window log position. - A windowed run of this branch: the window's path calls the same function at the same place as before; not run here. - Windows game runtime, AMD, the Steam Deck under gamescope (the switch's first use). ## Risks - A windowed run writes exactly what it wrote: the same function is called at the same place in `present_out`. - A headless line is written when the offscreen copy is submitted, not when a display shows it: there is no display. Its times are the game's and the replay's pace. Nobody should read them as presentation times, which the description of the switch does not promise either. - The file is opened at the first present, as before; a run without the switch pays one `getenv` at its first present and a null check after that. ## Overlaps Updated 2026-11-11 after the refresh: 49 open PRs touch one of this branch’s three files; one additional text conflict was reported, and its hunk was read. - #2325 (frame critical-path recorder / asynchronous presenter): its presenter-specific surface-loss and deferred-resize handling meets the old inline frame logger in `present_out`. Keep that handling and call the extracted logger once at the corresponding present boundary; do not restore a second inline writer. No combined runtime test was made. - The earlier `present_native_pq` declaration conflict is resolved in this rebase. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: GPT-6 Astra <noreply@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is for: main's game-free checks pass on Windows again. The test that came with my #2288 does not compile there, so
Python source checks (windows-2022, part 1/2)fails on every run since that merge. What is measured: the test with g++ and clang++ on Linux, on this branch. Not measured: Windows on my side: I have no Windows machine, so this pull request's own windows-2022 job is the proof, and I will confirm its result here.Problem
tests/test_native_astc_texture_count.pycompiles a small fixture with-Wall -Wextra -Werrorand reads its cases withstd::scanf. With clang and the Windows CRT's headersscanfis marked deprecated, which-Werrormakes an error:What changed
tests/test_native_astc_texture_count.py: the fixture defines_CRT_SECURE_NO_WARNINGSahead of its includes, as the other test sources that use these functions do (tests/capture_dimensions_test.cpp,tests/png_publication_test.cppand others). One line; no assertion, case or source file changes.How it was tested
Arch Linux, g++ 16.2.1 and clang++ 23.1.3, branch head
fc0157a1on main696d85b0:python -I -B tests/test_native_astc_texture_count.py --cxx g++and--cxx clang++:native ASTC texture count: 182 cases pass, as before the change.Not tested
Risks
Fixes #2396.
Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com