pass nmi timing accurarcy test; fix several annoying bugs

8 files changed, 130 insertions, 74 deletions

diff --git a/Cargo.lock b/Cargo.lock
index ce67c02..0adf88f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -134,7 +134,7 @@ dependencies = [
 
 [[package]]
 name = "runes"
-version = "0.1.4"
+version = "0.1.5"
 dependencies = [
  "clap 2.29.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "sdl2 0.31.0 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/Cargo.toml b/Cargo.toml
index 4c79f83..42bd0e2 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "runes"
-version = "0.1.4"
+version = "0.1.5"
 authors = ["Determinant <tederminant@gmail.com>"]
 description = "No-std NES emulator library and minimal emulator written purely in Rust."
 repository = "https://github.com/Determinant/runes"
diff --git a/src/apu.rs b/src/apu.rs
index 14bde24..1a8a4fa 100644
--- a/src/apu.rs
+++ b/src/apu.rs
@@ -1,24 +1,33 @@
 #![allow(dead_code)]
-use mos6502::{CPU_FREQ, CPU};
+use mos6502::CPU_FREQ;
 use memory::CPUBus;
 
 struct LPFilter {
     prev_out: i16
 }
 
-const AUDIO_LEVEL_MAX: i32 = 65536;
+const AUDIO_LEVEL_MAX: i32 = 32768;
 const LP_FACTOR: i32 = (0.815686 * AUDIO_LEVEL_MAX as f32) as i32;
 const HP_FACTOR1: i32 = (0.996039 * AUDIO_LEVEL_MAX as f32) as i32;
 const HP_FACTOR2: i32 = (0.999835 * AUDIO_LEVEL_MAX as f32) as i32;
 
+fn cutoff(mut x: i32) -> i16 {
+    if x < -32768 {
+        x = -32768
+    } else if x > 32767 {
+        x = 32767
+    }
+    x as i16
+}
+
 impl LPFilter {
     fn new() -> Self {
         LPFilter { prev_out: 0 }
     }
 
     fn output(&mut self, input: i16) -> i16 {
-        let out = ((input as i32 - self.prev_out as i32)
-                        * LP_FACTOR / AUDIO_LEVEL_MAX) as i16;
+        let out = cutoff((input as i32 - self.prev_out as i32)
+                        * LP_FACTOR / AUDIO_LEVEL_MAX);
         self.prev_out = out;
         out
     }
@@ -40,8 +49,9 @@ impl HPFilter {
     }
 
     fn output(&mut self, input: i16) -> i16 {
-        let out = (self.prev_out as i32 * self.hp_factor / AUDIO_LEVEL_MAX +
-                    input as i32 - self.prev_in as i32) as i16;
+        let out = cutoff(
+            self.prev_out as i32 * self.hp_factor / AUDIO_LEVEL_MAX +
+            input as i32 - self.prev_in as i32);
         self.prev_in = input;
         self.prev_out = out;
         out
@@ -603,10 +613,10 @@ impl DMC {
         self.rem_len = self.sample_len;
     }
 
-    fn try_refill(&mut self, cpu: &mut CPU) {
+    fn try_refill(&mut self, bus: &CPUBus) {
         if self.rem_len > 0 && self.dmc_cnt == 0 {
-            cpu.cycle += 4;
-            self.shift_reg = cpu.mem.read_without_tick(self.cur_addr);
+            bus.cpu_stall(4);
+            self.shift_reg = bus.get_cpu().mem.read_without_tick(self.cur_addr);
             self.dmc_cnt = 8;
             self.cur_addr = self.cur_addr.wrapping_add(1);
             if self.cur_addr == 0x0 {
@@ -617,7 +627,7 @@ impl DMC {
                 if self.dmc_loop {
                     self.restart()
                 } else if self.irq_enabled {
-                    cpu.trigger_irq()
+                    bus.get_cpu().trigger_irq()
                 }
             }
         }
@@ -638,9 +648,9 @@ impl DMC {
         self.dmc_cnt -= 1;
     }
 
-    fn tick_timer(&mut self, cpu: &mut CPU) {
+    fn tick_timer(&mut self, bus: &CPUBus) {
         if !self.enabled { return }
-        self.try_refill(cpu);
+        self.try_refill(bus);
         if self.timer_lvl == 0 {
             self.timer_lvl = self.timer_period;
             self.shift();
@@ -716,7 +726,7 @@ impl<'a> APU<'a> {
             let sample = self.output();
             self.spkr.queue(sample);
         }
-        self.tick_timer(bus.get_cpu());
+        self.tick_timer(bus);
         self.cycle_even = !self.cycle_even;
         irq
     }
@@ -727,6 +737,7 @@ impl<'a> APU<'a> {
         let tnd_out = TND_TABLE[(self.triangle.output() * 3 +
                                 self.noise.output() * 2 +
                                 self.dmc.output()) as usize];
+        //(pulse_out + tnd_out).wrapping_sub(0x8000) as i16
         self.lp_filter.output(
             self.hp_filter2.output(
                 self.hp_filter1.output(
@@ -778,12 +789,12 @@ impl<'a> APU<'a> {
         }
     }
 
-    fn tick_timer(&mut self, cpu: &mut CPU) {
+    fn tick_timer(&mut self, bus: &CPUBus) {
         if self.cycle_even {
             self.pulse1.tick_timer();
             self.pulse2.tick_timer();
             self.noise.tick_timer();
-            self.dmc.tick_timer(cpu);
+            self.dmc.tick_timer(bus);
         }
         self.triangle.tick_timer();
     }
diff --git a/src/bin.rs b/src/bin.rs
index 88c17d9..332126d 100644
--- a/src/bin.rs
+++ b/src/bin.rs
@@ -269,8 +269,8 @@ impl<'a> sdl2::audio::AudioCallback for SDLAudioPlayback<'a> {
         let mut m = self.0.buffer.lock().unwrap();
         {
             let b = &mut m.0;
-            let l1 = (b.tail + b.buffer.len() - b.head) % b.buffer.len();
-            //print!("{} ", l1);
+            /*let l1 = (b.tail + b.buffer.len() - b.head) % b.buffer.len();
+            print!("{} ", l1); */
             
             for x in out.iter_mut() {
                 *x = b.deque()
@@ -282,7 +282,7 @@ impl<'a> sdl2::audio::AudioCallback for SDLAudioPlayback<'a> {
             self.0.time_barrier.notify_one();
         } else {
             m.1 = 0;
-            //println!("audio frame skipping");
+            println!("audio frame skipping");
         }
     }
 }
@@ -326,12 +326,13 @@ fn print_cpu_trace(cpu: &CPU) {
     }
     println!("0x{:04x} {} a:{:02x} x:{:02x} y:{:02x} s: {:02x} sp: {:02x}",
              pc, disasm::parse(opcode as u8, &code[1..]),
-             cpu.get_a(), cpu.get_x(), cpu.get_y(), cpu.get_status(), cpu.get_sp());
+             cpu.get_a(), cpu.get_x(), cpu.get_y(),
+             cpu.get_status(), cpu.get_sp());
 }
 
 fn main() {
     let matches = App::new("RuNES")
-                    .version("0.1.4")
+                    .version("0.1.5")
                     .author("Ted Yin <tederminant@gmail.com>")
                     .about("A Rust NES emulator")
                     .arg(Arg::with_name("scale")
@@ -349,10 +350,13 @@ fn main() {
                          .required(true)
                          .index(1))
                     .get_matches();
+
     let scale = std::cmp::min(8,
                     std::cmp::max(1,
-                        value_t!(matches, "scale", u32).unwrap_or(4)));
+                        value_t!(matches, "scale", u32).unwrap_or(3)));
     let full = matches.is_present("full");
+
+    /* load and parse iNES file */
     let fname = matches.value_of("INPUT").unwrap();
     let mut file = File::open(fname).unwrap();
     let mut rheader = [0; 16];
@@ -383,6 +387,7 @@ fn main() {
     if chr_len == 0 {
         chr_len = 0x2000;
     }
+
     let mut prg_rom = vec![0; prg_len];
     let mut chr_rom = vec![0; chr_len];
     let sram = vec![0; 0x4000];
@@ -404,8 +409,9 @@ fn main() {
     let device = audio_subsystem.open_playback(None, &desired_spec, |_| {
         SDLAudioPlayback(&audio_sync)
     }).unwrap();
-
+    /* P1 controller */
     let p1ctl = stdctl::Joystick::new();
+    /* cartridge & mapper */
     let cart = SimpleCart::new(chr_rom, prg_rom, sram, mirror);
     let mut win = Box::new(SDLWindow::new(&sdl_context, &p1ctl, scale, full));
     let mut m: Box<mapper::Mapper> = match mapper_id {
@@ -416,7 +422,7 @@ fn main() {
     };
 
     let mapper = RefCell::new(&mut (*m) as &mut mapper::Mapper);
-    let mut cpu = CPU::new(CPUMemory::new(&mapper, Some(&p1ctl), None)/*, &mut f*/);
+    let mut cpu = CPU::new(CPUMemory::new(&mapper, Some(&p1ctl), None));
     let mut ppu = PPU::new(PPUMemory::new(&mapper), &mut (*win));
     let mut apu = APU::new(&mut spkr);
     let cpu_ptr = &mut cpu as *mut CPU;
@@ -426,7 +432,7 @@ fn main() {
     loop {
         /* consume the leftover cycles from the last instruction */
         while cpu.cycle > 0 {
-            cpu.mem.bus.tick();
+            cpu.mem.bus.tick()
         }
         //print_cpu_trace(&cpu);
         cpu.step();
diff --git a/src/lib.rs b/src/lib.rs
index ade3a2d..6439124 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,5 +1,5 @@
-#![no_std]
-//extern crate core;
+//#![no_std]
+extern crate core;
 mod memory;
 #[macro_use] mod mos6502;
 mod ppu;
diff --git a/src/memory.rs b/src/memory.rs
index 8954c91..985070f 100644
--- a/src/memory.rs
+++ b/src/memory.rs
@@ -5,7 +5,7 @@ use mos6502::{CPU, CPU_FREQ};
 use cartridge::MirrorType;
 use mapper::Mapper;
 use controller::Controller;
-use core::cell::RefCell;
+use core::cell::{RefCell, Cell};
 use core::ptr::null_mut;
 
 pub trait VMem {
@@ -18,6 +18,8 @@ pub struct CPUBus<'a> {
     ppu: *mut PPU<'a>,
     apu: *mut APU<'a>,
     ppu_sampler: RefCell<Sampler>,
+    nmi_after_tick: Cell<bool>,
+    cpu_stall: Cell<u32>
 }
 
 impl<'a> CPUBus<'a> {
@@ -26,6 +28,8 @@ impl<'a> CPUBus<'a> {
                 cpu: null_mut(),
                 apu: null_mut(),
                 ppu_sampler: RefCell::new(Sampler::new(CPU_FREQ, 60)),
+                nmi_after_tick: Cell::new(false),
+                cpu_stall: Cell::new(0)
             }
     }
 
@@ -41,17 +45,41 @@ impl<'a> CPUBus<'a> {
     #[inline(always)] pub fn get_ppu(&self) -> &'a mut PPU<'a> {unsafe{&mut *self.ppu}}
     #[inline(always)] pub fn get_apu(&self) -> &'a mut APU<'a> {unsafe{&mut *self.apu}}
 
+    pub fn cpu_stall(&self, delta: u32) {
+        self.cpu_stall.set(self.cpu_stall.get() + delta)
+    }
+
     pub fn tick(&self) {
         let cpu = self.get_cpu();
         let ppu = self.get_ppu();
         let apu = self.get_apu();
-        cpu.tick();
+
+        let cpu_stall = self.cpu_stall.get();
+        if cpu_stall == 0 {
+            cpu.tick()
+        } else {
+            self.cpu_stall.set(cpu_stall - 1)
+        }
         if apu.tick(self) {
             cpu.trigger_irq()
         }
-        if ppu.tick(self) || ppu.tick(self) || ppu.tick(self) {
-            cpu.trigger_nmi()
+
+        let first = ppu.tick(self);
+        let second = ppu.tick(self);
+        let third = ppu.tick(self);
+        let mut nmi_after_tick = false;
+
+        if first || second || third {
+            nmi_after_tick = !first;
+            if cpu.cycle == 0 && nmi_after_tick {
+                cpu.trigger_delayed_nmi()
+            } else {
+                cpu.trigger_nmi()
+            }
+            //println!("nmi");
         }
+        self.nmi_after_tick.set(nmi_after_tick);
+        //println!("tick {} {}", ppu.scanline, ppu.cycle);
         if let (true, _) = self.ppu_sampler.borrow_mut().tick() {
             ppu.scr.frame()
         }
@@ -88,7 +116,12 @@ impl<'a> CPUMemory<'a> {
             self.sram[(addr & 0x07ff) as usize]
         } else if addr < 0x4000 {
             match addr & 0x7 {
-                0x2 => ppu.read_status(cpu),
+                0x2 => {
+                    if ppu.cycle == 2 || ppu.cycle == 3 {
+                        cpu.suppress_nmi()
+                    } /* race condition when status is read near vbl/nmi */
+                    ppu.read_status()
+                },
                 0x4 => ppu.read_oamdata(),
                 0x7 => ppu.read_data(),
                 _ => 0
@@ -119,7 +152,10 @@ impl<'a> CPUMemory<'a> {
                 0x0 => {
                     let old = ppu.get_flag_nmi();
                     ppu.write_ctl(data);
-                    if !old && ppu.try_nmi() && ppu.vblank {
+                    if !ppu.try_nmi() && self.bus.nmi_after_tick.get() {
+                        cpu.suppress_nmi()
+                    } /* NMI could be suppressed if disabled near set */
+                    if !old && ppu.try_nmi() && ppu.vblank_lines {
                         cpu.trigger_delayed_nmi()
                     } /* toggle NMI flag can generate multiple ints */
                 },
@@ -155,7 +191,7 @@ impl<'a> CPUMemory<'a> {
                 0x4013 => apu.dmc.write_reg4(data),
                 0x4015 => apu.write_status(data),
                 0x4017 => apu.write_frame_counter(data),
-                0x4014 => ppu.write_oamdma(data, cpu),
+                0x4014 => ppu.write_oamdma(data, &self.bus),
                 0x4016 => {
                     if let Some(c) = self.ctl1 { c.write(data) }
                     if let Some(c) = self.ctl2 { c.write(data) }
diff --git a/src/mos6502.rs b/src/mos6502.rs
index 5f64b59..848dd1f 100644
--- a/src/mos6502.rs
+++ b/src/mos6502.rs
@@ -605,7 +605,6 @@ pub struct CPU<'a> {
     //pub elapsed: u32,
     int: Option<IntType>,
     pub mem: CPUMemory<'a>,
-    //sec_callback: &'a mut FnMut(),
 }
 
 macro_rules! make_int {
@@ -639,8 +638,7 @@ impl<'a> CPU<'a> {
     #[inline(always)] pub fn get_over(&self) -> u8 { (self.status >> 6) & 1 }
     #[inline(always)] pub fn get_neg(&self) -> u8 { (self.status >> 7) & 1 }
 
-    pub fn new(mem: CPUMemory<'a>/*,
-               sec_callback: &'a mut FnMut()*/) -> Self {
+    pub fn new(mem: CPUMemory<'a>) -> Self {
         let pc = 0;
         /* nes power up state */
         let a = 0;
@@ -650,12 +648,14 @@ impl<'a> CPU<'a> {
         let status = 0x34;
         let cycle = 0;
 
-        CPU{a, x, y,
+        CPU {a, x, y,
             pc, sp, status, cycle,
             opr: 0, ea: 0, imm_val: 0,
             int: None,
             acc: false,
-            mem, /*elapsed: 0, sec_callback*/}
+            mem,
+            //elapsed: 0
+        }
     }
 
     pub fn powerup(&mut self) {
@@ -667,7 +667,6 @@ impl<'a> CPU<'a> {
     make_int!(irq, IRQ_VECTOR);
 
     pub fn step(&mut self) {
-        //let cycle0 = self.cycle;
         if self.int.is_some() {
             match self.int {
                 Some(IntType::NMI) => {self.nmi(); self.int = None; return},
@@ -676,7 +675,7 @@ impl<'a> CPU<'a> {
                 _ => ()
             }
         }
-        self.cycle += 1;
+        self.cycle += 0xff;
         let pc = self.pc;
         let opcode = self.mem.read(pc) as usize;
         /* update opr pointing to operands of current inst */
@@ -685,7 +684,8 @@ impl<'a> CPU<'a> {
         self.pc = pc.wrapping_add(INST_LENGTH[opcode] as u16);
         /* get effective address based on addressing mode */
         self.acc = false;
-        self.cycle += INST_CYCLE[opcode] as u32 - 1;
+        self.cycle += INST_CYCLE[opcode] as u32;
+        self.cycle -= 0xff;
         self.cycle += (addr::ADDR_MODES[opcode](self) * INST_EXTRA_CYCLE[opcode]) as u32;
         /* execute the inst */
         ops::OPS[opcode](self);
@@ -694,13 +694,7 @@ impl<'a> CPU<'a> {
 
     pub fn tick(&mut self) {
         self.cycle -= 1;
-        /*
-        self.elapsed += 1;
-        if self.elapsed == CPU_FREQ {
-            self.elapsed = 0;
-            (self.sec_callback)();
-        }
-        */
+        //self.elapsed += 1;
     }
 
     pub fn reset(&mut self) {
diff --git a/src/ppu.rs b/src/ppu.rs
index 4ff9324..50303b6 100644
--- a/src/ppu.rs
+++ b/src/ppu.rs
@@ -1,6 +1,5 @@
 #![allow(dead_code)]
 use memory::{VMem, PPUMemory, CPUBus};
-use mos6502::CPU;
 use core::intrinsics::transmute;
 
 pub trait Screen {
@@ -48,12 +47,14 @@ pub struct PPU<'a> {
     sp_pixel: [u32; 8],
     sp_idx: [usize; 8],
     sp_cnt: [u8; 8],
-    pub vblank: bool,
+    vblank: bool,
+    pub vblank_lines: bool,
     buffered_read: u8,
     early_read: bool,
     /* IO */
     mem: PPUMemory<'a>,
     pub scr: &'a mut Screen,
+    //pub elapsed: u32,
 }
 
 impl<'a> PPU<'a> {
@@ -71,16 +72,12 @@ impl<'a> PPU<'a> {
     }
 
     #[inline]
-    pub fn read_status(&mut self, cpu: &mut CPU) -> u8 {
+    pub fn read_status(&mut self) -> u8 {
         let res = (self.ppustatus & !0x1fu8) | (self.reg & 0x1f);
         self.ppustatus &= !PPU::FLAG_VBLANK;
         self.w = false;
-        if self.scanline == 241 {
-            match self.cycle {
-                1 => self.early_read = true, /* read before cycle 1 */
-                2 | 3 => cpu.suppress_nmi(),
-                _ => ()
-            }
+        if self.scanline == 241 && self.cycle == 1 {
+            self.early_read = true /* read before cycle 1 */
         }
         res
     }
@@ -166,19 +163,20 @@ impl<'a> PPU<'a> {
     }
 
     #[inline]
-    pub fn write_oamdma(&mut self, data: u8, cpu: &mut CPU) {
+    pub fn write_oamdma(&mut self, data: u8, bus: &CPUBus) {
+        let cpu = bus.get_cpu();
         self.reg = data;
         let mut addr = (data as u16) << 8;
-        let cycle = 1 + (cpu.cycle & 1) + 256;
-        cpu.cycle += cycle;
+        let stall = 1 + (cpu.cycle & 1) + 512;
+        bus.cpu_stall(stall);
         let mut oamaddr = self.oamaddr;
-        for _ in 0..cycle - 0x100 {
+        for _ in 0..stall - 0x100 {
             cpu.mem.bus.tick()
         }
         {
             let oam_raw = self.get_oam_raw_mut();
             for _ in 0..0x100 {
-                oam_raw[oamaddr as usize] = cpu.mem.read(addr);
+                oam_raw[oamaddr as usize] = cpu.mem.read_without_tick(addr);
                 addr = addr.wrapping_add(1);
                 oamaddr = oamaddr.wrapping_add(1);
             }
@@ -457,8 +455,8 @@ impl<'a> PPU<'a> {
         let ppustatus = 0xa0;
         let oamaddr = 0x00;
         let buffered_read = 0x00;
-        let cycle = 340;
-        let scanline = 240;
+        let cycle = 0;
+        let scanline = 241;
         PPU {
             scanline,
             ppuctl,
@@ -476,9 +474,11 @@ impl<'a> PPU<'a> {
             sp_pixel: [0; 8],
             sp_cnt: [0; 8],
             vblank: false,
+            vblank_lines: true,
             buffered_read,
             early_read: false,
-            mem, scr
+            mem, scr,
+            //elapsed: 0,
         }
     }
 
@@ -488,8 +488,9 @@ impl<'a> PPU<'a> {
         self.ppustatus = self.ppustatus & 0x80;
         self.w = false;
         self.buffered_read = 0x00;
-        self.cycle = 340;
-        self.scanline = 240;
+        self.cycle = 0;
+        self.scanline = 241;
+        self.vblank_lines = true;
     }
 
     #[inline(always)]
@@ -504,6 +505,12 @@ impl<'a> PPU<'a> {
     }
 
     fn _tick(&mut self) -> bool {
+        if self.scanline == 240 {
+            self.vblank_lines = true
+        } else if self.scanline == 261 {
+            self.vblank_lines = false
+        }
+        //self.elapsed += 1;
         let cycle = self.cycle;
         if cycle == 0 {
             self.cycle = 1;
@@ -551,11 +558,11 @@ impl<'a> PPU<'a> {
                     self.cycle = 258;
                     return false
                 }
-                if pre_line && cycle == 339 && self.f {
-                        self.scanline = 0;
-                        self.cycle = 0;
-                        self.f = !self.f;
-                        return false;
+                /* skip at 338 because of 10-even_odd_timing test indicates an undocumented
+                 * behavior of NES */
+                if pre_line && cycle == 338 && self.f {
+                    self.cycle = 340;
+                    return false;
                 }
             }
         } else {
@@ -564,6 +571,8 @@ impl<'a> PPU<'a> {
                 if !self.early_read {
                     self.ppustatus |= PPU::FLAG_VBLANK
                 }
+                //self.elapsed = 0;
+                //println!("vbl");
                 self.early_read = false;
                 self.vblank = true;
                 self.scr.render();