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/accuracysnes-plan.md b/docs/accuracysnes-plan.md index 7c9a1d23..962fb5cb 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 @@ -642,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 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