MagicBeanDev/Longshot

Watch

Fork 98

WIP: fix(vk): an event signal's add writes the value the stream gives the event #772

Draft

dyst wants to merge 1 commit from dyst/Longshot:fix/event-signal-add into main

dyst commented

2026-11-15 11:19:41 +00:00

Member

Type: fix · Risk: low · Tested on: Windows 11 (the game-free checks only; no game run) · Needs: a game run, any OS and GPU

Why

When the GPU signals an event by adding to its value (NVN's ADD mode), the renderer wrote the wrong number into the event: it added to what the event held in the game's memory, which doesn't yet have the earlier signals of the same recording. Two adds in a row wrote the same value, and an add after a set added to the value before the set. The renderer's own waits were not affected (they keep the right value), but the game's reads of the event (nvnEventGetValue) saw fewer signals than were made, which can hold a thread that waits for the count or let it go early.

In Ocarina of Time this probably doesn't happen: in 133 run logs on this PC, all 200 sampled GPU event signals are SET, none ADD. The fix is for correctness where ADD is used. Found by reading the replay. The tracker has no issue for it (forge.py issue list, pr list: nothing on event signals).

How to reproduce

Seen on: not seen in the game; reproduced by the new game-free check
Settings and switches: defaults
Steps: python -I -B tests/test_event_signal_add.py against main's gpu/vk/vk_replay.cpp: two adds of 1 from 5, a set of 10 and an add of 5 in one recording
Expected: the GPU writes 6, 7, 10, 15
Actual: main writes 6, 6, 10, 10 (serial replay, and the parallel replay's workers alike)

What changed

An event signal's GPU write now takes the value the replay already keeps for the event in stream order, instead of guest memory's value plus the add. The parallel replay's workers, which don't keep those values, take each signal's value from the dry pass, the same way they take the render conditions' results. SET signals write exactly what they wrote before.

replay: NOP_SIGNAL_EVENT in replay() (gpu/vk/vk_replay.cpp) writes ev_values[e.gpu]; the dry pass records each signal's value in par_signals, which ParPoint and Segment carry as sig to record_segment (sig_at), beside par_conds / cond_at. A worker with no value from the dry pass falls back to the old add.
tests: tests/test_event_signal_add.py compiles the real NOP_SIGNAL_EVENT case in a stub replay loop and runs it serially, as a dry pass, as a worker, and as a worker starting at a later segment.

Measurements

Verdict: nothing to measure for speed: the change only alters the value an ADD signal writes (one map lookup it already made). The new check fails on main and passes here.

Flags and settings

None.

Testing

Tested: Windows 11 (llvm-mingw from the builder, v2.0.14 tools):

tests/test_event_signal_add.py passes on this branch and fails on main @ aa95d35f (output above).
gpu/vk/vk_replay.cpp compiles (-fsyntax-only) with the renderer's flags and the CPM Vulkan, SPIR-V and VMA headers.
python -I -B tests/run_all.py on this branch and on main @ aa95d35f, same PC and environment: 497 passed, 19 failed, 78 skipped here; 496 passed, 19 failed, 78 skipped on main. The only difference is the new check. The 19 fail on main too, on this PC (among them test_env_on, which flags main's RX_VK_PRESENT_AFTER_SIGNAL read at vk_replay.cpp:5794): test_appimage_player_build.py, test_bake_portability.py, test_cpm_discovery.py, test_cpp_preflight.py, test_crash_dialog.py, test_env_on.py, test_fpx_sticky.py, test_lift_cleanup_error.py, test_linux_preflight.py, test_make_game_cache.py, test_make_game_prepare.py, test_missing_nup.py, test_oot_state_tap.py, test_options_budget_once.py, test_readback_late.py, test_spawn_emit.py, test_tested_configurations.py, test_vk_pgo.py, test_walk_key.py.

Not tested: the game. No build was made; this PC's builds were not used for this change. The parallel replay's segments (RX_VK_PARALLEL) and the render thread are covered by the game-free check only.

Must be tested:

Any OS and GPU, with the defaults and once with RX_VK_PARALLEL=4: load a save in Kokiri Forest and play two minutes, expecting the same picture and frame rate as main and no new vk: a wait not met in replay order lines in the log (Test Farm: @Beanstalk any, Kokiri Forest two minutes with defaults and with RX_VK_PARALLEL=4, compare the log's wait lines with main)

Risks

What could break: the parallel replay's workers indexing the wrong signal (a segment starting at the wrong sig): an event would get another signal's value and a queue could wait too long or go on early. SET signals, the only kind seen in this game, write the same value as before in every mode.

Turning it off: revert the commit.

Compatibility: Nothing changes: no caches, settings or vk ABI involved.

Checklist

Everyone

One topic.
The title is a Conventional Commit (type(scope): description) of the status line's type.
Why gives the reason plainly and links the issues that fit (the tracker searched).
Measurements has a verdict, or one line saying why there is nothing to measure.
Testing names what was tested, what wasn't, and what must still be tested, where and how.
Rebased on current main.
python -I -B tests/run_all.py passes (any failure that also fails on main is named).
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A game-free test added for the fix, failing on main and passing here.

🤖 Generated with Claude Code

**Type:** fix · **Risk:** low · **Tested on:** Windows 11 (the game-free checks only; no game run) · **Needs:** a game run, any OS and GPU ## Why When the GPU signals an event by adding to its value (NVN's ADD mode), the renderer wrote the wrong number into the event: it added to what the event held in the game's memory, which doesn't yet have the earlier signals of the same recording. Two adds in a row wrote the same value, and an add after a set added to the value before the set. The renderer's own waits were not affected (they keep the right value), but the game's reads of the event (`nvnEventGetValue`) saw fewer signals than were made, which can hold a thread that waits for the count or let it go early. In Ocarina of Time this probably doesn't happen: in 133 run logs on this PC, all 200 sampled GPU event signals are SET, none ADD. The fix is for correctness where ADD is used. Found by reading the replay. The tracker has no issue for it (`forge.py issue list`, `pr list`: nothing on event signals). ## How to reproduce **Seen on:** not seen in the game; reproduced by the new game-free check **Settings and switches:** defaults **Steps:** `python -I -B tests/test_event_signal_add.py` against main's `gpu/vk/vk_replay.cpp`: two adds of 1 from 5, a set of 10 and an add of 5 in one recording **Expected:** the GPU writes 6, 7, 10, 15 **Actual:** main writes 6, 6, 10, 10 (serial replay, and the parallel replay's workers alike) ## What changed An event signal's GPU write now takes the value the replay already keeps for the event in stream order, instead of guest memory's value plus the add. The parallel replay's workers, which don't keep those values, take each signal's value from the dry pass, the same way they take the render conditions' results. SET signals write exactly what they wrote before. - replay: `NOP_SIGNAL_EVENT` in `replay()` (`gpu/vk/vk_replay.cpp`) writes `ev_values[e.gpu]`; the dry pass records each signal's value in `par_signals`, which `ParPoint` and `Segment` carry as `sig` to `record_segment` (`sig_at`), beside `par_conds` / `cond_at`. A worker with no value from the dry pass falls back to the old add. - tests: `tests/test_event_signal_add.py` compiles the real `NOP_SIGNAL_EVENT` case in a stub replay loop and runs it serially, as a dry pass, as a worker, and as a worker starting at a later segment. ## Measurements **Verdict:** nothing to measure for speed: the change only alters the value an ADD signal writes (one map lookup it already made). The new check fails on main and passes here. ### Flags and settings None. ## Testing **Tested:** Windows 11 (llvm-mingw from the builder, v2.0.14 tools): - `tests/test_event_signal_add.py` passes on this branch and fails on main @ aa95d35f (output above). - `gpu/vk/vk_replay.cpp` compiles (`-fsyntax-only`) with the renderer's flags and the CPM Vulkan, SPIR-V and VMA headers. - `python -I -B tests/run_all.py` on this branch and on main @ aa95d35f, same PC and environment: 497 passed, 19 failed, 78 skipped here; 496 passed, 19 failed, 78 skipped on main. The only difference is the new check. The 19 fail on main too, on this PC (among them `test_env_on`, which flags main's `RX_VK_PRESENT_AFTER_SIGNAL` read at `vk_replay.cpp:5794`): `test_appimage_player_build.py`, `test_bake_portability.py`, `test_cpm_discovery.py`, `test_cpp_preflight.py`, `test_crash_dialog.py`, `test_env_on.py`, `test_fpx_sticky.py`, `test_lift_cleanup_error.py`, `test_linux_preflight.py`, `test_make_game_cache.py`, `test_make_game_prepare.py`, `test_missing_nup.py`, `test_oot_state_tap.py`, `test_options_budget_once.py`, `test_readback_late.py`, `test_spawn_emit.py`, `test_tested_configurations.py`, `test_vk_pgo.py`, `test_walk_key.py`. **Not tested:** the game. No build was made; this PC's builds were not used for this change. The parallel replay's segments (`RX_VK_PARALLEL`) and the render thread are covered by the game-free check only. **Must be tested:** - Any OS and GPU, with the defaults and once with `RX_VK_PARALLEL=4`: load a save in Kokiri Forest and play two minutes, expecting the same picture and frame rate as main and no new `vk: a wait not met in replay order` lines in the log (Test Farm: @Beanstalk any, Kokiri Forest two minutes with defaults and with RX_VK_PARALLEL=4, compare the log's wait lines with main) ## Risks **What could break:** the parallel replay's workers indexing the wrong signal (a segment starting at the wrong `sig`): an event would get another signal's value and a queue could wait too long or go on early. SET signals, the only kind seen in this game, write the same value as before in every mode. **Turning it off:** revert the commit. **Compatibility:** Nothing changes: no caches, settings or vk ABI involved. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dyst added 1 commit

2026-11-15 11:19:42 +00:00

fix(vk): an event signal's add writes the value the stream gives the event

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped

Details

Third-party parts / The same as upstream (pull_request) Has been skipped

Details

PR description / Description follows the template (pull_request_target) Successful in 4s

Details

CI / Conventional Commit title (pull_request) Successful in 3s

Details

CI / Workflow lint (actionlint) (pull_request) Successful in 6s

Details

Guide screenshots / Guide images and local UI source review (pull_request) Failing after 5s

Details

Third-party parts / As their notes say (pull_request) Successful in 9s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

beanstalk/approval approved by dyst (reviewed)

Details

beanstalk/linux 0/1 pass, 1 error

Details

2c5a62e56d

The replay keeps each event's value in stream order (ev_values) for the GPU's waits, but the GPU write of an
NVN_EVENT_SIGNAL_MODE_ADD signal took guest memory's value plus the add. Guest memory does not have the writes of the
signals before it while their recording has not run on the GPU, so two adds in one recording wrote the same value, and
an add after a set added to the value before the set: the game's nvnEventGetValue saw fewer adds than the stream made.

The GPU write now takes the value from ev_values. The parallel replay's workers, which do not keep ev_values, take each
signal's value from the dry pass in stream order (par_signals, carried by ParPoint and Segment as sig), as they take
the render conditions' results. SET signals write what they wrote before.

tests/test_event_signal_add.py compiles the real NOP_SIGNAL_EVENT case and replays adds and a set serially and as a
dry pass and a worker: main writes 6 6 10 10 where the stream makes 6 7 10 15.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

dyst commented

2026-11-15 11:19:53 +00:00

Author

Member

@Beanstalk any OS and GPU: load a save in Kokiri Forest and play two minutes, once with the defaults and once with RX_VK_PARALLEL=4, on this PR and on main. Expected: the same picture and frame rate as main, and no new 'vk: a wait not met in replay order' lines in longshot_log.txt. SET signals (all this game uses) write the same values as before, so nothing should change on screen.

👀 1

Beanstalk commented

2026-11-15 11:19:55 +00:00

Member

Beanstalk test farm

Commit 2c5a62e · tier reviewed · approved by dyst · dashboard

What will be tested · AI plan (gpt-6.1-terra)

Compare the pull request with main in Kokiri Forest for two minutes per run, first with defaults and then with RX_VK_PARALLEL=4 to exercise parallel replay. The plan measures frame times, compares pictures, and checks for replay-order wait warnings. Crashes, warnings, substantial frame-time regressions, or picture differences fail the plan; this game run does not directly prove ADD-signal correctness.

Where: linux (1 report).
kokiri-default: from a new save, then move the stick, take the picture forest to compare with main, measure forest for 2 min (after 5 s warm-up); 1 run on each side.
kokiri-parallel: from a new save (switches RX_VK_PARALLEL=4), then move the stick, take the picture forest to compare with main, measure forest for 2 min (after 5 s warm-up); 1 run on each side.
Measured: crashes, frame times, stutters and hitches, the game's log.
The PR fails if: it crashes; or a picture differs from main by more than 5%; or frame time (p99) is more than 15% above main; or forbidden log lines is above 0.
Time: about 33 min of game runs per tester (4 game starts), plus the two builds.

Done. Run 2: re-planned by dyst. Comment @Beanstalk to run it again.

Target	Reports	Crash	Frame p99 (ms) vs main	VRAM peak (MB) vs main	Result
linux · 1: linux+nvidia+nvidia-proprietary+dgpu `NVIDIA GeForce RTX 4070, 615.71.9`	1/1 1	none	—	—	error

Measurements · linux

Verdict: could not be judged on linux (1/1 report).

Hardware: 1: linux+nvidia+nvidia-proprietary+dgpu NVIDIA GeForce RTX 4070, 615.71.9 AMD Ryzen 7 5800X3D 8-Core Processor, 63 GB RAM
Scene: kokiri-default, from a new save; kokiri-parallel, from a new save (RX_VK_PARALLEL=4); each cell's settings and switches as the table names them
Method: the Beanstalk test farm: main and this PR built and run on the same PC in one session, game starts alternating (ABBA); kokiri-default: 1 game start a side, forest 120 s after 5 s warm-up; kokiri-parallel: 1 game start a side, forest 120 s after 5 s warm-up; each number the median of the runs, then of 1 report
How to reproduce: the farm's plan above (@Beanstalk on this PR runs it again)

	Before: main @ `aa95d35`	After: this PR @ `2c5a62e`
kokiri-default · forbidden log lines	0.0	0.0
kokiri-parallel · forbidden log lines	0.0	0.0

Flags and settings

None: the plan varied no setting, switch or build option.

Markdown for the PR description

## Measurements

**Verdict:** could not be judged on linux (1/1 report).

Hardware: 1: linux+nvidia+nvidia-proprietary+dgpu `NVIDIA GeForce RTX 4070, 615.71.9` `AMD Ryzen 7 5800X3D 8-Core Processor, 63 GB RAM`  
Scene: kokiri-default, from a new save; kokiri-parallel, from a new save (RX_VK_PARALLEL=4); each cell's settings and switches as the table names them  
Method: the Beanstalk test farm: main and this PR built and run on the same PC in one session, game starts alternating (ABBA); kokiri-default: 1 game start a side, forest 120 s after 5 s warm-up; kokiri-parallel: 1 game start a side, forest 120 s after 5 s warm-up; each number the median of the runs, then of 1 report  
How to reproduce: the farm's plan above (`@Beanstalk` on this PR runs it again)

| | Before: main @ aa95d35 | After: this PR @ 2c5a62e |
|---|---|---|
| kokiri-default · forbidden log lines | 0.0 | 0.0 |
| kokiri-parallel · forbidden log lines | 0.0 | 0.0 |

### Flags and settings

None: the plan varied no setting, switch or build option.

### Beanstalk test farm Commit `2c5a62e` · tier **reviewed** · approved by dyst · [dashboard](https://beanstalk.magicbean.dev/prs/772) **What will be tested** · AI plan (`gpt-6.1-terra`) Compare the pull request with main in Kokiri Forest for two minutes per run, first with defaults and then with RX_VK_PARALLEL=4 to exercise parallel replay. The plan measures frame times, compares pictures, and checks for replay-order wait warnings. Crashes, warnings, substantial frame-time regressions, or picture differences fail the plan; this game run does not directly prove ADD-signal correctness. - **Where:** linux (1 report). - **kokiri-default:** from a new save, then move the stick, take the picture forest to compare with main, measure forest for 2 min (after 5 s warm-up); 1 run on each side. - **kokiri-parallel:** from a new save (switches `RX_VK_PARALLEL=4`), then move the stick, take the picture forest to compare with main, measure forest for 2 min (after 5 s warm-up); 1 run on each side. - **Measured:** crashes, frame times, stutters and hitches, the game's log. - **The PR fails if:** it crashes; or a picture differs from main by more than 5%; or frame time (p99) is more than 15% above main; or forbidden log lines is above 0. - **Time:** about 33 min of game runs per tester (4 game starts), plus the two builds. Done. Run 2: re-planned by dyst. Comment `@Beanstalk` to run it again. | Target | Reports | Crash | Frame p99 (ms) vs main | VRAM peak (MB) vs main | Result | | --- | --- | --- | --- | --- | --- | | linux · 1: linux+nvidia+nvidia-proprietary+dgpu `NVIDIA GeForce RTX 4070, 615.71.9` | 1/1 [1](https://beanstalk.magicbean.dev/reports/rp_a4aeeeca85a2bbee) | none | — | — | error | #### Measurements · linux **Verdict:** could not be judged on linux (1/1 report). Hardware: 1: linux+nvidia+nvidia-proprietary+dgpu `NVIDIA GeForce RTX 4070, 615.71.9` `AMD Ryzen 7 5800X3D 8-Core Processor, 63 GB RAM` Scene: kokiri-default, from a new save; kokiri-parallel, from a new save (RX_VK_PARALLEL=4); each cell's settings and switches as the table names them Method: the Beanstalk test farm: main and this PR built and run on the same PC in one session, game starts alternating (ABBA); kokiri-default: 1 game start a side, forest 120 s after 5 s warm-up; kokiri-parallel: 1 game start a side, forest 120 s after 5 s warm-up; each number the median of the runs, then of 1 report How to reproduce: the farm's plan above (`@Beanstalk` on this PR runs it again) | | Before: main @ aa95d35 | After: this PR @ 2c5a62e | |---|---|---| | kokiri-default · forbidden log lines | 0.0 | 0.0 | | kokiri-parallel · forbidden log lines | 0.0 | 0.0 | ### Flags and settings None: the plan varied no setting, switch or build option. <details><summary>Markdown for the PR description</summary> ```markdown ## Measurements **Verdict:** could not be judged on linux (1/1 report). Hardware: 1: linux+nvidia+nvidia-proprietary+dgpu `NVIDIA GeForce RTX 4070, 615.71.9` `AMD Ryzen 7 5800X3D 8-Core Processor, 63 GB RAM` Scene: kokiri-default, from a new save; kokiri-parallel, from a new save (RX_VK_PARALLEL=4); each cell's settings and switches as the table names them Method: the Beanstalk test farm: main and this PR built and run on the same PC in one session, game starts alternating (ABBA); kokiri-default: 1 game start a side, forest 120 s after 5 s warm-up; kokiri-parallel: 1 game start a side, forest 120 s after 5 s warm-up; each number the median of the runs, then of 1 report How to reproduce: the farm's plan above (`@Beanstalk` on this PR runs it again) | | Before: main @ aa95d35 | After: this PR @ 2c5a62e | |---|---|---| | kokiri-default · forbidden log lines | 0.0 | 0.0 | | kokiri-parallel · forbidden log lines | 0.0 | 0.0 | ### Flags and settings None: the plan varied no setting, switch or build option. ``` </details>

Beanstalk added the

beanstalk: approved @ 2c5a62e

label

2026-11-15 11:20:13 +00:00

mooki added the

labels

2026-11-15 13:59:37 +00:00

dyst commented

2026-11-15 14:56:48 +00:00

Author

Member

The farm's first run (rp_20b7db7482e51065, Linux RTX 4070) judged no scenario: 'a step ran out of time: timeout: the game did not read the controller', on main and on this PR alike, before any measurement, so it says nothing about the change either way. @Beanstalk once more, please: the same plan (Kokiri Forest two minutes, defaults and RX_VK_PARALLEL=4, this PR against main).

👀 1