Skip to content

Conversation

Copy link

Copilot AI commented Jul 10, 2025

Problem

The MMC3 mapper implementation had incorrect IRQ timing that could cause issues with games relying on precise interrupt timing. The original implementation used a simplified scanline-based approach that triggered IRQs once per scanline at cycle 260, but this doesn't match the actual MMC3 hardware behavior.

Root Cause

The MMC3 mapper's IRQ counter should be decremented on A12 address line toggles during CHR ROM reads, not on a fixed scanline schedule. The A12 line (bit 12 of the PPU address bus) toggles when the PPU switches between different pattern table regions during background tile fetching.

Solution

This PR implements proper A12 toggle detection for accurate MMC3 IRQ timing:

Key Changes

  1. Added A12 State Tracking: Added last_a12 field to the PPU to track the A12 address line state
  2. Modified CHR Reading: Updated read_chr() to detect A12 rising edge transitions (0→1) and trigger the mapper's IRQ logic
  3. Removed Scanline-Based IRQ: Eliminated the old fixed-timing IRQ mechanism that called step_scanline() at cycle 260
  4. Rendering Period Filtering: A12 toggle detection only occurs during active rendering periods (scanlines 0-239 and 261)

Before vs After

// Before: Fixed timing, once per scanline
if self.regs.rendering_enabled() && self.cycle == 260 && self.scanline < 240 {
    self.rom.mapper.step_scanline();
}

// After: A12 toggle detection during CHR reads
fn read_chr(&mut self, addr: u16) -> u8 {
    if self.regs.rendering_enabled() && (self.scanline < 240 || self.scanline == 261) {
        let new_a12 = (addr & 0x1000) != 0;
        if new_a12 && !self.last_a12 {
            self.rom.mapper.step_scanline(); // Trigger on A12 rising edge
        }
        self.last_a12 = new_a12;
    }
    self.rom.mapper.read(&mut self.rom.cart, addr)
}

Testing

Added comprehensive tests covering:

  • MMC3 initialization and state
  • IRQ assertion behavior
  • PRG RAM access functionality
  • Bank register operations
  • IRQ register management

Impact

This fix should resolve timing issues with MMC3 games that rely on precise interrupt timing for effects like:

  • Smooth scrolling
  • Split-screen effects
  • Status bar rendering
  • Sprite multiplexing

The changes are minimal and maintain backward compatibility while significantly improving accuracy to match real MMC3 hardware behavior.

Verification

  • ✅ All existing tests pass
  • ✅ New MMC3-specific tests pass
  • ✅ Debug and release builds work
  • ✅ Web build compatibility maintained
  • ✅ No breaking changes to public API

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits July 10, 2025 21:05
Co-authored-by: nathsou <4953827+nathsou@users.noreply.github.com>
Co-authored-by: nathsou <4953827+nathsou@users.noreply.github.com>
Copilot AI changed the title [WIP] Find a fix issues with the MMC3 mapper implementation Fix MMC3 mapper IRQ timing with proper A12 toggle detection Jul 10, 2025
Copilot AI requested a review from nathsou July 10, 2025 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants