fix(vk): release frame interpolation resources when setup fails #1445

Merged

Arkex-ai merged 3 commits from fix/framegen-resource-rollback into main

2026-11-10 04:26:53 +00:00

Arkex-ai commented

2026-11-10 02:55:38 +00:00

(Migrated from github.com)

Construct pipelines/images locally and publish them only when complete. Roll back earlier setup objects and partial image sets on failure, clear size/history, and preserve the existing session-long interpolation disable policy so ordinary presentation can continue. Keep the existing display-job/device drain before replacing submitted images. The failure predicate is atomic for the game-thread Reflex reader. Independent scaling retains its own state.

Validation:

Production-function CPU fixture: 112 failure positions, fixed 2x/max 8x, resize drain, ownership order, independent scale setup and fallback eligibility. All three negative controls fail on main; seven compiled mutations are detected.
Windows strict suite: candidate 189 passed/4 failed/16 skipped versus main 188/4/16. The same four local toolchain/environment failures occur in both; remote CI is tracked separately.
Official integrated host/backend build; Linux GCC and ASan/UBSan CPU fixture passed.
RTX 5070 Ti gameplay checkpoint smoke with 2x interpolation/RCAS and maximize/restore passed. Private sampler and fifth-image-view fault variants continued rendering the checkpoint with scaling and exited normally. These are simulated API failures, not genuine driver OOM/device-loss validation.

Based on main 9a2e80e. No game assets, generated guest code, private fault switches, new settings or quality-policy changes are included. The fixture includes the production cleanup helper used by independent scaling. Source overlap with #1386 and #1352 was checked; both merge cleanly at the fetched heads.

Current-head validation

Current head 9613130, based on main 5609af4. 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.

When an optional interpolation pipeline or image-view creation fails, the earlier Vulkan objects can remain allocated. Sampler creation errors are also ignored, allowing initialization to report success with an unusable sampler. Construct pipelines/images locally and publish them only when complete. Roll back earlier setup objects and partial image sets on failure, clear size/history, and preserve the existing session-long interpolation disable policy so ordinary presentation can continue. Keep the existing display-job/device drain before replacing submitted images. The failure predicate is atomic for the game-thread Reflex reader. Independent scaling retains its own state. Validation: - Production-function CPU fixture: 112 failure positions, fixed 2x/max 8x, resize drain, ownership order, independent scale setup and fallback eligibility. All three negative controls fail on main; seven compiled mutations are detected. - Windows strict suite: candidate 189 passed/4 failed/16 skipped versus main 188/4/16. The same four local toolchain/environment failures occur in both; remote CI is tracked separately. - Official integrated host/backend build; Linux GCC and ASan/UBSan CPU fixture passed. - RTX 5070 Ti gameplay checkpoint smoke with 2x interpolation/RCAS and maximize/restore passed. Private sampler and fifth-image-view fault variants continued rendering the checkpoint with scaling and exited normally. These are simulated API failures, not genuine driver OOM/device-loss validation. Based on main `9a2e80e`. No game assets, generated guest code, private fault switches, new settings or quality-policy changes are included. The fixture includes the production cleanup helper used by independent scaling. Source overlap with #1386 and #1352 was checked; both merge cleanly at the fetched heads.  ### Current-head validation Current head `9613130`, based on main `5609af4`. 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/42753308749): success. - [Windows builder](https://github.com/Arkex-ai/Hookshot/actions/runs/42753305658): success. - [Game-free source regressions](https://github.com/Arkex-ai/Hookshot/actions/runs/42753302903): success. Fork runs are separate from upstream Actions approval; no maintainer approval gate is bypassed.

NotRexed commented

2026-11-10 03:49:47 +00:00

(Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!

NotRexed commented

2026-11-10 03:55:45 +00:00

(Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

hckrman101 (Migrated from github.com) approved these changes

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

hckrman101 (Migrated from github.com) left a comment

Looks good to me. I went through the rollback paths for double frees, objects still in flight, and half-built state, and didn't find anything:

make_pipe / make_img now build into a local and only write to the caller's struct on success, so a failure leaves the target untouched. A later drop_setup() / drop_size() then only destroys handles that were really published, and nothing gets destroyed twice. init_scale clearing scale.linear on sampler failure closes the same gap there.
Sampler results are checked now, so init() can't report success with a null sampler anymore.
In ensure_size, both drop_size() calls run after the jobs/busy-set wait and device_wait_idle_locked(), so no submitted work still references the old images. The new images on the failure path were never recorded. Committing w/h/mult only after every image exists also fixes the stale fg.mult that main left behind on failure.
Fallback still works: framegen_record returning false leaves interpolated false, so do_present presents the frame directly in both the inline and fg_after paths, and framegen_on() stays false after that.
Making failed atomic is right, since Reflex reads it from the game thread through framegen_on().
The new test is registered in tests/run_all.py, the test_upscale_independent.py change picks up drop_pipe, and no settings or option ids are touched.

One small comment-only nit inline. There are no checks on the upstream PR yet (the fork runs are linked in the description).

Looks good to me. I went through the rollback paths for double frees, objects still in flight, and half-built state, and didn't find anything: - `make_pipe` / `make_img` now build into a local and only write to the caller's struct on success, so a failure leaves the target untouched. A later `drop_setup()` / `drop_size()` then only destroys handles that were really published, and nothing gets destroyed twice. `init_scale` clearing `scale.linear` on sampler failure closes the same gap there. - Sampler results are checked now, so `init()` can't report success with a null sampler anymore. - In `ensure_size`, both `drop_size()` calls run after the jobs/busy-set wait and `device_wait_idle_locked()`, so no submitted work still references the old images. The new images on the failure path were never recorded. Committing `w/h/mult` only after every image exists also fixes the stale `fg.mult` that main left behind on failure. - Fallback still works: `framegen_record` returning false leaves `interpolated` false, so `do_present` presents the frame directly in both the inline and `fg_after` paths, and `framegen_on()` stays false after that. - Making `failed` atomic is right, since Reflex reads it from the game thread through `framegen_on()`. - The new test is registered in `tests/run_all.py`, the `test_upscale_independent.py` change picks up `drop_pipe`, and no settings or option ids are touched. One small comment-only nit inline. There are no checks on the upstream PR yet (the fork runs are linked in the description).

gpu/vk/vk_framegen.cpp

Nit: dropping the pipelines/samplers here is only safe because framegen_record calls ensure_size before it records any pass() into cmd, and because older submissions were drained above. A one-line comment saying so would stop someone from later moving a pass() ahead of ensure_size without noticing.

Nit: dropping the pipelines/samplers here is only safe because `framegen_record` calls `ensure_size` before it records any `pass()` into `cmd`, and because older submissions were drained above. A one-line comment saying so would stop someone from later moving a `pass()` ahead of `ensure_size` without noticing.

Arkex-ai (Migrated from github.com) reviewed

2026-11-10 04:22:45 +00:00

gpu/vk/vk_framegen.cpp

Added the ordering comment: older submissions are drained and ensure_size runs before framegen_record records any pass. The production rollback regression passes; refreshing the three fork workflows on the comment-only head. Commit 944b550.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101