fix(vk): report device loss throughout native presentation #1515

Merged
Arkex-ai merged 2 commits from fix/presentation-device-loss into main 2026-11-10 18:11:06 +00:00
Arkex-ai commented 2026-11-10 05:45:05 +00:00 (Migrated from github.com)

Rebased onto main d224dc06 and resolved the test-runner conflict by moving this test's arguments into RUN_ALL_ARGS, following #1822. The production patch is unchanged. Focused presentation-device-loss, readback-visibility and runner-declaration checks pass.

New head: d446e99d0191759fcfe1f81fe7693fed1b01a2c6. Fresh checks: source, Windows, AppImage; pending. Full local suite is running. Historical native results remain tied to their original builds; no new API13 game run claimed.

A device reset reported during native presentation bypasses the replay's existing terminal device-loss handler. Fence/command-buffer calls ignore the result, acquisition returns silently, and present only logs it; dependent work can continue without the report/exit policy already used by normal submissions.

Route VK_ERROR_DEVICE_LOST from all result-bearing Vulkan calls directly in present_image to the existing device_lost() handler: fence wait/reset, command-pool reset, command begin/end, acquisition, submission and both presentation queues. Check before readback or dependent work and before Reflex counts the present. The existing handler writes its checkpoint report once, flushes the host log and exits. No API change, new dependency or successful rendering change.

This follows the project's terminal policy; it does not recover a device or address arbitrary submission OOM. Out-of-date/suboptimal/Wayland-timeout handling stays in place. The tests distinguish call-site routing from a real hardware reset.

Validation at 1d6e9a6 on main b2e1c5a:

  • Nine actual production call/guard sites (including both present queues), with loss and non-loss controls. Original main misses the loss report at all nine sites; removing each of the eight guards reproduces failure. The existing concurrent report-once/log-flush/exit test still passes.
  • Windows LLVM and Linux GCC pass, including Linux ASan/UBSan for the routing and concurrent report tests.
  • Full Windows strict suite: 208 passed / 10 failed / 22 skipped; exact main baseline 207/10/22, with the same ten failures and no new failing test. Shared fixture repairs are in #1498; the remaining local limitations are recorded there. This does not claim a green full suite.
  • Official standalone host/backend build succeeds with the matching API11 pair. Native healthy run reached the copied Kokiri checkpoint with FG2, maximized1720x720 to3440x1369, restored, and exited normally0. Original save hashes unchanged. No physical driver reset was induced; the failure evidence is the CPU routing controls. Guest generation provenance/input-equivalence is recorded locally; no game-derived files are included.

Exact-head fork workflows follow. Documentation: docs/presentation-device-loss.md.

Main BC retry/staging fallback changes introduced a shared source-fixture compilation regression. At f80c40dcd9d053d5508ee42df956bcacb84b8a14, the two staging-publication fixture files are integrated from fe1c22ceb699830965d75168bfe13efc9db690ef: existing assertions remain, mapped fallback/unmapped cleanup/reuse and multi-atom ASTC visibility are exercised. GPU/host/lifter/third-party source trees are identical to the previous head; earlier native results retain their actual hashes. This CI repair is tracked separately and adds no production behavior to this contribution. Exact-head fork workflows were refreshed.

Current-head validation

Current head d446e99, based on main d224dc0. Earlier local/runtime results describe their recorded source hashes.

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

<!-- current-refresh --> Rebased onto main `d224dc06` and resolved the test-runner conflict by moving this test's arguments into `RUN_ALL_ARGS`, following #1822. The production patch is unchanged. Focused presentation-device-loss, readback-visibility and runner-declaration checks pass. New head: `d446e99d0191759fcfe1f81fe7693fed1b01a2c6`. Fresh checks: [source](https://github.com/Arkex-ai/Hookshot/actions/runs/42869641605), [Windows](https://github.com/Arkex-ai/Hookshot/actions/runs/42869648123), [AppImage](https://github.com/Arkex-ai/Hookshot/actions/runs/42869653276); pending. Full local suite is running. Historical native results remain tied to their original builds; no new API13 game run claimed. <!-- /current-refresh --> A device reset reported during native presentation bypasses the replay's existing terminal device-loss handler. Fence/command-buffer calls ignore the result, acquisition returns silently, and present only logs it; dependent work can continue without the report/exit policy already used by normal submissions. Route `VK_ERROR_DEVICE_LOST` from all result-bearing Vulkan calls directly in `present_image` to the existing `device_lost()` handler: fence wait/reset, command-pool reset, command begin/end, acquisition, submission and both presentation queues. Check before readback or dependent work and before Reflex counts the present. The existing handler writes its checkpoint report once, flushes the host log and exits. No API change, new dependency or successful rendering change. This follows the project's terminal policy; it does not recover a device or address arbitrary submission OOM. Out-of-date/suboptimal/Wayland-timeout handling stays in place. The tests distinguish call-site routing from a real hardware reset. Validation at `1d6e9a6` on main `b2e1c5a`: - Nine actual production call/guard sites (including both present queues), with loss and non-loss controls. Original main misses the loss report at all nine sites; removing each of the eight guards reproduces failure. The existing concurrent report-once/log-flush/exit test still passes. - Windows LLVM and Linux GCC pass, including Linux ASan/UBSan for the routing and concurrent report tests. - Full Windows strict suite: **208 passed / 10 failed / 22 skipped**; exact main baseline **207/10/22**, with the same ten failures and no new failing test. Shared fixture repairs are in #1498; the remaining local limitations are recorded there. This does not claim a green full suite. - Official standalone host/backend build succeeds with the matching API11 pair. Native healthy run reached the copied Kokiri checkpoint with FG2, maximized1720x720 to3440x1369, restored, and exited normally0. Original save hashes unchanged. No physical driver reset was induced; the failure evidence is the CPU routing controls. Guest generation provenance/input-equivalence is recorded locally; no game-derived files are included. Exact-head fork workflows follow. Documentation: `docs/presentation-device-loss.md`. <!-- shared-staging-fixture-integration --> Main BC retry/staging fallback changes introduced a shared source-fixture compilation regression. At `f80c40dcd9d053d5508ee42df956bcacb84b8a14`, the two staging-publication fixture files are integrated from `fe1c22ceb699830965d75168bfe13efc9db690ef`: existing assertions remain, mapped fallback/unmapped cleanup/reuse and multi-atom ASTC visibility are exercised. GPU/host/lifter/third-party source trees are identical to the previous head; earlier native results retain their actual hashes. This CI repair is tracked separately and adds no production behavior to this contribution. Exact-head fork workflows were refreshed. <!-- goal15-current-validation --> ### Current-head validation Current head `d446e99`, based on main `d224dc0`. Earlier local/runtime results describe their recorded source hashes. - [Linux AppImage](https://github.com/Arkex-ai/Hookshot/actions/runs/42869653276): in_progress. - [Windows builder](https://github.com/Arkex-ai/Hookshot/actions/runs/42869648123): success. - [Game-free source regressions](https://github.com/Arkex-ai/Hookshot/actions/runs/42869641605): failure. 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 16:38:58 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. Checked every changed call site in present_image against the head: wait/reset/begin/end/acquire/submit and both present paths now go to device_lost(), which is declared earlier in the file (line ~2510) and is noreturn. The out-of-date/suboptimal/timeout handling is untouched, and device_lost() takes no locks, so calling it while holding present_mu or present_queue_mu can't deadlock. The new test is registered in run_all.py, and the present_semaphores test fix (whole function instead of a fixed 12000 chars) is sensible. CI is green.

Nit: the test is source-scraping plus a stubbed fixture, so it proves routing but not behaviour; fine given no GPU, just worth knowing.

Looks good. Checked every changed call site in present_image against the head: wait/reset/begin/end/acquire/submit and both present paths now go to device_lost(), which is declared earlier in the file (line ~2510) and is [[noreturn]]. The out-of-date/suboptimal/timeout handling is untouched, and device_lost() takes no locks, so calling it while holding present_mu or present_queue_mu can't deadlock. The new test is registered in run_all.py, and the present_semaphores test fix (whole function instead of a fixed 12000 chars) is sensible. CI is green. Nit: the test is source-scraping plus a stubbed fixture, so it proves routing but not behaviour; fine given no GPU, just worth knowing.
hckrman101 commented 2026-11-10 17:49:24 +00:00 (Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
Arkex-ai commented 2026-11-10 17:54:44 +00:00 (Migrated from github.com)

Rebased onto main d224dc06 and resolved the test-runner conflict by moving this test's arguments into RUN_ALL_ARGS, following #1822. The production patch is unchanged. Focused presentation-device-loss, readback-visibility and runner-declaration checks pass.

New head: d446e99d0191759fcfe1f81fe7693fed1b01a2c6. Fresh checks: source, Windows, AppImage; pending. Full local suite is running. Historical native results remain tied to their original builds; no new API13 game run claimed.

Rebased onto main `d224dc06` and resolved the test-runner conflict by moving this test's arguments into `RUN_ALL_ARGS`, following #1822. The production patch is unchanged. Focused presentation-device-loss, readback-visibility and runner-declaration checks pass. New head: `d446e99d0191759fcfe1f81fe7693fed1b01a2c6`. Fresh checks: [source](https://github.com/Arkex-ai/Hookshot/actions/runs/42869641605), [Windows](https://github.com/Arkex-ai/Hookshot/actions/runs/42869648123), [AppImage](https://github.com/Arkex-ai/Hookshot/actions/runs/42869653276); pending. Full local suite is running. Historical native results remain tied to their original builds; no new API13 game run claimed.
Commenting is not possible because the repository is archived.
No description provided.