From 5620f71a2058a708b3eebda4b17846c8ad69e2d0 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 20:58:46 -0400 Subject: [PATCH 1/3] feat(ppu): model the long scanline (B2.03) PAL, interlace on, field set, V=311 is 1368 master clocks and 341 dots. Its shape is the opposite of the short line's, which is why these are two changes and not one: B2.02 substitutes dot lengths, while this appends a whole extra 4-clock dot and leaves the two 6-clock dots alone (339*4 + 2*6 = 1368). So it moves the H wrap, via a new Ppu::dots_this_line, rather than the Bus's clock table -- and the H counter reaches dot 340 on that line and on no other. The H-IRQ comparator's upper bound follows the same per-line count instead of the constant, so an HTIME landing on dot 340 is a genuine match on the long line and stays suppressed everywhere else. It could previously never match anywhere. PAL interlaced frames alternate 425,568 / 425,572. The test enables interlace through SETINI $2133 bit 0 rather than poking the field, and asserts the SET of frame lengths rather than their order -- which phase carries the extra 4 clocks depends on the field's power-on value, and pinning that would pin an initial condition this row does not care about. Progressive PAL is asserted separately to be unaffected. Both fail under their own injection. 872 workspace tests pass; no golden moved, the long line being in vblank. Separate and still unmodelled: the interlaced frame's extra scanline (263/313 vs 262/312). B2.03 is reachable without it since V=311 is the last PAL line either way. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 24 +++++++++++++++ crates/rustysnes-core/src/bus.rs | 52 ++++++++++++++++++++++++++++++++ crates/rustysnes-ppu/src/lib.rs | 36 ++++++++++++++++++++-- docs/ppu.md | 8 +++-- docs/scheduler.md | 16 +++++++--- 5 files changed, 127 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 310bfbf0..ba3fa9a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,6 +116,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `docs/adr/0013`'s "bless only from a render the references agree on" cannot be satisfied for this ROM in either direction until that 23-row gap is explained. +- **Dot model: the long scanline (`B2.03`).** PAL, interlace on, field set, `V = 311` is 1368 master + clocks and **341** dots. Its shape is the *opposite* of the short line's, which is why the two are + separate changes rather than one: `B2.02` substitutes dot lengths, while this appends a whole extra + 4-clock dot and leaves the two 6-clock dots alone (`339 x 4 + 2 x 6 = 1368`). So it moves the H + wrap (`Ppu::dots_this_line`) rather than the Bus's clock table, and the H counter reaches dot 340 + on that line and on no other. + + The H-IRQ comparator's upper bound now follows the same per-line count instead of the constant, so + an `HTIME` landing on dot 340 is a genuine match on the long line and stays suppressed elsewhere — + previously it could never match anywhere. + + PAL interlaced frames alternate **425,568 / 425,572** clocks. The test enables interlace through + `SETINI $2133` bit 0 rather than poking the field, so it drives the path a game would, and asserts + the *set* of frame lengths rather than their order — which phase carries the extra 4 clocks depends + on the field's power-on value, and pinning that would pin an initial condition the row does not + care about. Progressive PAL is asserted separately to be unaffected. Both fail under their own + injection. + + Full workspace suite **872 passed, 0 failed**; no golden moved, the long line being in vblank. + + **Separate and still unmodelled:** the interlaced frame's extra *scanline* (263/313 rather than + 262/312). `Region::lines_per_frame` is the non-interlaced count by documentation, and `B2.03` is + reachable without it since `V = 311` is the last PAL line either way. + - **Dot model: the short scanline is now modelled (`B2.02`).** NTSC, progressive, field set, `V = 240` is 1360 master clocks rather than 1364 — under this project's measured convention that means the two 6-clock dots (323, 327) are **not** long on that line, leaving 340 dots of 4, which diff --git a/crates/rustysnes-core/src/bus.rs b/crates/rustysnes-core/src/bus.rs index b902c798..213b5b2f 100644 --- a/crates/rustysnes-core/src/bus.rs +++ b/crates/rustysnes-core/src/bus.rs @@ -1601,6 +1601,58 @@ impl core::fmt::Debug for Bus { mod tests { use super::*; + /// Dossier `B2.03`: PAL with interlace on has a 1368-clock, **341**-dot scanline at `V = 311` + /// on field-set frames, so the PAL frame alternates 425,568 / 425,572 master clocks. + /// + /// Interlace is enabled through `SETINI $2133` bit 0 rather than by poking the field directly, + /// so the test drives the same path a game would. + #[test] + fn the_pal_interlaced_frame_gains_the_long_scanline() { + let mut bus = Bus::new(Region::Pal); + bus.ppu.write_reg(0x2133, 0x01); + let mut frame = bus.ppu.frame_count(); + let mut last = bus.clock.master; + let mut lens = std::vec::Vec::new(); + while lens.len() < 4 { + bus.advance_master(MASTER_PER_DOT); + if bus.ppu.frame_count() != frame { + frame = bus.ppu.frame_count(); + lens.push(bus.clock.master - last); + last = bus.clock.master; + } + } + // One of the two phases carries the extra 4 clocks; which one depends on the field's + // power-on value, so assert the SET rather than a fixed order -- pinning the order would + // be pinning an initial condition this row does not care about. + let mut kinds = lens.clone(); + kinds.sort_unstable(); + kinds.dedup(); + assert_eq!( + kinds, + std::vec![425_568, 425_572], + "PAL interlaced frames must alternate by the 4 clocks the long scanline adds, got {lens:?}" + ); + } + + /// The long line is PAL+interlace only. Progressive PAL -- the default -- must not pick it up, + /// which is what separates it from `B2.02`'s NTSC case. + #[test] + fn progressive_pal_does_not_gain_the_long_scanline() { + let mut bus = Bus::new(Region::Pal); + let mut frame = bus.ppu.frame_count(); + let mut last = bus.clock.master; + let mut lens = std::vec::Vec::new(); + while lens.len() < 3 { + bus.advance_master(MASTER_PER_DOT); + if bus.ppu.frame_count() != frame { + frame = bus.ppu.frame_count(); + lens.push(bus.clock.master - last); + last = bus.clock.master; + } + } + assert_eq!(lens, std::vec![425_568, 425_568, 425_568]); + } + /// Dossier `B2.02`: the NTSC progressive frame alternates 357,368 / 357,364 master clocks, /// because scanline 240 of every other frame is 1360 clocks rather than 1364. /// diff --git a/crates/rustysnes-ppu/src/lib.rs b/crates/rustysnes-ppu/src/lib.rs index ad72df9d..a083e064 100644 --- a/crates/rustysnes-ppu/src/lib.rs +++ b/crates/rustysnes-ppu/src/lib.rs @@ -926,7 +926,7 @@ impl Ppu { self.pd_eval_over_flags(); self.h += 1; - if self.h >= DOTS_PER_LINE { + if self.h >= self.dots_this_line() { // End of a scanline: advance V and fire frame/VBlank events. Rendering already // happened above at RENDER_DOT, not here. self.h = 0; @@ -989,7 +989,10 @@ impl Ppu { // the next line, which would be a spurious match hardware/ares never produce. let h_target = self.irq_h + HIRQ_TRIGGER_DELAY; let h_match = if self.irq_enable_h { - h_target < DOTS_PER_LINE && self.h == h_target + // Bounded by THIS line's dot count, not the constant: the long line has a dot 340 that + // a normal line does not, so an `HTIME` landing there is a real match on that line and + // a suppressed one everywhere else. + h_target < self.dots_this_line() && self.h == h_target } else { // V-only IRQ: the comparator is sampled once near the start of the line, not held // across it. Modelling `h_match` as unconditionally true here made `V == VTIME` a @@ -1095,6 +1098,35 @@ impl Ppu { matches!(self.region, Region::Ntsc) && !self.io.interlace && self.field && self.v == 240 } + /// The scanline being generated right now is the **long** one — 1368 master clocks and + /// **341** dots instead of 340, dossier `B2.03`. + /// + /// PAL, interlace on, field set, `V = 311`. The shape differs from the short line and that is + /// the whole point: the short line substitutes dot *lengths* (its two 6-clock dots are not long + /// there), whereas this one appends an extra 4-clock dot and leaves the two long dots alone — + /// `339 x 4 + 2 x 6 = 1368`. So it moves the H wrap ([`Ppu::dots_this_line`]) rather than the + /// Bus's clock table, which is why it is a separate change from `B2.02`. + /// + /// Like the short line it sits in vblank, so no visible pixel moves. + #[must_use] + pub const fn is_long_scanline(&self) -> bool { + matches!(self.region, Region::Pal) && self.io.interlace && self.field && self.v == 311 + } + + /// Dots in the scanline being generated right now — [`DOTS_PER_LINE`], or one more on the long + /// line. + /// + /// The H counter runs `0..dots_this_line()`, so on the long line it reaches 340, a value it + /// takes on no other line. + #[must_use] + pub const fn dots_this_line(&self) -> u16 { + if self.is_long_scanline() { + DOTS_PER_LINE + 1 + } else { + DOTS_PER_LINE + } + } + /// Latch the current H/V dot counters into `OPHCT`/`OPVCT` ($213C/$213D) right now — the same /// effect a CPU read of `SLHV` ($2137) has (`regs.rs`'s `0x2137` arm calls this too, so there /// is one implementation of the latch itself). Real hardware also drives this from the WRIO diff --git a/docs/ppu.md b/docs/ppu.md index 3fbc3b91..6917d283 100644 --- a/docs/ppu.md +++ b/docs/ppu.md @@ -90,9 +90,11 @@ the rest four**; see `docs/scheduler.md` "Convention (binding)"): - Normal line = 1364 clocks = 340 dots (`338 × 4 + 2 × 6`); short = 1360 (NTSC non-interlace, V=240 alt frames); long = 1368 (PAL interlace field=1 V=311). 262/312 lines (NTSC/PAL), +1 interlaced. - The **short** line is modelled (`Ppu::is_short_scanline`): all 340 dots at 4, so the two 6-clock - dots are not long there, and the NTSC frame alternates 357,368 / 357,364 clocks. The **long** line - is not — it has 341 dots, one more than normal, which changes the H wrap rather than a dot length. + Both are modelled. The **short** line (`Ppu::is_short_scanline`) puts all 340 dots at 4, so its + two 6-clock dots are not long, and the NTSC frame alternates 357,368 / 357,364. The **long** line + (`Ppu::is_long_scanline`) instead appends an extra 4-clock dot, keeping the two long ones, so it + has 341 dots and moves the H wrap (`Ppu::dots_this_line`); PAL interlaced frames alternate + 425,568 / 425,572. The interlaced frame's extra *scanline* is a separate, still-unmodelled thing. - Active output dots 22–277 on lines 1–224 (or 1–239 overscan). **VBlank** at V=225 (or V=240 overscan). **HBlank** H=274→H=1. - **H/V latch:** read **SLHV $2137** latches H/V; **OPHCT $213C / OPVCT $213D** read twice for diff --git a/docs/scheduler.md b/docs/scheduler.md index b96f6443..38b1e63b 100644 --- a/docs/scheduler.md +++ b/docs/scheduler.md @@ -106,10 +106,18 @@ pins it. "Every other frame" is keyed on the field flag, following anomie's *"th (`docs/ppu.md` §Field flag). No visible pixel moves: the short line is inside vblank, and the whole 858-test workspace suite, every blessed scene, and both `hdmaen_latch_test` goldens were unchanged. -**Still unmodelled:** the long line (PAL interlace, field 1, V=311 — 1368 clocks, **341** dots, one -more than a normal line). That extra dot is a structural change to the H wrap rather than a clock -substitution, which is why it is separated from the short line rather than landed alongside it. -`B2.03` remains uncovered. +**The long line is modelled too** (`B2.03`): PAL, interlace on, field set, `V = 311` — 1368 clocks +and **341** dots. Its shape is the opposite of the short line's and that is why the two landed +separately: the short line substitutes dot *lengths*, while this one **appends a whole extra 4-clock +dot** and leaves the two 6-clock dots alone (`339 x 4 + 2 x 6 = 1368`). So it moves the H wrap +(`Ppu::dots_this_line`) rather than the Bus's clock table, and the H counter reaches 340 on that line +and on no other. The H-IRQ comparator's upper bound follows the same per-line count, so an `HTIME` +landing on dot 340 is a real match there and suppressed everywhere else. PAL interlaced frames +alternate 425,568 / 425,572. + +**Still unmodelled, and separate:** the interlaced frame's extra *scanline* (263 / 313 rather than +262 / 312). `Region::lines_per_frame` is documented as the non-interlaced count and nothing varies +it, so `B2.03` above is reachable without it — `V = 311` is the last PAL line either way. ## DMA / HDMA bus-steal From 34013008d9d6a9da118ccd6f1d0a6d8bf0ee7cdb Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 21:00:14 -0400 Subject: [PATCH 2/3] docs(accuracysnes): B2.02/B2.03 landed; B2.07 needs its own design Both scanline variants are modelled now, so the section that recorded a doubt about reachability is replaced with what was actually built and what remains. The part worth writing down: B2.07 does NOT follow from the model change. The frame-length difference is 4 master clocks -- one dot -- and the hv_begin/hv_end instrument costs a measured 175 dots, so resolving it needs a differential across many frames or a different instrument. That is the same wall A5.18 is parked on, and assuming B2.07 is now easy would waste a session. Co-Authored-By: Claude Opus 5 (1M context) --- docs/accuracysnes-plan.md | 49 +++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/docs/accuracysnes-plan.md b/docs/accuracysnes-plan.md index 7c9a1d23..605d7217 100644 --- a/docs/accuracysnes-plan.md +++ b/docs/accuracysnes-plan.md @@ -605,26 +605,35 @@ 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. +#### `B2.02`/`B2.03` — both landed in the emulator; the cart rows are the remaining half + +The doubt this section recorded — whether the short-line gate was reachable at all, given `field`'s +doc claimed it only toggles under interlace — was settled against the source: the comment was stale, +the code toggles unconditionally, and that is now pinned by a test. + +Both scanlines are now modelled, as two separate changes because their **shapes are opposite**: + +| | `B2.02` short | `B2.03` long | +|---|---|---| +| when | NTSC, progressive, field, `V=240` | PAL, interlace, field, `V=311` | +| clocks / dots | 1360 / 340 | 1368 / **341** | +| mechanism | the two 6-clock dots are **not long** | an extra 4-clock dot is **appended** | +| touches | the Bus's clock table | the PPU's **H wrap** | + +Observable: NTSC frames alternate 357,368 / 357,364; PAL interlaced frames 425,568 / 425,572. No +golden moved — both lines are in vblank. A related latent bug fell out of `B2.03`: `check_hv_irq` +bounded the H match with the 340 constant, so an `HTIME` landing on the long line's dot 340 could +never match anywhere; it now uses the per-line count. + +**What is still open is the cart side.** `B2.07` (NTSC 60.0988 Hz) is the row this was supposed to +unblock, and it is not a quick follow-on: the frame-length difference is **4 master clocks = one +dot**, and the `hv_begin`/`hv_end` instrument costs a measured 175 dots. Measuring a one-dot +difference in a quantity that large needs either a differential across many frames or a different +instrument — the same problem `A5.18` is parked on. Do not assume `B2.07` follows from the model +change; it needs its own design. + +Also still unmodelled and deliberately separate: the interlaced frame's extra **scanline** (263/313 +rather than 262/312). `B2.03` does not need it, since `V = 311` is the last PAL line either way. #### Group E — the batch to author next, picked against what the oracle can adjudicate From e09aa2b7a63977bce57873807accf3b035c37a88 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 22:52:59 -0400 Subject: [PATCH 3/3] docs(accuracysnes): work out the E8.01 design before authoring it Records the concrete design rather than leaving the next session to restart from the dossier's one-line assertion. E8.01 follows e8_02's shape, which already solves the hard parts: e8_02_sounding_voice builds the voice, e8_02_time_to_envx counts timer-2 ticks to a non-zero ENVX, and T2DIV = 2 gives exactly one tick per output sample. The assertion is a DIFFERENTIAL: write KON twice, the second offset by one sample (32 SPC cycles), and compare the delays. Polled every second sample means they differ by 1; polled every sample means they are equal. So the row asserts "the two differ", which assert_a16_range expresses -- one of the cases the never-hand-write-a-verdict-byte rule exists for. Carries forward both traps e8_02 already paid for: the voice must be fully silent before the second key-on (reuse its arming guard, do not re-derive it), and T2DIV must stay 2 because 1 puts the reading next to TnOUT's wrap and the NTSC/PAL drift gate catches it on PAL alone. Verify by injecting at the named site -- force the KON poll to every sample in the DSP and confirm this row's code fires. Co-Authored-By: Claude Opus 5 (1M context) --- docs/accuracysnes-plan.md | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/accuracysnes-plan.md b/docs/accuracysnes-plan.md index 605d7217..962fb5cb 100644 --- a/docs/accuracysnes-plan.md +++ b/docs/accuracysnes-plan.md @@ -651,9 +651,36 @@ Chosen 2026-08-01, now that both halves of the Mesen2 oracle arbitrate. Two corr | row | assertion | why first | |---|---|---| -| `E8.01` | KON/KOFF polled every **second** sample (16 kHz) `[ERRATA]` | pure DSP-register observable; the errata flag means the reference is explicit, and a 1-sample granularity is measurable through ENDX/ENVX without new opcodes | -| `E9.02` | noise output is **highpass-filtered** `[ERRATA]` | DSP-observable via OUTX on a NON voice; errata-flagged, so the expected behaviour is stated rather than inferred | -| `E5.06` | BRR 15-bit wrap: clamp to 16 bits, then `+4000h..+7FFFh → -4000h..-1` | already has decoder scaffolding from the landed `E5` rows | +| `E8.01` | KON/KOFF polled every **second** sample (16 kHz) `[ERRATA]` | pure DSP-register observable; errata-flagged so the reference is explicit | +| `E9.02` | noise output is **highpass-filtered** `[ERRATA]` | DSP-observable via OUTX on a NON voice; errata-flagged | +| `E5.06` | BRR 15-bit wrap: clamp to 16 bits, then `+4000h..+7FFFh → -4000h..-1` | reuses the landed `E5` decoder scaffolding | + +**`E8.01` design, worked out so the next session does not restart from the assertion text.** Follow +`e8_02`'s shape (`apu.rs:5298`), which already solves the hard parts — `e8_02_sounding_voice` builds +the voice, `e8_02_time_to_envx` counts timer-2 ticks to a non-zero `ENVX`, and `T2DIV = 2` gives +**exactly one tick per output sample**. + +The assertion is a **differential**, not an absolute: write `KON` twice, the second offset by **one +sample (32 SPC cycles)** from the first, and compare the two measured delays. + +- polled every **second** sample ⇒ the two delays differ by **1** (one write waits for the next + even poll, the other does not); +- polled every sample ⇒ they are **equal**. + +So the row asserts *"the two differ"*, which `assert_a16_range` expresses fine — and note this is one +of the cases the `never hand-write a verdict byte` rule exists for. + +Two traps carried over from `e8_02`, both already paid for once: +- the voice must be **fully silent** before the second key-on. A full-scale envelope is `$7F0` and + release steps down 8/sample, so ~254 samples; `e8_02`'s first version polled while `ENVX` was still + `~$40` and measured a delay of one tick. Reuse its arming guard (`dsp_read_to(0x08, …)` must read + zero) rather than re-deriving it. +- `T2DIV = 1` is finer but puts the reading at 15 of `TnOUT`'s 16 values, close enough to the wrap + that the NTSC/PAL drift gate caught it on the PAL image alone. Keep `T2DIV = 2`. + +**Verify by injection at the named site**: force the KON poll to every sample in +`rustysnes-apu`'s DSP and confirm *this* row's code fires — per [[accuracysnes-attribution-trap]], a +row that passes without moving under its own named injection is measuring something else. **Explicitly NOT first: `E1.11`** ("TSET1/TCLR1 read the target twice"). Inspected and set aside as a likely **vacuous** row: the second read is a dummy, and on a `$FD`-`$FF` timer-output target the