feat(accuracysnes): publish scenes on a known field (T-04-H) - #299
Conversation
The scene protocol had no frame-parity control, so an interlace scene asked a question whose answer alternates every frame. C7.12 produced three different hashes on three emulators for exactly that reason -- not three cores disagreeing about interlace, but each host landing on whichever field its own frame counter happened to be on. run_scenes now publishes the scene ID only on frames whose $213F bit 7 is set. SCENE_FRAMES grows 8 -> 12 because at half the publication rate the old value put the host's fourth sighting on the window's last frame, one of the two ends the capture protocol exists to avoid. The 53 blessed scenes are unaffected on all three hosts: a still picture hashes the same on either field. With the gate, snes9x and Mesen2 produce the IDENTICAL hash for the new c7-obj-interlace-halves-height scene. The three-way split is a two-way one. The residual is left unblessed and is fully characterised rather than argued about: pixel dumps show both renders agree on everything C7.12 asserts -- the 16x32 sprite occupies 16 display rows -- and differ only in which field's source rows are drawn. RustySNES is not alone on its side; its `row + field` and its $213F bit 7 are both ares', so this is the bsnes/ares lineage against the other two, which is 2 vs 1 under this project's provenance rule and not 2 vs 2. No primary source says which field maps to which rows. Recorded as a variant set, per the rule the Mode-5 hi-res cluster is already under. C9.03/C9.06 are deliberately not written on top of this: screen interlace has the identical parity dependency and they would land unblessed for the identical reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (5)
📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
🧰 Additional context used📓 Path-based instructions (17)**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
tests/roms/AccuracySNES/asm/runtime.inc⚙️ CodeRabbit configuration file
Files:
tests/roms/AccuracySNES/asm/**/*.s📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/roms/AccuracySNES/**/*.s📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/roms/AccuracySNES/asm/runtime.s⚙️ CodeRabbit configuration file
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
tests/roms/AccuracySNES/gen/src/**/*📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/roms/AccuracySNES/gen/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/roms/AccuracySNES/gen/src/**⚙️ CodeRabbit configuration file
Files:
tests/roms/AccuracySNES/gen/src/scenes.rs⚙️ CodeRabbit configuration file
Files:
CHANGELOG.md📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.md⚙️ CodeRabbit configuration file
Files:
docs/**/*.md📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
docs/**/*📄 CodeRabbit inference engine (docs/testing-strategy.md)
Files:
docs/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (4)📚 Learning: 2026-07-22T12:38:06.947ZApplied to files:
📚 Learning: 2026-07-21T02:10:49.581ZApplied to files:
📚 Learning: 2026-07-21T01:34:22.909ZApplied to files:
📚 Learning: 2026-07-21T05:22:58.848ZApplied to files:
🔇 Additional comments (5)
WalkthroughChangesAccuracySNES scene capture
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant run_scenes
participant PPU2_213F as PPU2 status $213F
participant V_SCENE_ID
run_scenes->>PPU2_213F: read field status each frame
alt bit 7 is set
run_scenes->>V_SCENE_ID: publish 1-based scene ID
else bit 7 is clear
run_scenes->>V_SCENE_ID: publish zero
end
Suggested reviewers: 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
Comment |
Antigravity review (Gemini via Ultra)This PR field-gates AccuracySNES scene publication on Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
There was a problem hiding this comment.
Pull request overview
Updates the AccuracySNES rendered-scene protocol to publish scene IDs only on a known PPU field (via $213F bit 7), eliminating host-dependent frame-parity ambiguity for interlaced scenes and enabling consistent cross-emulator hashing. Adds the new interlace-dependent scene C7.12 to validate OBJ interlace behavior under the now field-pinned protocol.
Changes:
- Field-gate the
run_scenespublication window and widenSCENE_FRAMES(8 → 12) to preserve an interior capture sighting. - Add
c7-obj-interlace-halves-height(C7.12) and regenerate the scene catalog/assembly outputs. - Update AccuracySNES planning/coverage docs and the changelog to reflect the new protocol and the newly-characterized (but unblessed) interlace scene.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/roms/AccuracySNES/gen/src/scenes.rs | Adds the new C7.12 scene definition to the generator source. |
| tests/roms/AccuracySNES/build/scenes.tsv | Regenerates the host manifest to include the new scene (index 54). |
| tests/roms/AccuracySNES/asm/scenes.s | Regenerates assembled scene implementation and updates _scene_count/entries. |
| tests/roms/AccuracySNES/asm/runtime.s | Implements field-gated scene-ID publishing in run_scenes. |
| tests/roms/AccuracySNES/asm/runtime.inc | Adds V_SCENE_ID scratch and updates SCENE_FRAMES to 12 with rationale. |
| docs/accuracysnes-plan.md | Records the new field-gating machinery and documents the remaining interlace variant split. |
| docs/accuracysnes-coverage.md | Updates the generated coverage listing to include C7.12 as a scene. |
| CHANGELOG.md | Documents the protocol change and the new interlace scene’s observed behavior across references. |
…m (T-04-H) (#302) C11.03 -- each M7x * ORG product is masked to a multiple of 64 before the terms accumulate -- is now covered by a Mode 7 scene with M7A and M7B both $0101, deliberately not round numbers. With M7B = $0101 the discarded part is `line MOD 64`, a different amount on every line, so a core that accumulates the full products samples a different texel on roughly a quarter of the columns of most lines. Every other Mode 7 scene uses round matrix values, which hide this completely -- none of them was evidence for the row. Blessed at 0xc032679c9076440a, which all three references produce, stable across repeated runs. Non-vacuity confirmed by removing the mask from fetch_mode7_column: this scene's hash moves. Two existing scenes move under that injection too, so the mask was already witnessed in aggregate -- what this adds is a scene whose stated purpose is the mask. RETRACTION. PR #299 claimed the field gate turned C7.12's three-way emulator split into a two-way one, citing snes9x and Mesen2 producing the identical hash. That was ONE RUN and it does not hold: the identical ROM run twice under Mesen2 gives 0x16f7ab8c7f97b7f8 then 0x266241e43ab85064, the two field parities alternating run to run. RustySNES and snes9x are each stable; Mesen2 is not. The gate is necessary but not sufficient and the missing half is host-side: the cart publishes only on its chosen field, but which RENDERED frame a host associates with the R_SCENE value it read is not pinned, and Mesen2's headless runner does not make that association deterministically. The interlace scene is withdrawn rather than left unblessed, because a scene reporting a different hash each run is noise in the gate output. The gate itself stays -- cart-side half, moves no golden, costs only battery runtime. Coverage is unchanged at 350 of 443: C11.03 replaces C7.12 one-for-one, because the withdrawn scene was being counted as C7.12's scene-cover while still unblessed. What changed is that the row now counted has a golden all three references agree on. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The second of the four
v1.29.0machinery items: publish a scene only on a known field — a change torun_scenes, not to a scene.The problem it fixes
The scene protocol had no frame-parity control. An interlace scene therefore asks a question whose answer alternates every frame, and
C7.12(a 16x32 sprite under OBJ interlace renders as 16x16) produced three different hashes on three emulators — the only three-way split any scene has produced. That was never three cores disagreeing about interlace: each host simply landed on whichever field its own frame counter happened to be on.run_scenesnow publishes the scene ID only on frames whose$213Fbit 7 is set, so every sighting the host counts is the same field.SCENE_FRAMESgrows 8 → 12, because at half the publication rate the old value put the host's fourth sighting on the window's last frame — one of the two ends the capture protocol exists to avoid.The 53 blessed scenes are unaffected on all three hosts. A still picture hashes the same on either field, so gating only moves which wall-clock frame the fourth sighting is.
It did what it was built to do
c7-obj-interlace-halves-heightisc7-objsel-size-6with oneSETINIbit changed and nothing else, so the pair differs by exactly OBJ interlace.0x266241e43ab850640x266241e43ab850640x16f7ab8c7f97b7f8Three-way split → two-way. The two references, which share no code and no host, now produce the identical picture.
The residual, characterised rather than argued about
Pixel dumps localise it completely. Both renders agree on everything
C7.12asserts — the 16x32 sprite occupies 16 display rows, its 32x64 neighbour 32 — and differ only in which field's source rows are drawn: the references take the even ones, RustySNES the odd. One source row, nothing else.Left unblessed, and not called a RustySNES bug. "RustySNES alone" is this project's signature for a real defect, and RustySNES is not alone: its
row + fieldand its$213Fbit 7 are both ares' (sfc/ppu/object.cpp:122,sfc/ppu/io.cpp:178), and both cores toggle the field at the same V wrap. So this is the bsnes/ares lineage against snes9x and Mesen2 — 2 vs 1 under this project's own provenance rule, not 2 vs 2 — and no primary source says which field maps to which rows. Recorded as a variant set, the same treatment the Mode-5 hi-res cluster already gets. The tiebreaker is ares actually running the cart, the same blocked itemA2.10waits on.C9.03/C9.06are deliberately not written on top of this. Screen interlace carries the identical parity dependency, so they would land unblessed for the identical reason; two more unblessable scenes is not progress.Verification
cargo fmt --check,cargo clippy --workspace --exclude rustysnes-android --all-targets -- -D warnings— clean.cargo test -p rustysnes-test-harness --test accuracysnes --features test-roms— 56 passed, 0 failed.cargo test -p rustysnes-test-harness --test accuracysnes_scenes --features test-roms— 53 blessed match, 1 unblessed.REF_PROJ=$PWD/ref-proj bash scripts/accuracysnes/crossval.sh—snes9x: OK (14 known),Mesen2: OK (2 known),53 scene(s) match, 1 unblessed, 0 mismatchedon both,cross-validation: 2 reference(s) agree with the cart.🤖 Generated with Claude Code
Summary
run_scenesnow publishes scene IDs only when$213Fbit 7 is set. This fixes inconsistent field parity for interlace scenes. The defect occurs when an interlace scene spans fields with different parity.The change adds dossier assertion
C7.12(c7-obj-interlace-halves-height).SCENE_FRAMESincreases from 8 to 12. The blessed-scene coverage denominator remains 53 becauseC7.12remains an unblessed variant.The claim is false if scene IDs publish on frames with bit 7 clear, if snes9x and Mesen2 produce different
C7.12hashes, or if the 53 existing blessed scenes change. RustySNES still differs in selected source rows.C9.03andC9.06remain intentionally unblessed.