Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **Mid-scanline/HDMA-driven register timing: regression-baseline test landed — `v0.6.0`
"Shippable" work, pulled forward.** `crates/rustysnes-core/tests/mid_scanline_hdma_baseline.rs`
is a minimal, self-authored hand-assembled 65C816 reproduction (HDMA drives `$2100` master
brightness; no BG/OBJ setup needed since a disabled-layers screen renders pure backdrop color,
isolating the exact compositor-vs-HDMA dot-timing bug) that locks in the confirmed-buggy
transition position (last-white row 99, first-black row 100 — exactly matching the
off-by-one-line shift the mechanism analysis predicts) as a numeric acceptance test. This
closes most of the "no dedicated test ROM" gap `docs/ppu.md` flagged as blocking a fix; the
cross-crate scheduler/PPU timing-communication design remains the real outstanding work. No
production code changed; full workspace + `--features test-roms` suites verified unaffected
(zero regressions).

- **Mid-scanline/HDMA-driven register timing + hi-res color-math precision: researched — `v0.5.0`
"Fidelity" work.** Confirmed a genuine, previously-undocumented off-by-one-line compositor bug
against ares' per-pixel reference model (`ppu/main.cpp`'s active-pixel rendering runs strictly
Expand Down
179 changes: 179 additions & 0 deletions crates/rustysnes-core/tests/mid_scanline_hdma_baseline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
//! Regression baseline for the confirmed off-by-one-line HDMA/compositor timing bug
//! (`docs/ppu.md` §"Mid-scanline/HDMA-driven register timing — researched, confirmed, deferred
//! (v0.5.0)"). This test does **not** assert correct hardware behavior — it locks in RustySNES's
//! current (confirmed-buggy) output for a minimal, self-authored reproduction, so any *unrelated*
//! regression to this exact scenario is still caught, and so the eventual fix has a concrete,
//! numeric target to flip.
//!
//! The reproduction drives `$2100` (`INIDISP`, master brightness) with HDMA mode 0 (1 byte, 1
//! register), alternating between full brightness (`$0F`, backdrop renders white) and force-off
//! brightness (`$00`, backdrop renders black) at a single transition partway through the frame —
//! chosen instead of a scroll register because it needs no BG/tilemap setup at all: with every
//! background layer left disabled, every pixel falls through to the backdrop color
//! (`Ppu::layer_color`'s `!p.opaque` path returns `cgram[0]`), so this isolates the exact
//! compositor-vs-HDMA dot-timing bug from any background-rendering code at all.

use rustysnes_core::System;
use rustysnes_core::cart::Cart;
use rustysnes_core::ppu::SCREEN_WIDTH;

/// Number of visible scanlines the first ("A", full brightness) HDMA table phase covers.
const PHASE_A_LINES: usize = 100;
/// Non-overscan visible height (`Ppu::visible_height()` with `overscan == false`).
const VISIBLE_LINES: usize = 224;

/// Build a minimal LoROM ROM whose reset-vector program:
/// 1. Sets CGRAM entry 0 (the backdrop color) to white (`$7FFF`) once.
/// 2. Sets `$2100` (`INIDISP`) to full brightness (`$0F`) once, force-blank off.
/// 3. Programs HDMA channel 0 for mode 0 (1 byte -> `$2100`), source = the table below.
/// 4. Enables HDMAEN channel 0, then spins forever.
///
/// No background layer is ever enabled (`$212C`/`$212D` stay at their power-on `0`), so the
/// entire visible frame renders as a flat backdrop-color field whose per-line brightness is
/// driven purely by the HDMA table — the simplest possible probe for the compositor's
/// end-of-line register-read timing relative to that same line's own HDMA run.
///
/// The HDMA table uses `count = 1`, non-repeat entries for every single line (never the `bit7`
/// "continuous" repeat mode) so this reproduction doesn't depend on this crate's own repeat-mode
/// data-pointer semantics being exercised correctly — only the well-exercised one-line-at-a-time
/// path already proven by the committed `undisbeliever` HDMA goldens.
fn mid_scanline_hdma_probe_rom() -> Vec<u8> {
let mut rom = vec![0u8; 0x1_0000];

#[rustfmt::skip]
let program: [u8; 52] = [
0x78, // SEI
0xA9, 0x00, // LDA #$00
0x8D, 0x21, 0x21, // STA $2121 (CGADD = 0)
0xA9, 0xFF, // LDA #$FF
0x8D, 0x22, 0x21, // STA $2122 (CGDATA low: backdrop = white low byte)
0xA9, 0x7F, // LDA #$7F
0x8D, 0x22, 0x21, // STA $2122 (CGDATA high: backdrop = white high byte)
0xA9, 0x0F, // LDA #$0F
0x8D, 0x00, 0x21, // STA $2100 (INIDISP: force-blank off, brightness 15)
0xA9, 0x00, // LDA #$00
0x8D, 0x00, 0x43, // STA $4300 (DMAP0: A->B, direct, mode 0 -- 1 byte/reg)
0x8D, 0x01, 0x43, // STA $4301 (BBAD0 = $00 -> target $2100)
0xA9, 0x00, // LDA #$00
0x8D, 0x02, 0x43, // STA $4302 (A1T0L: table addr low)
0xA9, 0x90, // LDA #$90
0x8D, 0x03, 0x43, // STA $4303 (A1T0H: table addr high -> $9000)
0xA9, 0x00, // LDA #$00
0x8D, 0x04, 0x43, // STA $4304 (A1B0: table bank 0)
0xA9, 0x01, // LDA #$01
0x8D, 0x0C, 0x42, // STA $420C (HDMAEN: enable channel 0)
0x4C, 0x31, 0x80, // loop: JMP $8031 (self -- spin forever; offset 49 = $8031)
];
rom[..program.len()].copy_from_slice(&program);

// HDMA table at ROM offset 0x1000 == CPU address $9000 (A1T0H:A1T0L above).
// Every entry is [count=$01 (non-repeat, 1 line), data byte]. VISIBLE_LINES entries total,
// then a $00 terminator. Phase A (full brightness) for the first PHASE_A_LINES scanlines,
// phase B (force-off) for the remainder.
let table_offset = 0x1000;
let mut w = table_offset;
for line in 0..VISIBLE_LINES {
let data = if line < PHASE_A_LINES { 0x0F } else { 0x00 };
rom[w] = 0x01;
rom[w + 1] = data;
w += 2;
}
rom[w] = 0x00; // terminator

let h = 0x7FC0;
rom[h..h + 21].copy_from_slice(b"MIDLINE HDMA PROBE ");
rom[h + 0x15] = 0x20; // LoROM, slow
rom[h + 0x16] = 0x00; // no coprocessor, no RAM, no battery
rom[h + 0x18] = 0x00; // RAM size 0
rom[h + 0x19] = 0x01; // North America / NTSC
let checksum: u16 = 0x1234;
let complement = !checksum;
rom[h + 0x1C..h + 0x1E].copy_from_slice(&complement.to_le_bytes());
rom[h + 0x1E..h + 0x20].copy_from_slice(&checksum.to_le_bytes());
rom[h + 0x3C..h + 0x3E].copy_from_slice(&0x8000u16.to_le_bytes()); // reset vector
rom
}

fn booted_system() -> System {
let mut sys = System::new(0);
sys.bus.cart = Some(Cart::from_rom(&mid_scanline_hdma_probe_rom()).expect("probe ROM header"));
sys.reset();
sys
}

/// Scans the composited framebuffer's backdrop column (x=0, uniform across every row since no
/// BG/OBJ layer is enabled) and returns `(last_white_row, first_black_row)`.
fn find_transition(framebuffer: &[u16]) -> (usize, usize) {
let mut last_white = None;
let mut first_black = None;
for row in 0..VISIBLE_LINES {
let px = framebuffer[row * SCREEN_WIDTH];
if px == 0x7FFF {
last_white = Some(row);
} else if px == 0x0000 && first_black.is_none() {
first_black = Some(row);
}
}
(
last_white.expect("at least one white row expected"),
first_black.expect("at least one black row expected"),
)
}

/// Locks in the CURRENT (confirmed-buggy) transition position: RustySNES's end-of-line
/// compositor reads `$2100` at dot 340, AFTER that same line's own HDMA run at dot 276 has
/// already written the *next* phase's value — so the transition lands one scanline EARLIER than
/// real hardware. `docs/ppu.md`'s analysis derives the two candidate positions precisely:
///
/// - **Real hardware** (not what this asserts): last-white row 100 (`V=101`), first-black row
/// 101 (`V=102`) — line `V`'s own HDMA write only takes effect starting `V+1`.
/// - **RustySNES today** (what this asserts): last-white row 99 (`V=100`), first-black row 100
/// (`V=101`) — the write meant for `V+1` visibly lands on `V` itself.
///
/// When the real fix lands (`docs/ppu.md`'s "What a future investigation/fix needs"), this
/// specific assertion is EXPECTED to change to `(100, 101)` — that flip, confirmed deliberately
/// and reviewed (a Golden-Vector update, not an accidental diff), is the fix's acceptance test.
#[test]
fn mid_scanline_hdma_transition_is_one_line_early_current_known_bug() {
let mut sys = booted_system();
sys.run_frame();

let (last_white, first_black) = find_transition(sys.bus.framebuffer());

assert_eq!(
(last_white, first_black),
(99, 100),
"current (buggy) transition position changed -- if this is an intentional fix for the \
off-by-one-line bug (docs/ppu.md §Mid-scanline/HDMA-driven register timing) landing at \
exactly (100, 101), update this assertion deliberately and cross-check against the full \
`--features test-roms` golden suite before committing; any other value is a real, \
unrelated regression"
);

// Sanity: exactly one transition (no HDMA-table or register-write bugs of this test's own
// making producing a noisier, ambiguous pattern) -- every row up to and including
// `last_white` is white, and every row after it is black.
let framebuffer = sys.bus.framebuffer();
for row in 0..VISIBLE_LINES {
let px = framebuffer[row * SCREEN_WIDTH];
let expected = if row <= last_white { 0x7FFF } else { 0x0000 };
assert_eq!(
px, expected,
"row {row} was {px:#06x}, expected {expected:#06x} -- the probe ROM's HDMA table or \
CGRAM setup is not behaving as this test assumes"
);
}
Comment thread
doublegate marked this conversation as resolved.
}

/// Determinism sanity: re-running the exact same probe from a fresh `System` produces the exact
/// same composited framebuffer (this is a pure function of the deterministic core,
/// `docs/adr/0004` — no wall-clock, no OS RNG anywhere on this path). Compares the full
/// framebuffer rather than just the derived transition point for a stronger guarantee.
#[test]
fn mid_scanline_hdma_probe_is_deterministic_across_fresh_runs() {
let mut a = booted_system();
a.run_frame();
let mut b = booted_system();
b.run_frame();
assert_eq!(a.bus.framebuffer(), b.bus.framebuffer());
}
25 changes: 25 additions & 0 deletions docs/ppu.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,31 @@ verification infrastructure to prove it's actually correct.
of register state for any ROM that already uses HDMA-driven per-line effects, and each such
shift needs confirming as "now correct" rather than blindly re-baselined.

### Regression-baseline groundwork (landed)

Item 1 above is now partially in place:
`crates/rustysnes-core/tests/mid_scanline_hdma_baseline.rs` is a minimal, self-authored
hand-assembled 65C816 program (mirroring the pattern already used for rewind/run-ahead's
synthetic per-frame signal tests) that drives `$2100` (`INIDISP`, master brightness) via HDMA
mode 0, alternating full-brightness (backdrop renders white) and force-off (backdrop renders
black) at a single transition partway through the frame. No BG/OBJ layer is ever enabled, so
every pixel falls through to the backdrop color (`Ppu::layer_color`'s `!p.opaque` path) — this
isolates the exact compositor-vs-HDMA dot-timing bug with no tilemap/tileset setup at all, at the
cost of not being the specific "Air Strike Patrol BG3 scroll" scenario (a scroll-register variant
remains open future work if a title-accurate reproduction is ever wanted).

The test **locks in the current (confirmed-buggy) transition position** — `(last-white row 99,
first-black row 100)`, i.e. `V=100`/`V=101` — derived and empirically confirmed exactly as this
section's mechanism analysis predicts (`row = V - 1`; the write meant for `V+1` lands on `V`
itself). It does **not** assert correct hardware behavior; a second test confirms the reproduction
is deterministic across fresh runs. When the real fix (item 2 above) lands, this specific
assertion is expected to flip to `(100, 101)` — a deliberate, reviewed Golden-Vector update
(cross-checked against the full `--features test-roms` suite per item 3), not an accidental diff.
This gives the eventual fix a concrete, already-passing-on-the-buggy-baseline acceptance test to
flip, closing most of the "no dedicated committed test ROM" gap this section originally flagged
as blocking — item 2 (the cross-crate scheduler/PPU timing-communication design) remains the real
outstanding work.

## Hi-res (Modes 5/6) color-math precision — researched, deferred (v0.5.0)

**Status: researched; blocked entirely on 512-px hi-res output not existing yet (a real feature
Expand Down
10 changes: 8 additions & 2 deletions to-dos/VERSION-PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,14 @@ AccuracyCoin-equivalent). See `to-dos/phase-6-accuracy-to-100/`.
state at dot 340, after line `V`'s own HDMA already ran) applies it to line `V` itself. This
un-defers (confirms as real, not yet fixes) Phase 2's flagged "mid-line raster deferred" gap.
Not fixed this pass — the fix touches the hottest code path in the engine (every frame, all 29
goldens) with no dedicated test ROM yet to verify against; full mechanism, why it's deferred,
and what a fix needs in `docs/ppu.md` §Mid-scanline/HDMA-driven register timing. **Researched,
goldens); **regression-baseline groundwork now landed** —
`crates/rustysnes-core/tests/mid_scanline_hdma_baseline.rs` is a minimal, self-authored
hand-assembled reproduction (HDMA-driven `$2100` brightness, no BG/OBJ setup needed) that locks
in the current confirmed-buggy transition position as a numeric acceptance test the eventual
fix flips deliberately — closing most of the "no dedicated test ROM" gap; the cross-crate
scheduler/PPU timing-communication design remains the real outstanding work. Full mechanism,
the baseline test, and what the fix still needs in `docs/ppu.md` §Mid-scanline/HDMA-driven
register timing. **Researched,
deferred (blocked on a larger feature, not a precision nuance):** hi-res color-math precision
(Bishoujo Janshi Suchie-Pai / Marvelous+SA-1) — confirmed against ares' `DAC::run()` that hi-res
is a dual-half-pixel output trick (alternating `above`/`below` compositor results at 2× the
Expand Down