fix(apu): clock frame sequencer at correct Hz

src/apu.rs

@@ -5,7 +5,7 @@ use types::ch1::{Sweep, SweepDirection};
 use types::ch3::Volume as Ch3Volume;
 use types::ch4::{CounterWidth, Frequency as Ch4Frequency, PolynomialCounter};
 use types::common::{EnvelopeDirection, FrequencyHigh, SoundDuty, VolumeEnvelope};
-use types::{ChannelControl, FrameSequencerState, SoundOutput};
+use types::{ChannelControl, SoundOutput};
 
 pub mod gen;
 mod types;
@@ -26,8 +26,7 @@ pub struct Apu {
     ch4: Channel4,
 
     // Frame Sequencer
-    frame_seq_state: FrameSequencerState,
+    div_prev: Option<u16>,
-    div_prev: Option<u8>,
 
     prod: Option<SampleProducer<f32>>,
     sample_counter: u64,
@@ -95,41 +94,30 @@ impl BusIo for Apu {
 
 impl Apu {
     pub(crate) fn tick(&mut self, div: u16) {
-        use FrameSequencerState::*;
         self.sample_counter += SAMPLE_INCREMENT;
 
-        // the 5th bit of the high byte
+        // Length Control (256Hz)
-        let bit_5 = (div >> 13 & 0x01) as u8;
+        if self.falling_edge(13, div) {
-
+            self.handle_length();
-        if let Some(0x01) = self.div_prev {
-            if bit_5 == 0x00 {
-                // Falling Edge, step the Frame Sequencer
-                self.frame_seq_state.step();
-
-                match self.frame_seq_state {
-                    Step0Length => self.handle_length(),
-                    Step2LengthAndSweep => {
-                        self.handle_length();
-                        self.handle_sweep();
-                    }
-                    Step4Length => self.handle_length(),
-                    Step6LengthAndSweep => {
-                        self.handle_length();
-                        self.handle_sweep();
-                    }
-                    Step7VolumeEnvelope => self.handle_volume(),
-                    Step1Nothing | Step3Nothing | Step5Nothing => {}
-                };
-            }
         }
 
+        // Sweep (128Hz)
+        if self.falling_edge(14, div) {
+            self.handle_sweep();
+        }
+
+        // Volume Envelope (64Hz)
+        if self.falling_edge(15, div) {
+            self.handle_volume();
+        }
+
+        self.div_prev = Some(div);
+
         self.ch1.clock();
         self.ch2.clock();
         self.ch3.clock();
         self.ch4.clock();
 
-        self.div_prev = Some(bit_5);
-
         if self.sample_counter >= SM83_CLOCK_SPEED {
             self.sample_counter %= SM83_CLOCK_SPEED;
 
@@ -176,7 +164,8 @@ impl Apu {
 
         if self.ctrl.enabled {
             // Frame Sequencer reset to Step 0
-            self.frame_seq_state = Default::default();
+            // TODO: With the current implementation of the frame sequencer,
+            // what does this even mean?
 
             // Square Duty units are reset to first step
             self.ch1.duty_pos = 0;
@@ -338,6 +327,13 @@ impl Apu {
             &mut self.ch4.current_volume,
         );
     }
+
+    fn falling_edge(&self, bit: u8, div: u16) -> bool {
+        match self.div_prev {
+            Some(p) => (p >> bit & 0x01) == 0x01 && (div >> bit & 0x01) == 0x00,
+            None => false,
+        }
+    }
 }
 
 #[derive(Debug, Default)]

src/apu/types.rs

@@ -427,41 +427,6 @@ pub(crate) mod common {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
-pub(crate) enum FrameSequencerState {
-    Step0Length,
-    Step1Nothing,
-    Step2LengthAndSweep,
-    Step3Nothing,
-    Step4Length,
-    Step5Nothing,
-    Step6LengthAndSweep,
-    Step7VolumeEnvelope,
-}
-
-impl FrameSequencerState {
-    pub(crate) fn step(&mut self) {
-        use FrameSequencerState::*;
-
-        *self = match *self {
-            Step0Length => Step1Nothing,
-            Step1Nothing => Step2LengthAndSweep,
-            Step2LengthAndSweep => Step3Nothing,
-            Step3Nothing => Step4Length,
-            Step4Length => Step5Nothing,
-            Step5Nothing => Step6LengthAndSweep,
-            Step6LengthAndSweep => Step7VolumeEnvelope,
-            Step7VolumeEnvelope => Step0Length,
-        };
-    }
-}
-
-impl Default for FrameSequencerState {
-    fn default() -> Self {
-        Self::Step0Length
-    }
-}
-
 bitfield! {
     pub struct SoundOutput(u8);
     impl Debug;

src/cpu.rs

@@ -104,10 +104,10 @@ impl Cpu {
         Instruction::execute(self, instruction)
     }
 
-    /// Perform the [`Cpu::fetch()`] [`Cpu::decode(opcode)`] [`Cpu::execute(instr)`]
+    /// Perform the [`Cpu::fetch()`] [`Cpu::decode()`] [`Cpu::execute()`]
     /// routine.
     ///
-    /// Handle HALT state and interrupts.
+    /// Handle HALT and interrupts.
     pub fn step(&mut self) -> Cycle {
         // Log instructions
         // if self.reg.pc > 0xFF {

src/instruction.rs

@@ -1531,7 +1531,8 @@ impl Instruction {
 
     /// Set program counter to Address.
     ///
-    /// This is explicitly meant to emulate the exact behaviour of JP, JR RET, RETI and CALL
+    /// This is explicitly meant to emulate the exact behaviour of [`Instruction::JP`], [`Instruction::JR`]
+    /// [`Instruction::RET`], [`Instruction::RETI`]
     /// (4 cycles)
     fn jump(cpu: &mut Cpu, addr: u16) {
         cpu.set_register_pair(RegisterPair::PC, addr);

src/main.rs

@@ -75,7 +75,7 @@ fn main() -> Result<()> {
     });
 
     let mut start = Instant::now();
-    let frame_time = Duration::from_secs_f64(1.0 / 60.0); // 60 Hz on Host
+    let frame_time = Duration::from_secs_f64(1.0 / 59.73); // 59.73 Hz on Host
     let mut cycle_count: Cycle = Default::default();
 
     event_loop.run(move |event, _, control_flow| {

src/ppu.rs

@@ -231,20 +231,23 @@ impl Ppu {
     fn draw(&mut self, _cycle: u32) {
         use FetcherState::*;
 
-        let iter = &mut self.obj_buffer.iter();
+        let mut iter = self.obj_buffer.iter_mut();
+        let default = &mut None;
 
         let obj_attr = loop {
-            match iter.flatten().next() {
+            match iter.next() {
-                Some(attr) => {
+                Some(attr_opt) => {
-                    if attr.x <= (self.x_pos + 8) {
+                    if let Some(attr) = attr_opt {
-                        self.fetch.back.reset();
+                        if attr.x <= (self.x_pos + 8) {
-                        self.fetch.back.pause();
+                            self.fetch.back.reset();
-                        self.fifo.pause();
+                            self.fetch.back.pause();
+                            self.fifo.pause();
 
-                        break Some(*attr);
+                            break attr_opt;
+                        }
                     }
                 }
-                None => break None,
+                None => break default,
             }
         };
 
@@ -252,10 +255,10 @@ impl Ppu {
             match self.fetch.obj.state {
                 TileNumber => {
                     self.fetch.obj.tile.with_id(attr.tile_index);
-                    self.fetch.obj.next(ToLowByteSleep);
+                    self.fetch.obj.next(SleepOne);
                 }
-                ToLowByteSleep => self.fetch.obj.next(TileLowByte),
+                SleepOne => self.fetch.obj.next(TileLow),
-                TileLowByte => {
+                TileLow => {
                     let obj_size = self.ctrl.obj_size();
 
                     let addr = PixelFetcher::get_obj_addr(&attr, &self.pos, obj_size);
@@ -263,10 +266,10 @@ impl Ppu {
                     let byte = self.read_byte(addr);
                     self.fetch.obj.tile.with_low_byte(byte);
 
-                    self.fetch.obj.next(ToHighByteSleep);
+                    self.fetch.obj.next(SleepTwo);
                 }
-                ToHighByteSleep => self.fetch.obj.next(TileHighByte),
+                SleepTwo => self.fetch.obj.next(TileHigh),
-                TileHighByte => {
+                TileHigh => {
                     let obj_size = self.ctrl.obj_size();
 
                     let addr = PixelFetcher::get_obj_addr(&attr, &self.pos, obj_size);
@@ -274,10 +277,10 @@ impl Ppu {
                     let byte = self.read_byte(addr + 1);
                     self.fetch.obj.tile.with_high_byte(byte);
 
-                    self.fetch.obj.next(ToFifoSleep);
+                    self.fetch.obj.next(SleepThree);
                 }
-                ToFifoSleep => self.fetch.obj.next(SendToFifoOne),
+                SleepThree => self.fetch.obj.next(ToFifoOne),
-                SendToFifoOne => {
+                ToFifoOne => {
                     // Load into Fifo
                     let (high, low) = self
                         .fetch
@@ -289,13 +292,12 @@ impl Ppu {
                     let tbpp = Pixels::from_bytes(high, low);
 
                     let palette_kind = attr.flags.palette();
-
-                    let end = Pixels::PIXEL_COUNT - self.fifo.obj.len();
-                    let start = Pixels::PIXEL_COUNT - end;
-
                     let x_flip = attr.flags.x_flip();
 
-                    for i in start..Pixels::PIXEL_COUNT {
+                    let pixel_count = (attr.x - self.x_pos) as usize;
+                    let start = self.fifo.obj.len();
+
+                    for i in start..pixel_count {
                         let x = if x_flip { 7 - i } else { i };
 
                         let priority = attr.flags.priority();
@@ -312,11 +314,11 @@ impl Ppu {
 
                     self.fetch.back.resume();
                     self.fifo.resume();
-                    self.obj_buffer.remove(&attr);
+                    let _ = std::mem::take(obj_attr);
 
-                    self.fetch.obj.next(SendToFifoTwo);
+                    self.fetch.obj.next(ToFifoTwo);
                 }
-                SendToFifoTwo => self.fetch.obj.reset(),
+                ToFifoTwo => self.fetch.obj.reset(),
             }
         }
 
@@ -346,31 +348,31 @@ impl Ppu {
                     self.fetch.back.tile.with_id(id);
 
                     // Move on to the Next state in 2 T-cycles
-                    self.fetch.back.next(ToLowByteSleep);
+                    self.fetch.back.next(SleepOne);
                 }
-                ToLowByteSleep => self.fetch.back.next(TileLowByte),
+                SleepOne => self.fetch.back.next(TileLow),
-                TileLowByte => {
+                TileLow => {
                     let addr = self.fetch.bg_byte_addr(&self.ctrl, &self.pos);
 
                     let low = self.read_byte(addr);
                     self.fetch.back.tile.with_low_byte(low);
 
-                    self.fetch.back.next(ToHighByteSleep);
+                    self.fetch.back.next(SleepTwo);
                 }
-                ToHighByteSleep => self.fetch.back.next(TileHighByte),
+                SleepTwo => self.fetch.back.next(TileHigh),
-                TileHighByte => {
+                TileHigh => {
                     let addr = self.fetch.bg_byte_addr(&self.ctrl, &self.pos);
 
                     let high = self.read_byte(addr + 1);
                     self.fetch.back.tile.with_high_byte(high);
 
-                    self.fetch.back.next(ToFifoSleep);
+                    self.fetch.back.next(SleepThree);
                 }
-                ToFifoSleep => self.fetch.back.next(SendToFifoOne),
+                SleepThree => self.fetch.back.next(ToFifoOne),
-                SendToFifoOne => {
+                ToFifoOne => {
-                    self.fetch.back.next(SendToFifoTwo);
+                    self.fetch.back.next(ToFifoTwo);
                 }
-                SendToFifoTwo => {
+                ToFifoTwo => {
                     if let Ok(()) = self.fetch.send_to_fifo(&mut self.fifo) {
                         self.fetch.x_pos += 1;
                         self.fetch.back.next(TileNumber);
@@ -381,12 +383,12 @@ impl Ppu {
         }
 
         if self.fifo.is_enabled() {
-            if self.x_pos == 0 && !self.fifo.back.is_empty() && self.scanline_start {
+            if self.x_pos == 0 && self.scanline_start {
                 self.to_discard = self.pos.scroll_x % 8;
                 self.scanline_start = false;
             }
 
-            if self.to_discard > 0 {
+            if self.to_discard > 0 && !self.fifo.back.is_empty() {
                 let _ = self.fifo.back.pop_front();
                 self.to_discard -= 1;
 
@@ -608,67 +610,34 @@ impl<'a> From<&'a [u8; 4]> for ObjectAttribute {
 
 #[derive(Debug)]
 struct ObjectBuffer {
-    buf: [Option<ObjectAttribute>; OBJECT_LIMIT],
+    inner: [Option<ObjectAttribute>; OBJECT_LIMIT],
     len: usize,
 }
 
-impl ObjectBuffer {
-    fn iter(&self) -> std::slice::Iter<'_, Option<ObjectAttribute>> {
-        self.into_iter()
-    }
-}
-
-impl<'a> IntoIterator for &'a ObjectBuffer {
-    type Item = &'a Option<ObjectAttribute>;
-
-    type IntoIter = std::slice::Iter<'a, Option<ObjectAttribute>>;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.buf.iter()
-    }
-}
-
-impl<'a> IntoIterator for &'a mut ObjectBuffer {
-    type Item = &'a Option<ObjectAttribute>;
-
-    type IntoIter = std::slice::Iter<'a, Option<ObjectAttribute>>;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.buf.iter()
-    }
-}
-
 impl ObjectBuffer {
     fn is_full(&self) -> bool {
         self.len == OBJECT_LIMIT
     }
 
     fn clear(&mut self) {
-        self.buf = [Default::default(); 10];
+        self.inner = [Default::default(); 10];
         self.len = 0;
     }
 
     fn add(&mut self, attr: ObjectAttribute) {
-        self.buf[self.len] = Some(attr);
+        self.inner[self.len] = Some(attr);
         self.len += 1;
     }
 
-    fn remove(&mut self, attr: &ObjectAttribute) {
+    fn iter_mut(&mut self) -> std::slice::IterMut<'_, Option<ObjectAttribute>> {
-        let maybe_index = self.buf.iter().position(|maybe_attr| match maybe_attr {
+        self.inner.iter_mut()
-            Some(other_attr) => attr == other_attr,
-            None => false,
-        });
-
-        if let Some(i) = maybe_index {
-            self.buf[i] = None;
-        }
     }
 }
 
 impl Default for ObjectBuffer {
     fn default() -> Self {
         Self {
-            buf: [Default::default(); OBJECT_LIMIT],
+            inner: [Default::default(); OBJECT_LIMIT],
             len: Default::default(),
         }
     }
@@ -903,13 +872,13 @@ impl WindowLineCounter {
 #[derive(Debug, Clone, Copy)]
 enum FetcherState {
     TileNumber,
-    ToLowByteSleep,
+    SleepOne,
-    TileLowByte,
+    TileLow,
-    ToHighByteSleep,
+    SleepTwo,
-    TileHighByte,
+    TileHigh,
-    ToFifoSleep,
+    SleepThree,
-    SendToFifoOne,
+    ToFifoOne,
-    SendToFifoTwo,
+    ToFifoTwo,
 }
 
 impl Default for FetcherState {