fix(vk): make capture readbacks visible before CPU reads #1502

Merged

Arkex-ai merged 4 commits from fix/capture-readback-visibility into main

2026-11-10 16:44:07 +00:00

Arkex-ai commented

2026-11-10 05:29:28 +00:00

(Migrated from github.com)

Record a COPY/TRANSFER_WRITE to HOST/HOST_READ dependency for each normal screenshot, generated screenshot and binary frame-log staging copy. Scope it to the copied buffer range, without changing queue ownership. After the existing timeline/fence wait, invalidate that allocation before the CPU copy or PNG consumer. Already CPU-owned pending screenshots retain their snapshot across parity-slot reuse and are not invalidated or reread.

Current head a454e6f is rebased on main ba40a1a, including the merged checked PNG writer. The standalone contribution patch is unchanged by that rebase; the earlier capture-cache tests are extended to execute the real producer copy/dependency tails and the CPU consumers. Existing resource-allocation and semaphore-lifecycle fixtures also retain their assertions with the new helper and complete function extraction.

Validation:

At pre-refresh head 5439672, eight permissive host-visibility controls pass while eight explicit-host-visibility cases fail on stale output, covering normal/owned/generated/log reads with coherent and non-coherent caches. These are authored protocol models, not observed NVIDIA corruption.
The completed handoff passes all 16 scenarios in Windows LLVM, Linux GCC and Linux ASan/UBSan. Thirteen incorrect variants detect omitted producer dependencies, stage/access/range/buffer mistakes, premature ordering, missing invalidation and missing completion. Capture resource/retry/cleanup and complete presentation semaphore tests pass as well.
Complete suite on current main: a454e6f has 236 passed, 2 failed, 20 skipped versus exact main ba40a1a: 235 passed, the same 2 failures and 20 skips. Those local limitations remain NUP symlink privileges and unavailable Windows SDK headers. Earlier pre-refresh results are retained separately; no checks were relaxed.
Official matching API12 build passes at current head a454e6f, using a fresh clean official guest generation on this main base. The renewed native checkpoint at this exact head produces a normal/generated 3440x1440 PNG pair that passes CRC/zlib/RGB decoding; its binary frame log contains 33 valid consecutive records (17 normal, 16 generated), no missing sequence numbers, and correct metadata. The game exits0, no incomplete PNG temporary remains, and original checkpoint hashes stay unchanged. Earlier native runs retain their original source/artifact hashes.

This changes capture memory visibility, not capture dimensions, scheduling, slot ownership, output format or renderer ABI. Native coverage is one serial/FG2 NVIDIA checkpoint; no physical non-coherent memory, campaign or performance claim. All game-derived files and private runtime data remain local.

Details and primary references: docs/capture-readback.md. Exact-head fork source, Windows-builder and AppImage workflows have been dispatched for the refreshed head; upstream approval requirements remain in place.

Main BC retry/staging fallback changes introduced a shared source-fixture compilation regression. At 13ac710494be50d3e900d4b66e73114a7647e13a, 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 b8511b0, based on main 05098f9. 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.

GPU-written screenshot and frame-log buffers are mapped, but mapping and submission completion alone do not establish host-domain availability or invalidate a non-coherent CPU cache. Reading those mappings can therefore return stale diagnostic pixels. Record a COPY/TRANSFER_WRITE to HOST/HOST_READ dependency for each normal screenshot, generated screenshot and binary frame-log staging copy. Scope it to the copied buffer range, without changing queue ownership. After the existing timeline/fence wait, invalidate that allocation before the CPU copy or PNG consumer. Already CPU-owned pending screenshots retain their snapshot across parity-slot reuse and are not invalidated or reread. Current head `a454e6f` is rebased on main `ba40a1a`, including the merged checked PNG writer. The standalone contribution patch is unchanged by that rebase; the earlier capture-cache tests are extended to execute the real producer copy/dependency tails and the CPU consumers. Existing resource-allocation and semaphore-lifecycle fixtures also retain their assertions with the new helper and complete function extraction. Validation: - At pre-refresh head `5439672`, eight permissive host-visibility controls pass while eight explicit-host-visibility cases fail on stale output, covering normal/owned/generated/log reads with coherent and non-coherent caches. These are authored protocol models, not observed NVIDIA corruption. - The completed handoff passes all 16 scenarios in Windows LLVM, Linux GCC and Linux ASan/UBSan. Thirteen incorrect variants detect omitted producer dependencies, stage/access/range/buffer mistakes, premature ordering, missing invalidation and missing completion. Capture resource/retry/cleanup and complete presentation semaphore tests pass as well. - Complete suite on current main: `a454e6f` has 236 passed, 2 failed, 20 skipped versus exact main `ba40a1a`: 235 passed, the same 2 failures and 20 skips. Those local limitations remain NUP symlink privileges and unavailable Windows SDK headers. Earlier pre-refresh results are retained separately; no checks were relaxed. - Official matching API12 build passes at current head `a454e6f`, using a fresh clean official guest generation on this main base. The renewed native checkpoint at this exact head produces a normal/generated 3440x1440 PNG pair that passes CRC/zlib/RGB decoding; its binary frame log contains 33 valid consecutive records (17 normal, 16 generated), no missing sequence numbers, and correct metadata. The game exits0, no incomplete PNG temporary remains, and original checkpoint hashes stay unchanged. Earlier native runs retain their original source/artifact hashes. This changes capture memory visibility, not capture dimensions, scheduling, slot ownership, output format or renderer ABI. Native coverage is one serial/FG2 NVIDIA checkpoint; no physical non-coherent memory, campaign or performance claim. All game-derived files and private runtime data remain local. Details and primary references: `docs/capture-readback.md`. Exact-head fork source, Windows-builder and AppImage workflows have been dispatched for the refreshed head; upstream approval requirements remain in place.  Main BC retry/staging fallback changes introduced a shared source-fixture compilation regression. At `13ac710494be50d3e900d4b66e73114a7647e13a`, 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 `b8511b0`, based on main `05098f9`. 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/42849527586): success. - [Windows builder](https://github.com/Arkex-ai/Hookshot/actions/runs/42849522472): success. - [Game-free source regressions](https://github.com/Arkex-ai/Hookshot/actions/runs/42849518289): 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 16:38:55 +00:00

hckrman101 (Migrated from github.com) left a comment

Looks good. The barrier is the right one (COPY/TRANSFER_WRITE to HOST/HOST_READ, buffer-scoped, no queue ownership change) and it is recorded after each of the three staging copies (made, normal, frame log). The CPU side invalidates after the existing waits (shot_pixels, the made-shot take(), framelog_collect), using the existing invalidate_mapped helper, which is a no-op on coherent memory. Already-CPU-owned pending shots are left alone. CI is green and the PR merges cleanly.

Nit: the new tests model the protocol rather than exercising real drivers, which the description says plainly, so that is fine.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101