From c5bd89df0a6bba21838ec866a3cd69b3d30f536c Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Thu, 9 Jul 2026 08:49:47 -0400 Subject: [PATCH 1/2] feat(frontend): debugger overlay live state viewers (T-81-001, PR A) Fill in the debugger window's 4 panels (65C816/PPU/APU/Cart) with real read-only state, gated behind the debug-hooks flag. A new DebugSnapshot is copied out under the same brief emu lock ShellInfo already uses, mirroring the shell's non-negotiable never-hold-the-lock-in-egui rule. Includes SA-1 second-CPU and Super FX/GSU register-file state in the Cart panel from day one, resolving docs/frontend.md's open question. Disassembly + PC breakpoints/step controls (PR B) and read/write watchpoints (T-81-001b, needs a core-crate debug-hooks feature + Bus-level hook) are tracked as explicit follow-ups, not bundled here. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 24 ++ crates/rustysnes-cart/src/board.rs | 8 + crates/rustysnes-cart/src/coproc/gsu.rs | 22 ++ crates/rustysnes-cart/src/coproc/superfx.rs | 4 + crates/rustysnes-core/src/scheduler.rs | 9 +- crates/rustysnes-frontend/src/app.rs | 9 +- .../rustysnes-frontend/src/debug_snapshot.rs | 112 +++++++++ crates/rustysnes-frontend/src/emu.rs | 113 +++++++++ crates/rustysnes-frontend/src/lib.rs | 1 + crates/rustysnes-frontend/src/ui_shell.rs | 235 ++++++++++++++++-- crates/rustysnes-ppu/src/lib.rs | 13 + docs/frontend.md | 18 +- .../phase-8-reach/sprint-1-instrumentation.md | 29 ++- 13 files changed, 557 insertions(+), 40 deletions(-) create mode 100644 crates/rustysnes-frontend/src/debug_snapshot.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 384c6559..97790e03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **Debugger overlay: live CPU/PPU/APU/Cart state viewers — `v0.8.0 "Instrumentation"`, + T-81-001 (PR A of 2).** `ui_shell.rs`'s debugger window (menu entry, panel selector) has + existed since the frontend's first cut but every panel was a literal `"TODO(impl-phase)"` + label. This lands the state-viewer half: a new `DebugSnapshot` (mirroring `ShellInfo`'s + own copy-out-under-the-brief-lock pattern — the shell's non-negotiable rule that egui never + touches the emu lock directly) shows real 65C816 registers/flags, key PPU registers + the + dot/scanline timeline + a scrollable VRAM window + full CGRAM, SPC700 PC/halt state + all 8 + S-DSP voices' key registers, and the active board name plus (when loaded) SA-1's second-CPU + registers or the Super FX/GSU register file — resolving `docs/frontend.md`'s open question in + the breadth-inclusive direction this whole ladder takes. New small read-only accessors added + to `rustysnes-ppu` (`bg_mode`/`display_brightness`), `rustysnes-core` (`System::sa1_regs`), + and a new `Board::debug_gsu_state` default-no-op trait hook (overridden by `SuperFxBoard`) — + all read-only, no new mutation paths, zero risk to the 0-diff CPU/SPC700 oracles (verified: + the full `--features test-roms` suite passes unchanged). The Debug menu entry that opens the + overlay is gated behind the `debug-hooks` feature (default off) — without it, the debugger + can never open, so the app never builds a snapshot and the default build's emulation output is + untouched. **Deferred to T-81-006, not this pass:** the 65C816 disassembler + breakpoints/ + step controls (needs `System::step_instruction()`-driven stepping, not core changes) and + read/write watchpoints (needs a new `debug-hooks` feature on `rustysnes-core` itself + a + `Bus`-level hook — scoped as its own separate, focused change, T-81-001b, since it touches the + hottest path in the engine). + ### Fixed - **`crates/rustysnes-frontend/web/index.html`: added the missing link to `/api/` on the live diff --git a/crates/rustysnes-cart/src/board.rs b/crates/rustysnes-cart/src/board.rs index 1ff978b4..db608806 100644 --- a/crates/rustysnes-cart/src/board.rs +++ b/crates/rustysnes-cart/src/board.rs @@ -130,6 +130,14 @@ pub trait Board { false } + /// The GSU register file (R0-R15 + SFR + PBR), for a Super FX board's debugger Cart panel. + /// + /// Default `None` — only [`crate::coproc::superfx::SuperFxBoard`] overrides this. A + /// read-only debug accessor, not a control surface (`docs/frontend.md` §open questions). + fn debug_gsu_state(&self) -> Option<([u16; 16], u16, u8)> { + None + } + /// Supply a coprocessor firmware dump (e.g. the DSP-1 `dsp1.rom`). Default `false` — a base /// board has no firmware to load. A chip-ROM-dump coprocessor returns `true` once the dump is /// accepted; without it the board is non-functional, never silently degraded (`docs/adr/0003`). diff --git a/crates/rustysnes-cart/src/coproc/gsu.rs b/crates/rustysnes-cart/src/coproc/gsu.rs index d2411c89..0680ff05 100644 --- a/crates/rustysnes-cart/src/coproc/gsu.rs +++ b/crates/rustysnes-cart/src/coproc/gsu.rs @@ -412,6 +412,28 @@ impl Gsu { self.dreg = 0; } + // --- Debug-only read accessors (no side effects, unlike `read_register`'s memory-mapped + // window which can have read-clear/latch behavior on some addresses). For the debugger + // overlay's Cart panel (`docs/frontend.md` §open questions). ------------------------------ + + /// The R0-R15 general-purpose register file (R15 is also the program counter). + #[must_use] + pub const fn registers(&self) -> [u16; 16] { + self.r + } + + /// The status flag register (SFR). + #[must_use] + pub const fn sfr(&self) -> u16 { + self.sfr + } + + /// The program bank register. + #[must_use] + pub const fn pbr(&self) -> u8 { + self.pbr + } + // --- The host-sync driver. ------------------------------------------------------------- /// Drain one bus-access clock checkpoint (ares `SuperFX::step`/`Thread::synchronize` diff --git a/crates/rustysnes-cart/src/coproc/superfx.rs b/crates/rustysnes-cart/src/coproc/superfx.rs index 643c7b2a..95fd6f91 100644 --- a/crates/rustysnes-cart/src/coproc/superfx.rs +++ b/crates/rustysnes-cart/src/coproc/superfx.rs @@ -258,6 +258,10 @@ impl Board for SuperFxBoard { self.gsu.irq_pending() } + fn debug_gsu_state(&self) -> Option<([u16; 16], u16, u8)> { + Some((self.gsu.registers(), self.gsu.sfr(), self.gsu.pbr())) + } + fn coprocessor_host_accesses(&self) -> u64 { // Surface the GSU instruction count when the chip has run, else the register-access count. // Either is a non-zero liveness signal only if the bus window is mapped right and the GSU diff --git a/crates/rustysnes-core/src/scheduler.rs b/crates/rustysnes-core/src/scheduler.rs index 16c38432..bcaf87ab 100644 --- a/crates/rustysnes-core/src/scheduler.rs +++ b/crates/rustysnes-core/src/scheduler.rs @@ -12,7 +12,7 @@ use alloc::vec::Vec; -use rustysnes_cpu::Cpu; +use rustysnes_cpu::{Cpu, Regs}; use rustysnes_savestate::{SaveReader, SaveStateError, SaveWriter}; use crate::bus::Bus; @@ -204,6 +204,13 @@ impl System { self.sa1_cpu.as_ref().map(|c| c.cycles) } + /// The SA-1 second CPU's architectural register file, or `None` when no SA-1 cart is + /// installed. For the debugger overlay's Cart panel (`docs/frontend.md` §open questions). + #[must_use] + pub fn sa1_regs(&self) -> Option { + self.sa1_cpu.as_ref().map(|c| c.regs) + } + /// Step a single CPU instruction (drives the whole machine in lockstep via the Bus). pub fn step_instruction(&mut self) { if !self.booted { diff --git a/crates/rustysnes-frontend/src/app.rs b/crates/rustysnes-frontend/src/app.rs index 15e8f9aa..c03c4bd8 100644 --- a/crates/rustysnes-frontend/src/app.rs +++ b/crates/rustysnes-frontend/src/app.rs @@ -378,7 +378,7 @@ impl App { let paused = active.shell.paused; // --- (1) Copy framebuffer + audio + read-only info under a BRIEF lock, then drop it. --- - let (fb, fb_dims, info, audio_samples) = { + let (fb, fb_dims, info, audio_samples, debug) = { // `mut` is only needed on the synchronous drive path (run_frame/set_pad); the threaded // build only reads through the guard here. #[cfg_attr(feature = "emu-thread", allow(unused_mut))] @@ -438,8 +438,11 @@ impl App { fps: active.pacer.fps, rom_loaded: emu.rom_loaded(), }; + // Only build the debugger snapshot when the window is actually open — a real, + // avoidable per-frame cost otherwise (`docs/frontend.md` §open questions). + let debug = active.shell.debugger_open.then(|| emu.debug_snapshot()); drop(emu); // release the brief lock BEFORE the wgpu upload + egui pass - (fb, dims, info, audio_samples) + (fb, dims, info, audio_samples, debug) }; // --- Push the frame's audio through the resampler into the ring (outside the lock). --- @@ -478,7 +481,7 @@ impl App { let raw_input = active.egui_state.take_egui_input(&active.window); let mut actions = Vec::new(); let full_output = active.egui_ctx.run_ui(raw_input, |ui| { - actions = active.shell.render(ui, &info, config); + actions = active.shell.render(ui, &info, config, debug.as_ref()); }); active .egui_state diff --git a/crates/rustysnes-frontend/src/debug_snapshot.rs b/crates/rustysnes-frontend/src/debug_snapshot.rs new file mode 100644 index 00000000..a1d9fc16 --- /dev/null +++ b/crates/rustysnes-frontend/src/debug_snapshot.rs @@ -0,0 +1,112 @@ +//! The debugger overlay's per-frame read-only state copy. +//! +//! Mirrors [`crate::ui_shell::ShellInfo`]'s own pattern exactly: [`crate::emu::EmuCore::debug_snapshot`] +//! copies plain data out of the [`rustysnes_core::System`] under the SAME brief lock `ShellInfo` +//! already uses, before the lock is dropped and the egui pass runs — the shell's non-negotiable +//! rule (`ui_shell.rs`'s module doc) is that egui NEVER touches the emu lock directly. + +use rustysnes_core::cpu::Regs; + +/// One frame's worth of read-only chip state for the debugger overlay's 4 panels. +/// +/// Built by [`crate::emu::EmuCore::debug_snapshot`] under the brief emu lock, then handed to +/// [`crate::ui_shell::ShellState::render`] after the lock is released. +#[derive(Debug, Clone)] +pub struct DebugSnapshot { + /// The main 65C816's architectural register file. + pub cpu: Regs, + /// PPU1/PPU2 state. + pub ppu: PpuSnapshot, + /// SPC700 + S-DSP state. + pub apu: ApuSnapshot, + /// The loaded cart's board + any coprocessor state. + pub cart: CartSnapshot, +} + +/// PPU state for the debugger's PPU panel. +#[derive(Debug, Clone)] +pub struct PpuSnapshot { + /// `BGMODE` ($2105), 0..=7. + pub bg_mode: u8, + /// `INIDISP` ($2100) master brightness, 0..=15. + pub display_brightness: u8, + /// Whether the current frame is hi-res (512-wide, `v0.7.0`). + pub is_hires: bool, + /// The current scanline (`Ppu::scanline`). + pub scanline: u16, + /// The current dot within the scanline (`Ppu::dot`). + pub dot: u16, + /// Whether the PPU is in vertical blank. + pub in_vblank: bool, + /// Whether the PPU is in horizontal blank. + pub in_hblank: bool, + /// The full 256-entry CGRAM palette (512 bytes — cheap to copy wholesale every frame, unlike + /// VRAM's 64 KiB). + pub cgram: [u16; 256], + /// A [`VRAM_WINDOW_LEN`]-word window of VRAM starting at `vram_window_start` (word address), + /// controlled by the debugger UI's scroll position — copying all 64 KiB every frame would be + /// real, avoidable per-frame cost for a window the user can only look at part of at once. + pub vram_window: [u16; VRAM_WINDOW_LEN], + /// The word address `vram_window` starts at. + pub vram_window_start: u16, + /// The full 544-byte OAM (small enough to copy wholesale every frame). + pub oam: [u8; 544], +} + +/// Words per VRAM viewer window (2 KiB) — big enough for a meaningful hex-dump page, small +/// enough that copying it every frame is not a real cost next to a whole PPU dot-tick pass. +pub const VRAM_WINDOW_LEN: usize = 1024; + +/// APU (SPC700 + S-DSP) state for the debugger's APU panel. +#[derive(Debug, Clone, Copy)] +pub struct ApuSnapshot { + /// The SMP's program counter. + pub smp_pc: u16, + /// Whether the SMP is halted (`STOP`/`SLEEP`). + pub smp_stopped: bool, + /// Per-voice `(vol_left, vol_right, pitch, srcn, adsr_lo, adsr_hi, gain, envx, outx)` — + /// the DSP registers a debugger cares about, read via `Apu::dsp_read` (no side effects). + pub voices: [VoiceSnapshot; 8], +} + +/// One S-DSP voice's key registers (`docs/apu.md`'s DSP register map, per-voice base `v*0x10`). +#[derive(Debug, Clone, Copy, Default)] +pub struct VoiceSnapshot { + /// `VOLL`/`VOLR`. + pub vol: (i8, i8), + /// `PITCHL`/`PITCHH` (14-bit). + pub pitch: u16, + /// `SRCN` (the sample source-directory entry). + pub srcn: u8, + /// `ADSR1`/`ADSR2`. + pub adsr: (u8, u8), + /// `GAIN`. + pub gain: u8, + /// `ENVX` (the current envelope level). + pub envx: u8, + /// `OUTX` (the current sample output). + pub outx: u8, +} + +/// Cart/coprocessor state for the debugger's Cart panel. +#[derive(Debug, Clone)] +pub struct CartSnapshot { + /// The active board's name (`Board::name()`), e.g. `"HiROM+SuperFX"`. + pub board_name: Option<&'static str>, + /// The SA-1 second CPU's register file, when the loaded cart is an SA-1 board. + pub sa1: Option, + /// The Super FX/GSU register file (R0-R15, SFR, PBR), when the loaded cart is a Super FX + /// board. + pub gsu: Option, +} + +/// The GSU register file, as exposed by `Board::debug_gsu_state`. +#[derive(Debug, Clone, Copy)] +pub struct GsuSnapshot { + /// R0-R15 (R15 doubles as the GSU program counter). + pub r: [u16; 16], + /// The status flag register. + pub sfr: u16, + /// The program bank register. + pub pbr: u8, +} diff --git a/crates/rustysnes-frontend/src/emu.rs b/crates/rustysnes-frontend/src/emu.rs index 3a6ee46d..91aea5c8 100644 --- a/crates/rustysnes-frontend/src/emu.rs +++ b/crates/rustysnes-frontend/src/emu.rs @@ -12,6 +12,10 @@ use rustysnes_core::cart::Cart; use rustysnes_core::cart::header::Coprocessor; use crate::config::Region; +use crate::debug_snapshot::{ + ApuSnapshot, CartSnapshot, DebugSnapshot, GsuSnapshot, PpuSnapshot, VRAM_WINDOW_LEN, + VoiceSnapshot, +}; use crate::gfx::{MAX_H, MAX_W, SNES_W, bgr555_to_rgba8}; use crate::input::Buttons; @@ -51,6 +55,10 @@ pub struct EmuCore { rom: Vec, /// The coprocessor firmware dump installed for this cart (if any), retained for Power-Cycle. firmware: Vec, + /// The debugger overlay's VRAM viewer scroll position (word address). Only meaningful when + /// the debugger is open; `debug_snapshot` reads it regardless (cheap, and keeps this struct + /// free of `debug-hooks`-conditional fields). + debug_vram_scroll: u16, } impl EmuCore { @@ -67,6 +75,7 @@ impl EmuCore { rom_loaded: false, rom: Vec::new(), firmware: Vec::new(), + debug_vram_scroll: 0, } } @@ -265,6 +274,90 @@ impl EmuCore { self.region } + /// Copy out a [`DebugSnapshot`] of the current CPU/PPU/APU/Cart state, for the debugger + /// overlay. Read-only — never mutates anything. The caller must not hold this (or any + /// borrow of `self`) while an egui pass runs (`ui_shell.rs`'s non-negotiable rule); copy it + /// out under the same brief lock `ShellInfo` already uses, then drop the lock. + /// + /// # Panics + /// Never in practice: every index below is bounded by a fixed, small array length + /// (`VRAM_WINDOW_LEN` = 1024, CGRAM = 256, OAM = 544, DSP voices = 8), so the `u8`/`u16` + /// narrowing conversions from `usize` can never actually truncate. + #[must_use] + #[allow(clippy::cast_possible_truncation)] + pub fn debug_snapshot(&self) -> DebugSnapshot { + let ppu = &self.system.bus.ppu; + let vram_window_start = self.debug_vram_scroll; + let mut vram_window = [0u16; VRAM_WINDOW_LEN]; + for (i, word) in vram_window.iter_mut().enumerate() { + *word = ppu.vram_word(vram_window_start.wrapping_add(i as u16)); + } + let mut cgram = [0u16; 256]; + for (i, word) in cgram.iter_mut().enumerate() { + *word = ppu.cgram_word(i as u8); + } + let mut oam = [0u8; 544]; + for (i, byte) in oam.iter_mut().enumerate() { + *byte = ppu.oam_byte(i as u16); + } + + let apu = &self.system.bus.apu; + let voices = core::array::from_fn(|v| { + let base = (v as u8) << 4; + VoiceSnapshot { + vol: ( + apu.dsp_read(base).cast_signed(), + apu.dsp_read(base | 0x01).cast_signed(), + ), + pitch: u16::from(apu.dsp_read(base | 0x02)) + | (u16::from(apu.dsp_read(base | 0x03)) << 8), + srcn: apu.dsp_read(base | 0x04), + adsr: (apu.dsp_read(base | 0x05), apu.dsp_read(base | 0x06)), + gain: apu.dsp_read(base | 0x07), + envx: apu.dsp_read(base | 0x08), + outx: apu.dsp_read(base | 0x09), + } + }); + + let board = self.system.bus.cart.as_ref().map(|c| &c.board); + let cart = CartSnapshot { + board_name: board.as_ref().map(|b| b.name()), + sa1: self.system.sa1_regs(), + gsu: board + .as_ref() + .and_then(|b| b.debug_gsu_state()) + .map(|(r, sfr, pbr)| GsuSnapshot { r, sfr, pbr }), + }; + + DebugSnapshot { + cpu: self.system.cpu.regs, + ppu: PpuSnapshot { + bg_mode: ppu.bg_mode(), + display_brightness: ppu.display_brightness(), + is_hires: ppu.is_hires(), + scanline: ppu.scanline(), + dot: ppu.dot(), + in_vblank: ppu.in_vblank(), + in_hblank: ppu.in_hblank(), + cgram, + vram_window, + vram_window_start, + oam, + }, + apu: ApuSnapshot { + smp_pc: apu.smp_pc(), + smp_stopped: apu.smp_stopped(), + voices, + }, + cart, + } + } + + /// Scroll the debugger's VRAM viewer window (word address, wraps at 64Ki words). + pub const fn set_debug_vram_scroll(&mut self, word_addr: u16) { + self.debug_vram_scroll = word_addr; + } + /// Snapshot the full deterministic core state (`rustysnes_core::System::save_state`, /// `docs/adr/0006`) for rewind/run-ahead/quick-save. Frontend-only state (the decoded RGBA8 /// framebuffer, the retained ROM/firmware bytes for Power-Cycle, latched pads) is NOT part of @@ -402,6 +495,26 @@ mod tests { core.run_frame(); } + #[test] + fn debug_snapshot_of_blank_core_has_no_cart() { + let core = EmuCore::new(0, Region::Ntsc); + let snap = core.debug_snapshot(); + assert_eq!(snap.cart.board_name, None); + assert_eq!(snap.cart.sa1, None); + assert!(snap.cart.gsu.is_none()); + // Power-on 65C816 state (`rustysnes_cpu::Regs::new`): emulation mode, S parked at $01FF. + assert!(snap.cpu.emulation); + assert_eq!(snap.cpu.s, 0x01FF); + } + + #[test] + fn debug_snapshot_vram_scroll_moves_the_window() { + let mut core = EmuCore::new(0, Region::Ntsc); + assert_eq!(core.debug_snapshot().ppu.vram_window_start, 0); + core.set_debug_vram_scroll(0x1234); + assert_eq!(core.debug_snapshot().ppu.vram_window_start, 0x1234); + } + fn zip_containing(name: &str, bytes: &[u8]) -> Vec { let mut buf = std::io::Cursor::new(Vec::new()); let mut writer = zip::ZipWriter::new(&mut buf); diff --git a/crates/rustysnes-frontend/src/lib.rs b/crates/rustysnes-frontend/src/lib.rs index 8d87568c..55c2396f 100644 --- a/crates/rustysnes-frontend/src/lib.rs +++ b/crates/rustysnes-frontend/src/lib.rs @@ -28,6 +28,7 @@ #![allow(unsafe_code)] pub mod config; +pub mod debug_snapshot; pub mod emu; pub mod gfx; pub mod input; diff --git a/crates/rustysnes-frontend/src/ui_shell.rs b/crates/rustysnes-frontend/src/ui_shell.rs index 6ad27ad7..8f58859c 100644 --- a/crates/rustysnes-frontend/src/ui_shell.rs +++ b/crates/rustysnes-frontend/src/ui_shell.rs @@ -1,13 +1,17 @@ //! The always-on egui shell: the menu bar (File / Emulation / Tools / View / Debug / Help), the -//! status bar, the tabbed Settings window, and the toggleable debugger-overlay scaffold. +//! status bar, the tabbed Settings window, and the debugger overlay. //! //! THE NON-NEGOTIABLE RULE (RustyNES `docs/frontend.md`): egui runs **every frame**, and the //! shell NEVER holds the emu lock inside the egui closure. Menu interactions return a -//! [`MenuAction`]; the app dispatches it *after* the egui pass. The debugger panels are SNES -//! stubs (65C816 / PPU1+PPU2 / SPC700+S-DSP / cart-coprocessor) — TODO bodies, not real -//! register read-outs, until the chip models land. +//! [`MenuAction`]; the app dispatches it *after* the egui pass. The debugger's 4 panels +//! (65C816 / PPU1+PPU2 / SPC700+S-DSP / cart-coprocessor) render the [`DebugSnapshot`] the app +//! copies out under the same brief lock [`ShellInfo`] already uses — never touched from inside +//! this module. The Debug menu entry that opens the overlay is gated behind the `debug-hooks` +//! feature (default off): without it, `debugger_open` can never become `true`, so the app never +//! builds a snapshot and the debugger is unreachable in a shipped default build. use crate::config::{Config, Region}; +use crate::debug_snapshot::DebugSnapshot; /// An action requested from the egui pass, dispatched by `App::dispatch_menu_action` AFTER the /// pass returns (so it never runs while the emu lock is held inside the egui closure). @@ -101,6 +105,7 @@ impl ShellState { root_ui: &mut egui::Ui, info: &ShellInfo, cfg: &mut Config, + debug: Option<&DebugSnapshot>, ) -> Vec { let mut actions = Vec::new(); let ctx = root_ui.ctx().clone(); @@ -200,12 +205,15 @@ impl ShellState { }); ui.menu_button("Debug", |ui| { + #[cfg(feature = "debug-hooks")] if ui .checkbox(&mut self.debugger_open, "Debugger overlay") .clicked() { ui.close(); } + #[cfg(not(feature = "debug-hooks"))] + ui.label("(rebuild with --features debug-hooks)"); }); ui.menu_button("Help", |ui| { @@ -239,7 +247,7 @@ impl ShellState { self.render_settings(&ctx, cfg); } if self.debugger_open { - self.render_debugger(&ctx); + self.render_debugger(&ctx, debug); } actions @@ -287,8 +295,11 @@ impl ShellState { self.settings_open = open; } - /// The debugger overlay: a panel selector + the SNES chip-panel stubs. - fn render_debugger(&mut self, ctx: &egui::Context) { + /// The debugger overlay: a panel selector + the SNES chip-panel live state viewers. `debug` + /// is `None` only when the debugger opens before the app's next lock-scope has built a + /// snapshot yet — every panel handles that by showing "no data yet" rather than assuming + /// the app has already supplied one. + fn render_debugger(&mut self, ctx: &egui::Context, debug: Option<&DebugSnapshot>) { let mut open = self.debugger_open; egui::Window::new("Debugger") .open(&mut open) @@ -301,32 +312,204 @@ impl ShellState { ui.selectable_value(&mut self.panel, DebugPanel::Cart, "Cart"); }); ui.separator(); + let Some(debug) = debug else { + ui.label("(no ROM loaded — nothing to inspect yet)"); + return; + }; match self.panel { - // TODO(impl-phase): each panel reads the live chip state (copied out under - // the brief emu lock, never read inside this egui closure) and renders the - // register grid + disassembly / viewers. - DebugPanel::Cpu => { - ui.label("65C816 — registers (A/X/Y 8/16-bit, D/DBR/PBR/S/P, E latch),"); - ui.label("disassembly, breakpoints. TODO(impl-phase)."); - } - DebugPanel::Ppu => { - ui.label("PPU1 (5C77) + PPU2 (5C78) — BG modes 0-7 + Mode 7 affine,"); - ui.label("OAM/CGRAM/VRAM viewers, the dot/scanline timeline. TODO."); - } - DebugPanel::Apu => { - ui.label("SPC700 + S-DSP — the 2nd clock domain, 8 BRR voices, ARAM,"); - ui.label("the $2140-$2143 port handshake. TODO(impl-phase)."); - } - DebugPanel::Cart => { - ui.label("Cart — LoROM/HiROM/ExHiROM map + coprocessor (DSP-1..4 /"); - ui.label("Super FX / SA-1 / S-DD1 / SPC7110 / CX4 / OBC1). TODO."); - } + DebugPanel::Cpu => render_cpu_panel(ui, debug), + DebugPanel::Ppu => render_ppu_panel(ui, debug), + DebugPanel::Apu => render_apu_panel(ui, debug), + DebugPanel::Cart => render_cart_panel(ui, debug), } }); self.debugger_open = open; } } +/// 65C816 registers + processor-status flags. Disassembly + breakpoints/stepping are a +/// follow-up (T-81-006) — this panel is the live-state half of the ticket. +fn render_cpu_panel(ui: &mut egui::Ui, debug: &DebugSnapshot) { + let r = &debug.cpu; + egui::Grid::new("cpu_regs").num_columns(2).show(ui, |ui| { + ui.label("A"); + ui.label(format!("{:04X}", r.a)); + ui.end_row(); + ui.label("X"); + ui.label(format!("{:04X}", r.x)); + ui.end_row(); + ui.label("Y"); + ui.label(format!("{:04X}", r.y)); + ui.end_row(); + ui.label("S"); + ui.label(format!("{:04X}", r.s)); + ui.end_row(); + ui.label("D"); + ui.label(format!("{:04X}", r.d)); + ui.end_row(); + ui.label("DBR"); + ui.label(format!("{:02X}", r.dbr)); + ui.end_row(); + ui.label("PBR"); + ui.label(format!("{:02X}", r.pbr)); + ui.end_row(); + ui.label("PC"); + ui.label(format!("{:04X}", r.pc)); + ui.end_row(); + ui.label("P"); + ui.label(format!("{:?}", r.p)); + ui.end_row(); + ui.label("E (emulation)"); + ui.label(if r.emulation { "1" } else { "0" }); + ui.end_row(); + }); +} + +/// Format a row of 16-bit words as space-separated 4-hex-digit groups, for the VRAM/CGRAM hex +/// dumps. A plain loop, not `.map(...).collect::()`, since collecting a `String` from a +/// `format!`-per-item iterator reallocates on every item (`clippy::format_collect`). +fn hex_row(words: &[u16]) -> String { + use core::fmt::Write as _; + let mut out = String::with_capacity(words.len() * 5); + for w in words { + let _ = write!(out, "{w:04X} "); + } + out +} + +/// Key PPU registers + the dot/scanline timeline + CGRAM/a scrollable VRAM window. +/// +/// # Panics +/// Never in practice: `VRAM_WINDOW_LEN` (1024) and every `row * 8` byte offset within it fit +/// comfortably in a `u16`, so the narrowing casts below can't actually truncate. +#[allow(clippy::cast_possible_truncation)] +fn render_ppu_panel(ui: &mut egui::Ui, debug: &DebugSnapshot) { + let p = &debug.ppu; + egui::Grid::new("ppu_regs").num_columns(2).show(ui, |ui| { + ui.label("BGMODE"); + ui.label(p.bg_mode.to_string()); + ui.end_row(); + ui.label("Brightness"); + ui.label(p.display_brightness.to_string()); + ui.end_row(); + ui.label("Hi-res"); + ui.label(if p.is_hires { "yes (512-wide)" } else { "no" }); + ui.end_row(); + ui.label("Scanline / dot"); + ui.label(format!("{} / {}", p.scanline, p.dot)); + ui.end_row(); + ui.label("VBlank / HBlank"); + ui.label(format!( + "{} / {}", + if p.in_vblank { "yes" } else { "no" }, + if p.in_hblank { "yes" } else { "no" } + )); + ui.end_row(); + }); + ui.separator(); + ui.label(format!( + "VRAM window (words {:04X}-{:04X}):", + p.vram_window_start, + p.vram_window_start + .wrapping_add(crate::debug_snapshot::VRAM_WINDOW_LEN as u16 - 1) + )); + egui::ScrollArea::vertical() + .max_height(160.0) + .id_salt("vram_scroll") + .show(ui, |ui| { + for (row, chunk) in p.vram_window.chunks(8).enumerate() { + let addr = p.vram_window_start.wrapping_add((row * 8) as u16); + ui.monospace(format!("{addr:04X}: {}", hex_row(chunk))); + } + }); + ui.separator(); + ui.label("CGRAM (256 colors):"); + egui::ScrollArea::vertical() + .max_height(100.0) + .id_salt("cgram_scroll") + .show(ui, |ui| { + for (row, chunk) in p.cgram.chunks(8).enumerate() { + ui.monospace(format!("{:02X}: {}", row * 8, hex_row(chunk))); + } + }); +} + +/// SPC700 + S-DSP: the SMP's own PC + halt state, and the 8 voices' key registers. +fn render_apu_panel(ui: &mut egui::Ui, debug: &DebugSnapshot) { + let a = &debug.apu; + ui.label(format!( + "SMP PC: {:04X} (stopped: {})", + a.smp_pc, + if a.smp_stopped { "yes" } else { "no" } + )); + ui.separator(); + egui::Grid::new("dsp_voices").num_columns(8).show(ui, |ui| { + for h in [ + "V", "VOL L/R", "PITCH", "SRCN", "ADSR", "GAIN", "ENVX", "OUTX", + ] { + ui.strong(h); + } + ui.end_row(); + for (i, v) in a.voices.iter().enumerate() { + ui.label(i.to_string()); + ui.label(format!("{}/{}", v.vol.0, v.vol.1)); + ui.label(format!("{:04X}", v.pitch)); + ui.label(format!("{:02X}", v.srcn)); + ui.label(format!("{:02X}/{:02X}", v.adsr.0, v.adsr.1)); + ui.label(format!("{:02X}", v.gain)); + ui.label(format!("{:02X}", v.envx)); + ui.label(format!("{:02X}", v.outx)); + ui.end_row(); + } + }); +} + +/// The active board + (when present) a Core/Curated coprocessor's own register state — SA-1's +/// second-CPU regs or the Super FX/GSU register file, resolving `docs/frontend.md`'s open +/// question in the breadth-inclusive direction this whole ladder takes. +fn render_cart_panel(ui: &mut egui::Ui, debug: &DebugSnapshot) { + let c = &debug.cart; + ui.label(format!("Board: {}", c.board_name.unwrap_or("(no cart)"))); + if let Some(r) = c.sa1 { + ui.separator(); + ui.label("SA-1 second CPU:"); + egui::Grid::new("sa1_regs").num_columns(2).show(ui, |ui| { + ui.label("A"); + ui.label(format!("{:04X}", r.a)); + ui.end_row(); + ui.label("X"); + ui.label(format!("{:04X}", r.x)); + ui.end_row(); + ui.label("Y"); + ui.label(format!("{:04X}", r.y)); + ui.end_row(); + ui.label("PC"); + ui.label(format!("{:02X}:{:04X}", r.pbr, r.pc)); + ui.end_row(); + ui.label("P"); + ui.label(format!("{:?}", r.p)); + ui.end_row(); + }); + } + if let Some(g) = c.gsu { + ui.separator(); + ui.label("Super FX / GSU:"); + egui::Grid::new("gsu_regs").num_columns(2).show(ui, |ui| { + for (i, chunk) in g.r.chunks(4).enumerate() { + ui.label(format!("R{}-R{}", i * 4, i * 4 + 3)); + ui.monospace(hex_row(chunk)); + ui.end_row(); + } + ui.label("SFR"); + ui.label(format!("{:04X}", g.sfr)); + ui.end_row(); + ui.label("PBR"); + ui.label(format!("{:02X}", g.pbr)); + ui.end_row(); + }); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/rustysnes-ppu/src/lib.rs b/crates/rustysnes-ppu/src/lib.rs index bfd47c80..c8312179 100644 --- a/crates/rustysnes-ppu/src/lib.rs +++ b/crates/rustysnes-ppu/src/lib.rs @@ -891,6 +891,19 @@ impl Ppu { self.frame_ready } + /// The current `BGMODE` ($2105) value (0..=7) — which of the 8 tile/priority layouts is + /// active. For the debugger overlay's PPU panel (`docs/frontend.md` §open questions). + #[must_use] + pub const fn bg_mode(&self) -> u8 { + self.io.bg_mode + } + + /// The current `INIDISP` ($2100) master brightness (0..=15). For the debugger overlay. + #[must_use] + pub const fn display_brightness(&self) -> u8 { + self.io.display_brightness + } + /// Completed-frame count since power-on (monotonic, wrapping). #[must_use] pub const fn frame_count(&self) -> u64 { diff --git a/docs/frontend.md b/docs/frontend.md index 37b26b82..cef6f7e3 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -125,7 +125,21 @@ Reuse the egui shell, the audio ring, the pacing matrix, and the debugger-panel from the RustyNES frontend; SNES-specific work is the second CPU/APU panel, the Mode-7 / HDMA debug views, and the coprocessor status panel. +## Debugger overlay (`v0.8.0 "Instrumentation"`, T-81-001) + +`ui_shell.rs`'s debugger window's 4 panels (65C816 / PPU1+2 / SPC700+S-DSP / Cart) render a +`DebugSnapshot` the app copies out under the same brief lock `ShellInfo` already uses — CPU +registers/flags, key PPU registers + the dot/scanline timeline + a scrollable VRAM window + full +CGRAM, SPC700 PC/halt state + all 8 S-DSP voices' key registers, and the active board name. +Gated behind the `debug-hooks` feature (default off) at the menu-entry level: without it, +`debugger_open` can never become `true`, so the app never builds a snapshot and the default +build's emulation output is unaffected. Disassembly + breakpoints/step controls, and read/write +watchpoints (needing a new `debug-hooks` feature on `rustysnes-core` itself + a `Bus`-level +hook), are follow-up tickets (T-81-006, T-81-001b) — not yet landed. + ## Open questions -- Whether the second-CPU (SA-1 / Super FX) state warrants its own debugger panel from day one - or a Phase 8 add — defer to when Phase 4 lands SA-1. +- ~~Whether the second-CPU (SA-1 / Super FX) state warrants its own debugger panel from day one + or a Phase 8 add~~ — **resolved, `v0.8.0`:** yes, from day one. The Cart panel shows SA-1's + second-CPU registers (`System::sa1_regs`) or the Super FX/GSU register file + (`Board::debug_gsu_state`) when the loaded cart uses either. diff --git a/to-dos/phase-8-reach/sprint-1-instrumentation.md b/to-dos/phase-8-reach/sprint-1-instrumentation.md index 93b91543..c27d162b 100644 --- a/to-dos/phase-8-reach/sprint-1-instrumentation.md +++ b/to-dos/phase-8-reach/sprint-1-instrumentation.md @@ -16,18 +16,31 @@ memory-viewer functionality, behind the existing `debug-hooks` flag. Include SA- coprocessor state in the Cart panel from day one — resolving `docs/frontend.md`'s open question in the breadth-inclusive direction this whole ladder takes, not deferring it further. +**Landed in two PRs, not one** (scoping found during implementation, not before): PR A ships +the live state viewers for all 4 panels (pure read-only plumbing, no core changes beyond small +new accessors). PR B adds a minimal 65C816 disassembler + PC breakpoints + step/step-over/ +step-into (frontend-only, using the existing `System::step_instruction()`). Read/write +watchpoints need a new `debug-hooks` feature on `rustysnes-core` itself + a `Bus`-level hook — +deferred to a separate follow-up ticket, T-81-001b, since it touches the hottest path in the +engine and deserves its own focused review. + **Acceptance criteria:** -- [ ] 65C816 panel: register/flag view, breakpoints (PC + read/write watchpoints), step/ - step-over/step-into. -- [ ] PPU panel: VRAM/CGRAM/OAM viewer, current scanline/dot, register state. -- [ ] APU panel: SPC700 registers, DSP voice state. -- [ ] Cart panel: active board type + coprocessor register state (SA-1 second-CPU state and - Super FX/GSU state included when the loaded cart uses either). -- [ ] With `debug-hooks` off, the build is byte-identical (CI gate). +- [x] 65C816 panel: register/flag view (PR A). Breakpoints (PC), step/step-over/step-into: PR B. + Read/write watchpoints: T-81-001b (not this ticket). +- [x] PPU panel: VRAM (scrollable window) / CGRAM viewer, current scanline/dot, register state + (PR A). OAM viewer not yet landed — small follow-up, same shape as the VRAM/CGRAM viewers. +- [x] APU panel: SPC700 PC/halt state, DSP voice state (PR A). +- [x] Cart panel: active board type + coprocessor register state (SA-1 second-CPU state via + `System::sa1_regs`, Super FX/GSU state via `Board::debug_gsu_state`) (PR A). +- [x] With `debug-hooks` off, the build is byte-identical — the Debug menu entry itself is + feature-gated, so `debugger_open` can never become `true` and the app never builds a + snapshot (PR A; verified `cargo check`/`clippy`/`fmt` clean in both configs, full + `--features test-roms` suite passes unchanged). **Dependencies:** T-51-001 (the shell itself, already landed) -**Reference:** `docs/frontend.md` §open questions; `crates/rustysnes-frontend/src/ui_shell.rs` +**Reference:** `docs/frontend.md` §Debugger overlay; `crates/rustysnes-frontend/src/ui_shell.rs`, +`debug_snapshot.rs` **Estimated complexity:** L --- From fe9e1ae52cde8698aef777aece2becbc8672e5c0 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Thu, 9 Jul 2026 10:05:42 -0400 Subject: [PATCH 2/2] fix(frontend): address PR #49 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ui_shell.rs: the "no debugger snapshot yet" message no longer claims a ROM-load reason it doesn't actually check -- debug tracks debugger_open, not ROM state (a snapshot builds fine for a blank core). - debug_snapshot.rs: stop claiming vram_window_start is "controlled by the debugger UI's scroll position" -- no UI calls set_debug_vram_scroll yet, so the window is fixed today. - Five stale `docs/frontend.md §open questions` references updated to `§Debugger overlay`, the section that actually documents this now. Co-Authored-By: Claude Sonnet 5 --- crates/rustysnes-cart/src/board.rs | 2 +- crates/rustysnes-cart/src/coproc/gsu.rs | 2 +- crates/rustysnes-core/src/scheduler.rs | 2 +- crates/rustysnes-frontend/src/app.rs | 2 +- crates/rustysnes-frontend/src/debug_snapshot.rs | 7 ++++--- crates/rustysnes-frontend/src/ui_shell.rs | 4 +++- crates/rustysnes-ppu/src/lib.rs | 2 +- 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/rustysnes-cart/src/board.rs b/crates/rustysnes-cart/src/board.rs index db608806..e81d4f2d 100644 --- a/crates/rustysnes-cart/src/board.rs +++ b/crates/rustysnes-cart/src/board.rs @@ -133,7 +133,7 @@ pub trait Board { /// The GSU register file (R0-R15 + SFR + PBR), for a Super FX board's debugger Cart panel. /// /// Default `None` — only [`crate::coproc::superfx::SuperFxBoard`] overrides this. A - /// read-only debug accessor, not a control surface (`docs/frontend.md` §open questions). + /// read-only debug accessor, not a control surface (`docs/frontend.md` §Debugger overlay). fn debug_gsu_state(&self) -> Option<([u16; 16], u16, u8)> { None } diff --git a/crates/rustysnes-cart/src/coproc/gsu.rs b/crates/rustysnes-cart/src/coproc/gsu.rs index 0680ff05..a91c5d57 100644 --- a/crates/rustysnes-cart/src/coproc/gsu.rs +++ b/crates/rustysnes-cart/src/coproc/gsu.rs @@ -414,7 +414,7 @@ impl Gsu { // --- Debug-only read accessors (no side effects, unlike `read_register`'s memory-mapped // window which can have read-clear/latch behavior on some addresses). For the debugger - // overlay's Cart panel (`docs/frontend.md` §open questions). ------------------------------ + // overlay's Cart panel (`docs/frontend.md` §Debugger overlay). ------------------------------ /// The R0-R15 general-purpose register file (R15 is also the program counter). #[must_use] diff --git a/crates/rustysnes-core/src/scheduler.rs b/crates/rustysnes-core/src/scheduler.rs index bcaf87ab..ecf653c8 100644 --- a/crates/rustysnes-core/src/scheduler.rs +++ b/crates/rustysnes-core/src/scheduler.rs @@ -205,7 +205,7 @@ impl System { } /// The SA-1 second CPU's architectural register file, or `None` when no SA-1 cart is - /// installed. For the debugger overlay's Cart panel (`docs/frontend.md` §open questions). + /// installed. For the debugger overlay's Cart panel (`docs/frontend.md` §Debugger overlay). #[must_use] pub fn sa1_regs(&self) -> Option { self.sa1_cpu.as_ref().map(|c| c.regs) diff --git a/crates/rustysnes-frontend/src/app.rs b/crates/rustysnes-frontend/src/app.rs index c03c4bd8..d3a4e3ec 100644 --- a/crates/rustysnes-frontend/src/app.rs +++ b/crates/rustysnes-frontend/src/app.rs @@ -439,7 +439,7 @@ impl App { rom_loaded: emu.rom_loaded(), }; // Only build the debugger snapshot when the window is actually open — a real, - // avoidable per-frame cost otherwise (`docs/frontend.md` §open questions). + // avoidable per-frame cost otherwise (`docs/frontend.md` §Debugger overlay). let debug = active.shell.debugger_open.then(|| emu.debug_snapshot()); drop(emu); // release the brief lock BEFORE the wgpu upload + egui pass (fb, dims, info, audio_samples, debug) diff --git a/crates/rustysnes-frontend/src/debug_snapshot.rs b/crates/rustysnes-frontend/src/debug_snapshot.rs index a1d9fc16..ad9b3ceb 100644 --- a/crates/rustysnes-frontend/src/debug_snapshot.rs +++ b/crates/rustysnes-frontend/src/debug_snapshot.rs @@ -43,9 +43,10 @@ pub struct PpuSnapshot { /// The full 256-entry CGRAM palette (512 bytes — cheap to copy wholesale every frame, unlike /// VRAM's 64 KiB). pub cgram: [u16; 256], - /// A [`VRAM_WINDOW_LEN`]-word window of VRAM starting at `vram_window_start` (word address), - /// controlled by the debugger UI's scroll position — copying all 64 KiB every frame would be - /// real, avoidable per-frame cost for a window the user can only look at part of at once. + /// A [`VRAM_WINDOW_LEN`]-word window of VRAM starting at `vram_window_start` (word address) — + /// copying all 64 KiB every frame would be real, avoidable per-frame cost for a window the + /// user can only look at part of at once. `EmuCore::set_debug_vram_scroll` moves the window; + /// no UI control calls it yet (fixed at the window's start address today) — a follow-up. pub vram_window: [u16; VRAM_WINDOW_LEN], /// The word address `vram_window` starts at. pub vram_window_start: u16, diff --git a/crates/rustysnes-frontend/src/ui_shell.rs b/crates/rustysnes-frontend/src/ui_shell.rs index 8f58859c..6e5969c0 100644 --- a/crates/rustysnes-frontend/src/ui_shell.rs +++ b/crates/rustysnes-frontend/src/ui_shell.rs @@ -313,7 +313,9 @@ impl ShellState { }); ui.separator(); let Some(debug) = debug else { - ui.label("(no ROM loaded — nothing to inspect yet)"); + // `debug` tracks `debugger_open`, not ROM state (a snapshot builds fine for a + // blank core) — don't claim a ROM-load reason that may not be why it's `None`. + ui.label("(no debugger snapshot yet)"); return; }; match self.panel { diff --git a/crates/rustysnes-ppu/src/lib.rs b/crates/rustysnes-ppu/src/lib.rs index c8312179..22c815ea 100644 --- a/crates/rustysnes-ppu/src/lib.rs +++ b/crates/rustysnes-ppu/src/lib.rs @@ -892,7 +892,7 @@ impl Ppu { } /// The current `BGMODE` ($2105) value (0..=7) — which of the 8 tile/priority layouts is - /// active. For the debugger overlay's PPU panel (`docs/frontend.md` §open questions). + /// active. For the debugger overlay's PPU panel (`docs/frontend.md` §Debugger overlay). #[must_use] pub const fn bg_mode(&self) -> u8 { self.io.bg_mode