fix(ppu): derive the H-IRQ dot from the clock, not a constant (T-06-A) (#300)

* fix(ppu): derive the H-IRQ dot from the clock, not a constant (T-06-A)

HIRQ_TRIGGER_DELAY = 4 was a dot-domain rounding of ares'
`hcounter(10) == (HTIME+1)<<2`, exact only while every dot is four clocks --
which stopped being true when dots 323 and 327 became six. The match is now
computed where it happens: clock 4*HTIME + 14, mapped to the first dot
boundary at or after it.

Below the long dots the two agree EXACTLY, because 4*HTIME + 14 is never a
multiple of 4 and the next boundary is HTIME + 4. They diverge only for HTIME
321..=337, where the six-clock dots have displaced every later boundary: the
old constant fired up to a whole dot late, and at HTIME 336 suppressed an IRQ
that does fire. HTIME 337 lands on dot 340's boundary, which exists only on
the long line, so it is honoured there and suppressed elsewhere.

The plan records this change as attempted and reverted once because it moved
hdmaen_latch_test_2's golden. It does not this time: no framebuffer golden
moved, the undisbeliever suite passes unchanged, and cross-validation is
byte-identical.

B4.16 is a weaker guard than its own doc claimed. Measured either side of the
change both its readings are unchanged, including the HTIME = 330 one whose
trigger dot moved 334 -> 333: the CPU takes an IRQ at an instruction boundary,
so the handler-entry dot quantises to the spin loop's instruction length and a
one-dot shift is absorbed. It can say "nothing regressed"; it cannot say "the
change took effect". The new unit test does that, sweeping every HTIME and
asserting equality with the old constant below the long dots and strict
inequality above them.

LONG_DOTS and the per-dot clock count move to rustysnes-ppu, which owns the
dot model and now needs the layout twice; the scheduler delegates rather than
keeping a second copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(roadmap,plan): close T-06-A's H-IRQ item and correct two predictions

The roadmap predicted the clock-domain conversion "would shift IRQ timing by
~2 clocks" and re-bless framebuffer goldens. It shifts nothing below the long
dots and moved no golden; only HTIME 321..=337 changes.

The plan predicted the change was "the likeliest thing under B1.05's
residual". It is not, and B4.16 shows why directly: its HTIME = 330 trigger
dot moved 334 -> 333 and its recorded reading did not move, because the
handler-entry dot quantises to the spin loop's instruction length. B1.05 stays
blocked on the same missing clock-domain instrument.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: doublegate

CHANGELOG.md

@@ -11,6 +11,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **The H-IRQ comparator moves into the clock domain (`T-06-A`), and nothing below the long dots
+  moves with it.** `HIRQ_TRIGGER_DELAY = 4` was a *dot-domain rounding* of ares'
+  `hcounter(10) == (HTIME+1)<<2` — exact only while every dot is four clocks, which stopped being
+  true when dots 323 and 327 became six. The match is now computed where it actually happens:
+  clock `4·HTIME + 14` (`hirq_match_clock`), mapped to the first dot boundary at or after it
+  (`hirq_trigger_dot`).
+
+  **Below the long dots the two agree exactly**, because `4·HTIME + 14` is never a multiple of 4 and
+  the next boundary is `HTIME + 4`. They diverge only for `HTIME` **321..=337**, where the six-clock
+  dots have displaced every later boundary — the old constant fired up to a whole dot late, and at
+  `HTIME = 336` suppressed an IRQ that does fire. `HTIME = 337` lands on dot 340's boundary, which
+  exists only on the long line, so it is honoured there and suppressed elsewhere.
+
+  This is the change the plan recorded as *"attempted and reverted because it moves
+  `hdmaen_latch_test_2`'s golden"*. It does not, this time: **no framebuffer golden moved**, the
+  undisbeliever suite passes unchanged, and cross-validation is byte-identical
+  (`snes9x: OK (14 known)`, `Mesen2: OK (2 known)`).
+
+  **`B4.16` is a weaker guard than its own doc claimed, and that is worth knowing.** Measured either
+  side of the change, *both* of its readings are unchanged — including the `HTIME = 330` one, whose
+  trigger dot moved 334 → 333. The CPU takes an IRQ at an instruction boundary, so the handler-entry
+  dot quantises to the spin loop's instruction length and a one-dot shift is absorbed. `B4.16` can
+  say "nothing regressed"; it cannot say "the change took effect". A unit test does that, sweeping
+  every `HTIME` and asserting equality with the old constant below the long dots and strict
+  inequality above them.
+
+  `LONG_DOTS` and the per-dot clock count also move to `rustysnes-ppu`, which owns the dot model and
+  now needs the same layout twice. `rustysnes-core`'s scheduler delegates to it rather than keeping
+  a second copy.
+
 - **AccuracySNES: the scene protocol publishes on a known field, and the interlace three-way split
   is down to a two-way one.** `run_scenes` now sets the scene ID only on frames whose `$213F` bit 7
   is set, so every sighting the host counts is the same field. `SCENE_FRAMES` grew 8 → 12: at half

crates/rustysnes-core/src/bus.rs

@@ -68,20 +68,11 @@ const RDNMI_OPEN_BUS_MASK: u8 = 0x70;
 const TIMEUP_OPEN_BUS_MASK: u8 = 0x7F;
 const HVBJOY_OPEN_BUS_MASK: u8 = 0x3E;
 
-/// The two dots that take **6** master clocks instead of 4 (`T-06-A`).
-///
-/// Hardware's scanline is 340 dots and 1364 master clocks, which `338 × 4 + 2 × 6` satisfies and a
-/// uniform `341 × 4` also satisfies — which is why a model can be wrong here and keep perfect frame
-/// timing. fullsnes' *PPU H-Counter-Latch Quantities* histogram settles it by measurement rather
-/// than prose: sampling `$2137` once per master clock across a line reports dots 323 and 327
-/// latching **six** times each, dot 340 **never**, and everything else four. bsnes, ares and Mesen2
-/// all implement exactly this; snes9x uses 322/326 and is the outlier.
-///
-/// Both sit deep in hblank — past the visible window (dots 22-277), past hblank's start at 274, and
-/// past [`HDMA_RUN_DOT`] — so dots `0..=322` keep their previous clock alignment exactly and no
-/// rendered pixel or HDMA transfer moves. What does change is the `OPHCT`/`$213C` latch value for
-/// `H >= 323`, which was up to one whole dot early.
-const LONG_DOTS: [u16; 2] = [323, 327];
+// The two 6-clock dots (`T-06-A`) used to be declared here. They now live in the PPU
+// (`rustysnes_ppu::LONG_DOTS` / `dot_clocks`), which owns the dot model and needs the same layout
+// to place the H-IRQ comparator — and two copies of "which dots are six clocks" is exactly the kind
+// of fact that drifts apart. The measurement that settles it, and why hblank is where they sit, are
+// documented at the declaration.
 
 /// Master clocks the dot currently being completed lasts for.
 ///
@@ -92,11 +83,7 @@ const LONG_DOTS: [u16; 2] = [323, 327];
 /// ([`rustysnes_ppu::Ppu::is_short_scanline`]) because every input is its own; this function only
 /// turns that into clocks.
 const fn dot_length(dot: u16, short_line: bool) -> u32 {
-    if !short_line && (dot == LONG_DOTS[0] || dot == LONG_DOTS[1]) {
-        MASTER_PER_DOT + 2
-    } else {
-        MASTER_PER_DOT
-    }
+    rustysnes_ppu::dot_clocks(dot, short_line)
 }
 /// PPU dot at which each visible scanline's HDMA transfer fires — ares' `hdmaPosition` of hcounter
 /// 1104 (`sfc/cpu/timing.cpp`) divided by [`MASTER_PER_DOT`]. Running the table at this exact dot

crates/rustysnes-ppu/src/lib.rs

@@ -118,10 +118,82 @@ const ACTIVE_DOT_START: u16 = 22;
 /// test and `cgram_write_target`'s gate so the two cannot desync.
 const HBLANK_START_DOT: u16 = 274;
 
-/// Dots by which the HV-IRQ horizontal comparator lags the programmed `HTIME`, modelling the
-/// SNES hardware communication delay between the counter unit and the CPU's interrupt logic
-/// (ares `hcounter(10) == (HTIME+1)<<2` ⇒ fire at dot `HTIME + 3.5`; see `check_hv_irq`).
-const HIRQ_TRIGGER_DELAY: u16 = 4;
+/// Master clocks into a scanline at which the HV-IRQ horizontal comparator matches `HTIME`.
+///
+/// ares stores `io.htime = (HTIME + 1) << 2` and tests `hcounter(10) == io.htime`
+/// (`sfc/cpu/io.cpp:181-183`, `sfc/cpu/irq.cpp`), where `hcounter(n)` is the counter `n` clocks
+/// ago — so the match is at clock `4·HTIME + 4 + 10`. **That is a clock, not a dot.** The two
+/// 6-clock dots do not move it; they move which dot contains it, which is the whole reason this
+/// is computed rather than folded into a constant. See [`hirq_trigger_dot`].
+const fn hirq_match_clock(htime: u16) -> u32 {
+    4 * htime as u32 + 14
+}
+
+/// The dot on which an `HTIME` H-IRQ is observed: the first dot boundary at or after
+/// [`hirq_match_clock`].
+///
+/// # This replaces a constant that was only ever right by accident
+///
+/// It used to be `HTIME + 4`, a **dot-domain rounding** of the clock-domain compare above — exact
+/// while every dot is 4 clocks, and silently wrong once dots 323 and 327 became 6. Below the long
+/// dots the two agree *exactly*: `4·HTIME + 14` is never a multiple of 4, so the next boundary is
+/// `HTIME + 4` and not one framebuffer golden moves. They diverge only for `HTIME` in **321..=337**,
+/// where the six-clock dots have displaced every later dot boundary by 2 or 4 clocks and the old
+/// constant fired up to a whole dot late — and, at `HTIME = 336`, suppressed an IRQ that does fire.
+///
+/// # `B4.16` guards this, but less finely than its own doc claims
+///
+/// It records the raw latched dot from an IRQ handler at an `HTIME` below the long dots and one
+/// above, precisely so the before/after is a fact. Measured either side of this change, **both of
+/// its readings are unchanged** — including the `HTIME = 330` one, whose trigger dot moved from 334
+/// to 333. The reason is that the CPU takes an IRQ at an *instruction boundary*, so the
+/// handler-entry dot quantises to the spin loop's instruction length and a one-dot shift is
+/// absorbed. `B4.16` therefore says "nothing regressed", which is what a guard is for, but it
+/// cannot say "the change took effect" — `the_h_irq_dot_is_unchanged_below_the_long_dots_and_moves_
+/// above_them` is what does that, and this note exists so the next person does not read a static
+/// `B4.16` as evidence the model did not move.
+///
+/// Returns [`u16::MAX`] when the match clock falls past the end of the line — on hardware the
+/// counter never reaches it, so the IRQ simply never fires for that `HTIME`.
+const fn hirq_trigger_dot(htime: u16, short_line: bool) -> u16 {
+    let target = hirq_match_clock(htime);
+    let mut dot = 0u16;
+    let mut clock = 0u32;
+    // Walked rather than closed-form: the layout is two irregular dots in a 340-dot line, and a
+    // closed form would have to encode their positions a second time. `DOTS_PER_LINE + 1` covers
+    // the long line's extra dot; the loop is const-evaluable and runs at most 341 steps.
+    while dot <= DOTS_PER_LINE {
+        if clock >= target {
+            return dot;
+        }
+        clock += dot_clocks(dot, short_line);
+        dot += 1;
+    }
+    u16::MAX
+}
+
+/// Master clocks dot `dot` lasts for: 4, except the two that are 6.
+///
+/// The canonical statement of the long-dot layout lives here because the PPU owns the dot model;
+/// `rustysnes-core`'s scheduler consumes it to distribute clocks across a line. `short_line` is
+/// `B2.02`'s scanline, where the decomposition the references give is a flat `340 × 4 = 1360`.
+#[must_use]
+pub const fn dot_clocks(dot: u16, short_line: bool) -> u32 {
+    if !short_line && (dot == LONG_DOTS[0] || dot == LONG_DOTS[1]) {
+        6
+    } else {
+        4
+    }
+}
+
+/// The two dots that last six master clocks instead of four.
+///
+/// fullsnes' *PPU H-Counter-Latch Quantities* histogram settles this by measurement: sampling
+/// `$2137` once per master clock across a line reports dots 323 and 327 latching **six** times
+/// each and dot 340 never. bsnes, ares and Mesen2 all implement exactly this; snes9x uses 322/326
+/// and is the outlier. Both sit deep in hblank, past the visible window and past [`RENDER_DOT`],
+/// so dots `0..=322` keep their clock alignment exactly.
+pub const LONG_DOTS: [u16; 2] = [323, 327];
 
 /// The dot at which a V-only IRQ's comparator is sampled.
 ///
@@ -974,20 +1046,19 @@ impl Ppu {
 
     /// Level-evaluate the HV-IRQ comparator at the current (h, v).
     ///
-    /// The horizontal match is asserted [`HIRQ_TRIGGER_DELAY`] dots *after* the programmed
-    /// `HTIME`, modelling the SNES's hardware communication delay between the H/V counter unit
-    /// and the CPU's interrupt logic. ares encodes this as `hcounter(10) == io.htime` with
-    /// `io.htime` stored as `(HTIME + 1) << 2` clocks (`sfc/cpu/irq.cpp`, `sfc/cpu/io.cpp`), i.e.
-    /// the IRQ fires at hcounter `HTIME*4 + 14` = dot `HTIME + 3.5`. Without this delay an
-    /// IRQ-gated register write (e.g. the `hdmaen_latch_test` `STA $420C`) lands ~3–4 dots early,
-    /// which — combined with the dot-1104 HDMA latch — collapses the test's banded HDMAEN-vs-latch
-    /// crossing into a uniform per-line alternation.
+    /// The horizontal match is derived in the clock domain — [`hirq_match_clock`] gives the clock
+    /// the comparator matches at, [`hirq_trigger_dot`] the dot boundary it is observed on — which
+    /// models the SNES's hardware communication delay between the H/V counter unit and the CPU's
+    /// interrupt logic. Without that delay an IRQ-gated register write (e.g. the
+    /// `hdmaen_latch_test` `STA $420C`) lands ~3–4 dots early, which — combined with the dot-1104
+    /// HDMA latch — collapses the test's banded HDMAEN-vs-latch crossing into a uniform per-line
+    /// alternation.
     const fn check_hv_irq(&mut self) {
         // Adding the delay can push the target past the end of the line. On hardware the H counter
         // never reaches those values (ares' stored `(HTIME+1)<<2 + 10` clocks then exceeds the max
         // hcounter), so the IRQ simply never fires for such HTIME — suppress rather than wrap into
         // the next line, which would be a spurious match hardware/ares never produce.
-        let h_target = self.irq_h + HIRQ_TRIGGER_DELAY;
+        let h_target = hirq_trigger_dot(self.irq_h, self.is_short_scanline());
         let h_match = if self.irq_enable_h {
             // 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
@@ -1629,13 +1700,60 @@ mod tests {
         }
         assert!(fired_at.is_some());
         let (h, v) = fired_at.unwrap();
-        // The H comparator lags HTIME by `HIRQ_TRIGGER_DELAY` dots (hardware counter→IRQ
-        // communication delay; see `check_hv_irq`), and it is evaluated at the start of tick
-        // before H increments, so the IRQ is observed at dot `HTIME + HIRQ_TRIGGER_DELAY` (or the
-        // dot after) on the programmed scanline.
+        // The comparator matches at CLOCK `4*HTIME + 14` (hardware counter→IRQ communication
+        // delay; see `hirq_match_clock`) and the IRQ is observed at the next dot boundary. It is
+        // evaluated at the start of tick before H increments, hence the "or the dot after".
         assert_eq!(v, 50);
-        let target = 100 + HIRQ_TRIGGER_DELAY;
+        let target = hirq_trigger_dot(100, false);
         assert!(h == target || h == target + 1);
+        // HTIME 100 is far below the long dots, so the clock-domain derivation must agree with the
+        // old `HTIME + 4` constant EXACTLY. This is the assertion that says the change moved
+        // nothing it was not supposed to move.
+        assert_eq!(target, 104);
+    }
+
+    /// The clock-domain H-IRQ mapping agrees with the old constant everywhere the old constant was
+    /// right, and only there.
+    ///
+    /// This is the whole safety argument for `T-06-A`'s comparator change written as an assertion.
+    /// `HIRQ_TRIGGER_DELAY = 4` was a dot-domain rounding of `hcounter(10) == (HTIME+1)<<2`, exact
+    /// while every dot is 4 clocks. Below dot 323 it still is — so every existing framebuffer
+    /// golden, every raster test and every H-IRQ a game arms in the visible window must be
+    /// untouched. Above it the six-clock dots have displaced the boundaries and the old constant
+    /// fired late.
+    #[test]
+    fn the_h_irq_dot_is_unchanged_below_the_long_dots_and_moves_above_them() {
+        // Everything that can land before dot 323 keeps the old answer exactly.
+        for htime in 0..=319u16 {
+            assert_eq!(
+                hirq_trigger_dot(htime, false),
+                htime + 4,
+                "HTIME {htime} moved, and nothing below the long dots is allowed to"
+            );
+        }
+        // 320 still coincides: its match clock is 1294, and the next boundary is dot 324's at 1298
+        // — which is also 320 + 4. The first genuinely displaced HTIME is 321.
+        assert_eq!(hirq_trigger_dot(320, false), 324);
+        for htime in 321..=336u16 {
+            assert!(
+                hirq_trigger_dot(htime, false) < htime + 4,
+                "HTIME {htime} is past the long dots and must now fire EARLIER than the old                  constant said, not at the same dot"
+            );
+        }
+        // The old constant suppressed HTIME 336 by pushing it to 340; the clock it matches at is
+        // 1358, which is inside the line, so it fires on the last dot.
+        assert_eq!(hirq_trigger_dot(336, false), 339);
+        // 337 lands on dot 340's boundary, which exists only on the LONG line — so the caller's
+        // `h_target < dots_this_line()` guard suppresses it on an ordinary line and honours it on a
+        // long one. That asymmetry is the point of bounding by the line rather than by a constant.
+        assert_eq!(hirq_trigger_dot(337, false), 340);
+        // Past the end of the line there is genuinely nothing to match, on any line.
+        assert_eq!(hirq_trigger_dot(338, false), u16::MAX);
+
+        // On the short line every dot is 4 clocks again, so the old constant is right throughout.
+        for htime in 0..=335u16 {
+            assert_eq!(hirq_trigger_dot(htime, true), htime + 4);
+        }
     }
 
     #[test]

docs/accuracysnes-plan.md

@@ -1504,6 +1504,16 @@ that is where the line-length non-uniformity the row is trying to see enters the
 Note the shape: `B1.05` cannot be measured because the cart reads dots, and the same limit is why
 `A5.20` is marked `[NOT CART-MEASURABLE]`. These are one obstacle, not two.
 
+**Re-assessed 2026-08-01, after the H-IRQ comparator moved into the clock domain.** The `v1.29.0`
+plan flagged that change as *"the likeliest thing under `B1.05`'s residual"*. It is not, and the
+reason is the one above: the change corrects where the *comparator* fires, which the cart observes
+through an IRQ handler that latches `H` — **in dots**. `B4.16` demonstrates the limit directly:
+its `HTIME = 330` trigger dot moved 334 → 333 and its recorded reading did not move at all, because
+the CPU takes an IRQ at an instruction boundary and the handler-entry dot quantises to the spin
+loop's instruction length. A model change strictly finer than the instrument cannot show up in it.
+`B1.05` stays blocked on the same missing clock-domain instrument, and the H-IRQ item is now closed
+without having moved it.
+
 ### `A6.15` — "all 256 opcodes defined" is a table-extension job, not a test
 
 The 65816 has no illegal opcodes: all 256 encodings are defined, and only `STP` halts. What a core

docs/ppu.md

@@ -262,8 +262,15 @@ The crate is a working dual-chip model. Public API the scheduler/bus call:
   frame, as vblank begins, and **only while forced blank is off** (`end_of_scanline`). Sprite
   evaluation leaves the running counter wherever it finished, so without the reload an address a
   game programmed would not survive a frame. AccuracySNES **C1.06** covers it.
-- **HV-IRQ sampling:** the horizontal comparator fires `HIRQ_TRIGGER_DELAY` (4) dots after the
-  programmed `HTIME`. With H-IRQ **disabled** (V-only), the comparator is sampled at a single dot
+- **HV-IRQ sampling:** the horizontal comparator matches at **clock** `4·HTIME + 14` within the
+  line, and the IRQ is observed at the first dot boundary at or after it (`hirq_match_clock` /
+  `hirq_trigger_dot`). Below the two 6-clock dots that is `HTIME + 4` exactly, which is what the
+  retired `HIRQ_TRIGGER_DELAY` constant hardcoded; from `HTIME = 321` up the six-clock dots have
+  displaced the boundaries and the constant fired up to a whole dot late, and at `HTIME = 336`
+  suppressed an IRQ that does fire. `HTIME = 337` lands on dot 340's boundary, which exists only on
+  the long line — so it is honoured there and suppressed elsewhere, which is why the caller bounds
+  by `dots_this_line()` and not by a constant. With H-IRQ **disabled** (V-only), the comparator is
+  sampled at a single dot
   `VIRQ_TRIGGER_DOT` (2) rather than being treated as matching across the whole line — otherwise
   `V == VTIME` is a level that re-raises the IRQ every dot and `$4211` cannot acknowledge it. See
   `docs/scheduler.md` §H/V-IRQ; AccuracySNES **B4.08**/**B4.12**.
@@ -276,10 +283,11 @@ The crate is a working dual-chip model. Public API the scheduler/bus call:
 - **Timeline:** `tick_dot(&mut self, bus: &mut impl VideoBus)` advances H 0..=340 / V per region
   (262 NTSC / 312 PAL), sets VBlank at V=225 (V=240 overscan) and HBlank, fires
   `notify_scanline`/`notify_vblank`, raises NMI at VBlank start, and level-fires the HV-IRQ
-  comparator (`set_hv_irq(enable_h, enable_v, h, v)` programs it). The horizontal match is
-  asserted `HIRQ_TRIGGER_DELAY` (4) dots **after** the programmed `HTIME`, modelling the SNES
-  counter→CPU interrupt communication delay (ares `hcounter(10) == (HTIME+1)<<2`; see
-  `docs/scheduler.md` §H/V-IRQ). Without it an IRQ-gated register write lands a few dots early.
+  comparator (`set_hv_irq(enable_h, enable_v, h, v)` programs it). The horizontal match is derived
+  in the **clock** domain (ares `hcounter(10) == (HTIME+1)<<2`, i.e. clock `4·HTIME + 14`) and then
+  mapped to a dot, which is what makes it survive the two 6-clock dots; see the HV-IRQ bullet above
+  and `docs/scheduler.md` §H/V-IRQ. Without the delay an IRQ-gated register write lands a few dots
+  early.
 - **Polls (the scheduler reads these — no extra `VideoBus` methods were added):**
   `nmi_pending()`/`ack_nmi()`, `irq_pending()`/`ack_irq()`, `in_vblank()`/`in_hblank()`,
   `dot()`/`scanline()`, `frame_ready()`/`take_frame()`/`frame_count()`, `framebuffer() -> &[u16]`.