From 1504241109983a3e23a830df1adc269250dba4c8 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 19:50:50 -0400 Subject: [PATCH 1/2] fix(ppu): the field flag toggles every frame, not only when interlaced `Ppu::field` is `$213F` bit 7 and is toggled unconditionally at frame end, but its doc comment claimed it toggles "when interlace is on". Only the flag's use is interlace-conditional -- interlace consumes it to select the odd/even row. The stale reading is load-bearing rather than cosmetic: it makes the short/long-scanline gate that keys on the field look unreachable in progressive mode, which is exactly the v1.29.0 prerequisite for B2.02 and B2.03. Correct the comment and docs/ppu.md, and pin the behaviour with a test that ticks four progressive frames and requires bit 7 to alternate. Injecting the gate the old doc described makes it fail with a constant [0,0,0,0]. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 12 +++++++++++ crates/rustysnes-ppu/src/lib.rs | 36 ++++++++++++++++++++++++++++++++- docs/ppu.md | 5 +++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1476e88f..be967d90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **PPU: the field flag's doc contradicted the code, and nothing pinned either.** `Ppu::field` is + `$213F` bit 7 and toggles at the end of **every** frame; its doc comment said "toggles each frame + when interlace is on", which describes neither the code nor the hardware. Only the flag's *use* is + interlace-conditional (picking the odd/even row). The stale reading matters beyond tidiness: it + makes the short/long-scanline gate that keys on the field look unreachable in progressive mode + when it is not, which is a `v1.29.0` prerequisite for `B2.02`/`B2.03`. + + Corrected in the code, in `docs/ppu.md`, and pinned by a test that ticks four progressive frames + and requires bit 7 to alternate. Injecting the gate the old doc described (`if self.io.interlace`) + makes it fail with a constant `[0,0,0,0]`, so the test measures the behaviour rather than + restating it. + - **App Store §4.7 self-audit done (`v1.30.0`).** Checked against the shipped tree rather than against intent. Two of the three risk classes are clean: there is **no ROM acquisition path** (`load_rom` takes bytes from the caller; no download, fetch or URL anywhere in the mobile crates), diff --git a/crates/rustysnes-ppu/src/lib.rs b/crates/rustysnes-ppu/src/lib.rs index fe1249ce..cfda5074 100644 --- a/crates/rustysnes-ppu/src/lib.rs +++ b/crates/rustysnes-ppu/src/lib.rs @@ -648,7 +648,14 @@ pub struct Ppu { h: u16, /// Vertical scanline counter, 0..=(lines_per_frame-1). v: u16, - /// Interlace field (toggles each frame when interlace is on). + /// The PPU2 field flag — `$213F` bit 7. + /// + /// It toggles at the end of **every** frame, interlaced or not ([`Ppu::end_of_scanline`]); + /// only its *use* is conditional. Interlace consumes it to pick the odd/even row + /// (`render.rs`), and it is readable through `$213F` regardless. The doc here previously said + /// "toggles each frame when interlace is on", which describes neither the code nor the + /// hardware, and would make the short/long-scanline gate that keys on it look unreachable in + /// progressive mode when it is not. field: bool, /// Currently inside vertical blank. vblank: bool, @@ -1327,6 +1334,33 @@ mod tests { assert_eq!(p.region, Region::Ntsc); } + #[test] + fn the_field_flag_toggles_every_frame_even_in_progressive_mode() { + // Pins the `field` doc: the flag is `$213F` bit 7 and toggles unconditionally. Only its + // *use* is interlace-conditional. A model that gated the toggle on `io.interlace` would + // leave bit 7 constant here, and would also make the short/long-scanline gate that keys + // on `field` unreachable in progressive mode -- which is why this is pinned rather than + // left as a comment. + let mut p = Ppu::new(); + assert!(!p.io.interlace, "this test is about the progressive case"); + + let mut bus = NullVideoBus; + let mut seen = [0u8; 4]; + for slot in &mut seen { + let before = p.frame_count; + while p.frame_count == before { + p.tick_dot(&mut bus); + } + *slot = p.read_reg(0x213F) & 0x80; + } + + assert_eq!( + seen, + [0x80, 0x00, 0x80, 0x00], + "bit 7 must alternate on every frame boundary, not stay put" + ); + } + #[test] fn full_state_round_trips_through_save_state() { let mut p = Ppu::with_region(Region::Pal); diff --git a/docs/ppu.md b/docs/ppu.md index 220466c6..2c995a13 100644 --- a/docs/ppu.md +++ b/docs/ppu.md @@ -94,6 +94,11 @@ the rest four**; see `docs/scheduler.md` "Convention (binding)"): overscan). **HBlank** H=274→H=1. - **H/V latch:** read **SLHV $2137** latches H/V; **OPHCT $213C / OPVCT $213D** read twice for the 9-bit values; **STAT78 $213F** read clears the latch + resets the toggle. +- **Field flag ($213F bit 7):** toggles at the end of **every** frame, interlaced or not. Only its + *use* is conditional — interlace consumes it to select the odd/even row. A model that gated the + toggle itself on interlace would leave bit 7 constant in progressive mode, and would make any + short/long-scanline behaviour keyed on the field unreachable there. Pinned by + `the_field_flag_toggles_every_frame_even_in_progressive_mode`. ## CGRAM, VRAM, color math, windows From d332947850e50f698d28114c98bf470e4e9997de Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 19:53:57 -0400 Subject: [PATCH 2/2] docs(accuracysnes): the short-line gate is reachable; record what remains v1.29.0's plan doubted whether B2.02's short-line gate could be reached at all, because `field` is one of its inputs and that field's doc comment said it only toggles when interlace is on. Settled against the source: the code toggles it unconditionally, which is what $213F bit 7 does; the comment was stale. All four gate inputs are live. Record that, and that the model change itself is not started -- and why: `dot_length` is a pure function of the dot and cannot express a per-line variation, so the line context has to be threaded to it, and the guard test lands before it is touched. Co-Authored-By: Claude Opus 5 (1M context) --- docs/accuracysnes-plan.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/accuracysnes-plan.md b/docs/accuracysnes-plan.md index 5c209069..6bd73d50 100644 --- a/docs/accuracysnes-plan.md +++ b/docs/accuracysnes-plan.md @@ -605,6 +605,27 @@ before the H counter wraps and silently returns a plausible small number. | `B2.07` | gated on `B2.02` | `B2.04` covers the 262-line count, not the clock total, so the frequency is not implied by it. Getting to 60.0988 Hz needs clocks-per-frame, and the frame alternates 357,368/357,364 because of the short scanline — which *is* `B2.02`, a `T-06-A` dot-model residual scheduled for `v1.29.0`. | | `B2.09` | `v1.29.0` by its own dossier note | the picture window is "not CPU-observable directly; reachable through the framebuffer oracle once the dot-resolution compositor lands". | +#### `B2.02`/`B2.03` — the prerequisite is cleared, the model change is not started + +The plan for `v1.29.0` recorded a doubt about whether the short-line gate was even reachable: +`field` is one of its four inputs, and `Ppu::field`'s doc comment said it "toggles each frame when +interlace is on", which would leave it constant in the progressive NTSC case `B2.02` needs. + +**Settled against the source: the doc was wrong, the code is right.** `end_of_scanline` toggles +`field` unconditionally, which is what `$213F` bit 7 does on hardware; only the flag's *use* is +interlace-conditional (`render.rs` picks the odd/even row with it). The comment and `docs/ppu.md` +are corrected, and `the_field_flag_toggles_every_frame_even_in_progressive_mode` pins it — +injecting the `if self.io.interlace` gate the old comment described makes that test fail with a +constant `[0,0,0,0]`. So all four gate inputs (`io.interlace`, `field`, `v`, `region`) are live and +the gate is reachable. + +What is **not** done is the model change itself. `dot_length` (`rustysnes-core::bus`) is a pure +function of the dot, so it cannot express a per-line variation; the short line (1360 clocks, all 340 +dots at 4 — the long dots vanish) and the long line (1368, 341 dots) both need the line context +threaded to it. Expect goldens to move, and note that a previous attempt at the neighbouring H-IRQ +mapping change was reverted for exactly that reason — so the guard test named in the roadmap lands +**first**, before `dot_length` is touched. + #### `A5.18` — why it is parked despite working `BRK` cannot be measured without `RTI`: a `BRK` that never returns cannot be repeated, so any