feat(vk): trace effective presentation geometry on changes #1457

Merged

Arkex-ai merged 4 commits from feat/presentation-geometry-trace into main

2026-11-10 07:48:26 +00:00

Arkex-ai commented

2026-11-10 03:39:01 +00:00

(Migrated from github.com)

Add opt-in RX_VK_PRESENT_LOG=1 geometry-change diagnostics: source/crop/content/output/destination, observed host aspect, sampled-view availability and selected direct/scaled/clear route. Filter/sharpness/frame-generation values are labeled requests. Deduplicate separately for game, interpolated and startup frames; normalize invalid aspects and cap output at256 changes plus one suppression notice. Unset/0 stays disabled. No rendering/settings policy or image-readback changes.

Validation:

Production presentation/trace blocks with CPU spies: disabled/0/enabled, crop/output/resize/options, actual route selection/fallback, independent frame kinds, invalid aspects and bounded output. The compiled main control lacks the enabled reporting; candidate passes. Six compiled mutations are detected.
Linux GCC and ASan/UBSan passed. Local strict suite189 passed/4 failed/16 skipped versus main188/4/16 (same local environment/toolchain failures). Combined seven-package source suite195/4/17.
Official integrated host/backend build passed. RTX5070Ti checkpoint smoke with FG2x/RCAS recorded normal/interpolated states,1720x720 ->3440x1369 ->restore, and scaled ->direct after setting sharpness0; normal exit. A route is a CPU recording decision, not evidence of driver present success or a performance gain.

Based on main a37ab47, whose advance from 9a2e80e changes only readme.md. Earlier runtime/integration manifests retain their actual source hashes; production/test source contents are unchanged by that rebase. Six source/test/doc files only, no game data or private logs. F01 integration needs keeping its aspect calculation and this adjacent route declaration; both consumer tests pass in the combined checkout. #1352 merges cleanly at the fetched head; #1025's capture plumbing is a different capability.

Updated at 1331051 on main b2e1c5a to address review and the newly merged presentation paths:

Request-only filter/sharpness/frame-generation changes no longer consume the finite geometry-log budget. Request values remain context when effective geometry or route changes; CPU controls cover 10,000 request changes and 1,000 actual slider updates.
Reuse the actual PresentAspect snapshot used for fitting; the host callback is sampled once. Empty-source presentation does not query it.
Preserve main's HDR conversion and report the effective HDR route alongside direct, scaled and clear routes. This does not change the rendering branch selection.

Validation: focused Windows LLVM and Linux GCC tests pass, including GCC ASan/UBSan. The official standalone backend and integrated host/backend builds pass. Native API11 run reached the copied Kokiri checkpoint, recorded separate game/made geometry, resized from1720x720 to3440x1369 (destination85,0 3270x1369 at43:18) and restored; normal exit0. Original checkpoint hashes unchanged. Guest translation was generated officially on8240446, then reused only after verifying main's sole intervening change is merged1496's duplicate menu formatter removal. No generated C was edited or published. HDR route behavior is CPU-tested; this run does not claim a physical HDR test. Shared source-suite repairs are tracked in1498.

Current-head validation

Current head 6b3b2b7, based on main 7d1562d. Earlier local/runtime results describe their recorded source hashes.

The standalone contribution patch is unchanged after the audited main refresh; exact-head build/source checks are refreshed below. Earlier runtime evidence remains tied to its recorded hash.

Fork runs are separate from upstream Actions approval; no maintainer approval gate is bypassed.

The current upscale diagnostic reports only its first dispatch, so a later resize does not show the crop, fitted destination or whether filtering actually returned an image. Frame timing CSV and image-readback capture serve different purposes. Add opt-in `RX_VK_PRESENT_LOG=1` geometry-change diagnostics: source/crop/content/output/destination, observed host aspect, sampled-view availability and selected direct/scaled/clear route. Filter/sharpness/frame-generation values are labeled requests. Deduplicate separately for game, interpolated and startup frames; normalize invalid aspects and cap output at256 changes plus one suppression notice. Unset/0 stays disabled. No rendering/settings policy or image-readback changes. Validation: - Production presentation/trace blocks with CPU spies: disabled/0/enabled, crop/output/resize/options, actual route selection/fallback, independent frame kinds, invalid aspects and bounded output. The compiled main control lacks the enabled reporting; candidate passes. Six compiled mutations are detected. - Linux GCC and ASan/UBSan passed. Local strict suite189 passed/4 failed/16 skipped versus main188/4/16 (same local environment/toolchain failures). Combined seven-package source suite195/4/17. - Official integrated host/backend build passed. RTX5070Ti checkpoint smoke with FG2x/RCAS recorded normal/interpolated states,1720x720 ->3440x1369 ->restore, and scaled ->direct after setting sharpness0; normal exit. A route is a CPU recording decision, not evidence of driver present success or a performance gain. Based on main `a37ab47`, whose advance from `9a2e80e` changes only readme.md. Earlier runtime/integration manifests retain their actual source hashes; production/test source contents are unchanged by that rebase. Six source/test/doc files only, no game data or private logs. F01 integration needs keeping its aspect calculation and this adjacent route declaration; both consumer tests pass in the combined checkout. #1352 merges cleanly at the fetched head; #1025's capture plumbing is a different capability.  Updated at `1331051` on main `b2e1c5a` to address review and the newly merged presentation paths: - Request-only filter/sharpness/frame-generation changes no longer consume the finite geometry-log budget. Request values remain context when effective geometry or route changes; CPU controls cover 10,000 request changes and 1,000 actual slider updates. - Reuse the actual `PresentAspect` snapshot used for fitting; the host callback is sampled once. Empty-source presentation does not query it. - Preserve main's HDR conversion and report the effective HDR route alongside direct, scaled and clear routes. This does not change the rendering branch selection. Validation: focused Windows LLVM and Linux GCC tests pass, including GCC ASan/UBSan. The official standalone backend and integrated host/backend builds pass. Native API11 run reached the copied Kokiri checkpoint, recorded separate game/made geometry, resized from1720x720 to3440x1369 (destination85,0 3270x1369 at43:18) and restored; normal exit0. Original checkpoint hashes unchanged. Guest translation was generated officially on8240446, then reused only after verifying main's sole intervening change is merged1496's duplicate menu formatter removal. No generated C was edited or published. HDR route behavior is CPU-tested; this run does not claim a physical HDR test. Shared source-suite repairs are tracked in1498.  ### Current-head validation Current head `6b3b2b7`, based on main `7d1562d`. Earlier local/runtime results describe their recorded source hashes. The standalone contribution patch is unchanged after the audited main refresh; exact-head build/source checks are refreshed below. Earlier runtime evidence remains tied to its recorded hash. - [Linux AppImage](https://github.com/Arkex-ai/Hookshot/actions/runs/42774724753): success. - [Windows builder](https://github.com/Arkex-ai/Hookshot/actions/runs/42774721322): success. - [Game-free source regressions](https://github.com/Arkex-ai/Hookshot/actions/runs/42774717583): success. Fork runs are separate from upstream Actions approval; no maintainer approval gate is bypassed.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 04:16:48 +00:00

hckrman101 (Migrated from github.com) left a comment

Looks good to me. Opt-in, bounded, and cheap: with RX_VK_PRESENT_LOG unset the present path pays one static bool check, and when it's on it's a ~100-byte compare per present with logf only on change. The statics are only touched under present_image_mu (all callers go through present_image, and the early returns are before the trace), so thread-safety is fine. Reading opts without a lock is the same as framegen_scale already does. Test is registered in run_all.py, and the fork's Linux/Windows builds are green.

A few small things, all nits:

#1339 / #1448: #1339 rewrites the two preserve_stretch/present_rect lines right above the new trace_route line, so whichever lands second will probably hit a small textual conflict. Easy to fix, and the test already handles the PresentAspect aspect{ marker. Once #1339 is in, it'd be nicer for the trace to log the aspect that was actually passed to present_rect (which #1339 zeroes when there's no src) than to call host->aspect_ratio() a second time. With #1448, blit_crop_to_rect clears instead of blitting when the crop is invalid, and that will still be logged as route=direct.
The filter/sharpness/framegen requests are part of the dedupe key, and the 256 budget is shared across kinds. So dragging the sharpness slider (counted twice with FG on) can use up most of the budget before the resize you actually wanted to see. Maybe keep the requests out of the key, or give them their own small budget. Not blocking, since the doc already says the output is bounded.
The test slices present_image between const bool preserve_stretch = / const PresentAspect aspect{ and if (src && framelog_on()), then compiles that slice against fakes. That checks the real code, but any rewording there breaks it. Fine for now, just worth knowing.

Looks good to me. Opt-in, bounded, and cheap: with `RX_VK_PRESENT_LOG` unset the present path pays one static bool check, and when it's on it's a ~100-byte compare per present with `logf` only on change. The statics are only touched under `present_image_mu` (all callers go through `present_image`, and the early returns are before the trace), so thread-safety is fine. Reading `opts` without a lock is the same as `framegen_scale` already does. Test is registered in run_all.py, and the fork's Linux/Windows builds are green. A few small things, all nits: - **#1339 / #1448:** #1339 rewrites the two `preserve_stretch`/`present_rect` lines right above the new `trace_route` line, so whichever lands second will probably hit a small textual conflict. Easy to fix, and the test already handles the `PresentAspect aspect{` marker. Once #1339 is in, it'd be nicer for the trace to log the aspect that was actually passed to `present_rect` (which #1339 zeroes when there's no `src`) than to call `host->aspect_ratio()` a second time. With #1448, `blit_crop_to_rect` clears instead of blitting when the crop is invalid, and that will still be logged as `route=direct`. - The filter/sharpness/framegen requests are part of the dedupe key, and the 256 budget is shared across kinds. So dragging the sharpness slider (counted twice with FG on) can use up most of the budget before the resize you actually wanted to see. Maybe keep the requests out of the key, or give them their own small budget. Not blocking, since the doc already says the output is bounded. - The test slices `present_image` between `const bool preserve_stretch =` / `const PresentAspect aspect{` and `if (src && framelog_on())`, then compiles that slice against fakes. That checks the real code, but any rewording there breaks it. Fine for now, just worth knowing.

gpu/vk/vk_replay.cpp

Nit: #1339 replaces the two lines just above, so expect a small merge conflict here. Also, once #1448 lands, blit_crop_to_rect can clear instead of blit on an invalid crop, and that case would still be traced as direct.

Nit: #1339 replaces the two lines just above, so expect a small merge conflict here. Also, once #1448 lands, `blit_crop_to_rect` can clear instead of blit on an invalid crop, and that case would still be traced as `direct`.

gpu/vk/vk_replay.cpp

Nit: since the requests are in the dedupe key, a sharpness slider drag gives one line per value (x2 with FG) out of the shared 256 budget. Consider dropping them from the key, or only logging them along with a geometry/route change.