fix(apu): replace mpsc with spsc ringbuffer

Cargo.lock

@@ -178,6 +178,12 @@ version = "1.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b700ce4376041dcd0a327fd0097c41095743c4c8af8887265942faf1100bd040"
 
+[[package]]
+name = "cache-padded"
+version = "1.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "631ae5198c9be5e753e5cc215e1bd73c2b466a3565173db433f52bb9d3e66dba"
+
 [[package]]
 name = "calloop"
 version = "0.6.5"
@@ -684,10 +690,10 @@ dependencies = [
  "anyhow",
  "bitfield",
  "clap",
- "crossbeam-channel",
  "gilrs",
  "pixels",
  "rodio",
+ "rtrb",
  "winit",
  "winit_input_helper",
 ]
@@ -1677,6 +1683,15 @@ dependencies = [
  "petgraph",
 ]
 
+[[package]]
+name = "rtrb"
+version = "0.1.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "318256ac02f7e11a48a10339ba5dca8bd7eb17496abf384e8ea909bb2ae5275f"
+dependencies = [
+ "cache-padded",
+]
+
 [[package]]
 name = "rustc-hash"
 version = "1.1.0"

Cargo.toml

@@ -15,7 +15,7 @@ pixels = "^0.5"
 winit = "^0.25"
 winit_input_helper = "^0.10"
 rodio = "^0.14"
-crossbeam-channel = "^0.5"
+rtrb = "^0.1.4"
 
 [profile.release]
 debug = true

src/apu.rs

@@ -1,6 +1,6 @@
 use crate::bus::BusIo;
 use crate::emu::SM83_CLOCK_SPEED;
-use gen::{AudioBuffer, AudioSender};
+use gen::SampleProducer;
 use types::ch1::{Sweep, SweepDirection};
 use types::ch3::Volume as Ch3Volume;
 use types::ch4::{CounterWidth, Frequency as Ch4Frequency, PolynomialCounter};
@@ -10,14 +10,10 @@ use types::{ChannelControl, FrameSequencerState, SoundOutput};
 pub mod gen;
 mod types;
 
+const SAMPLE_INCREMENT: u64 = gen::SAMPLE_RATE as u64;
 const WAVE_PATTERN_RAM_LEN: usize = 0x10;
 
-const SAMPLE_RATE: u32 = 48000; // Hz
-const AUDIO_BUFFER_LEN: usize = 512;
-const CHANNEL_COUNT: usize = 2;
-const SAMPLE_INCREMENT: u64 = SAMPLE_RATE as u64;
-
-#[derive(Default, Debug, Clone)]
+#[derive(Default, Debug)]
 pub struct Apu {
     ctrl: SoundControl,
     /// Tone & Sweep
@@ -33,15 +29,14 @@ pub struct Apu {
     frame_seq_state: FrameSequencerState,
     div_prev: Option<u8>,
 
-    sender: Option<AudioSender<f32>>,
+    prod: Option<SampleProducer<f32>>,
     sample_counter: u64,
-
-    buffer: AudioBuffer<(f32, f32)>,
 }
 
 impl BusIo for Apu {
     fn read_byte(&self, addr: u16) -> u8 {
         match addr & 0x00FF {
+            0x10 => self.ch1.sweep(),
             0x11 => self.ch1.duty(),
             0x12 => self.ch1.envelope(),
             0x14 => self.ch1.freq_hi(),
@@ -136,10 +131,11 @@ impl Apu {
 
         self.div_prev = Some(bit_5);
 
-        if self.sender.is_some() && self.sample_counter >= SM83_CLOCK_SPEED {
+        if let Some(ref mut prod) = self.prod {
+            if self.sample_counter >= SM83_CLOCK_SPEED {
                 self.sample_counter %= SM83_CLOCK_SPEED;
-            // Sample the APU
 
+                // Sample the APU
                 let ch1_amplitude = self.ch1.amplitude();
                 let ch1_left = self.ctrl.output.ch1_left() as u8 as f32 * ch1_amplitude;
                 let ch1_right = self.ctrl.output.ch1_right() as u8 as f32 * ch1_amplitude;
@@ -159,32 +155,21 @@ impl Apu {
                 let left = (ch1_left + ch2_left + ch3_left + ch4_left) / 4.0;
                 let right = (ch1_right + ch2_right + ch3_right + ch4_right) / 4.0;
 
-            self.buffer.push_back((left, right));
+                prod.push(left)
+                    .and(prod.push(right))
+                    .expect("Add samples to ring buffer");
+            }
         }
     }
 
-    pub fn set_audio_sender(&mut self, sender: AudioSender<f32>) {
-        self.sender = Some(sender);
+    pub fn set_producer(&mut self, prod: SampleProducer<f32>) {
+        self.prod = Some(prod);
     }
 
     pub(crate) fn is_full(&self) -> bool {
-        self.buffer.len() >= AUDIO_BUFFER_LEN * CHANNEL_COUNT
-    }
-
-    pub(crate) fn flush_samples(&mut self) {
-        if let Some(sender) = self.sender.as_ref() {
-            while self.buffer.len() >= CHANNEL_COUNT {
-                match self.buffer.pop_front() {
-                    Some((left, right)) => {
-                        sender
-                            .send_samples(left, right)
-                            .expect("Successfully sent samples across threads");
-                    }
-                    None => unreachable!(
-                        "While loop ensures that there are at least two items in AudioBuffer"
-                    ),
-                }
-            }
+        match self.prod.as_ref() {
+            Some(prod) => prod.is_full(),
+            None => false,
         }
     }
 
@@ -349,7 +334,7 @@ impl Apu {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct SoundControl {
     /// 0xFF24 | NR50 - Channel Control
     channel: ChannelControl,
@@ -394,7 +379,7 @@ impl SoundControl {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Channel1 {
     /// 0xFF10 | NR10 - Channel 1 Sweep Register
     sweep: Sweep,
@@ -557,7 +542,7 @@ impl Channel1 {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Channel2 {
     /// 0xFF16 | NR21 - Channel 2 Sound length / Wave Pattern Duty
     duty: SoundDuty,
@@ -660,7 +645,7 @@ impl Channel2 {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Channel3 {
     /// 0xFF1A | NR30 - Channel 3 Sound on/off
     enabled: bool,
@@ -798,7 +783,7 @@ impl Channel3 {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Channel4 {
     /// 0xFF20 | NR41 - Channel 4 Sound Length
     len: u8,

src/apu/gen.rs

@@ -1,47 +1,74 @@
-use super::{AUDIO_BUFFER_LEN, CHANNEL_COUNT, SAMPLE_RATE};
-use crossbeam_channel::{Receiver, SendError, Sender};
 use rodio::Source;
-use std::collections::VecDeque;
+use rtrb::{Consumer, Producer, PushError, RingBuffer};
 
-pub struct AudioMPSC;
+pub(crate) const SAMPLE_RATE: u32 = 48000; // Hz
+const CHANNEL_COUNT: usize = 2;
+const AUDIO_BUFFER_LEN: usize = 4096;
 
-impl AudioMPSC {
-    pub fn init() -> (AudioSender<f32>, AudioReceiver<f32>) {
-        // TODO: Can we provide an upper limit for this?
-        // The larger this channel is, the more lag there is between the Audio and
-        // Emulator
-        let (send, recv) = crossbeam_channel::unbounded();
+pub struct AudioSPSC<T> {
+    inner: RingBuffer<T>,
+}
 
-        (AudioSender { inner: send }, AudioReceiver { inner: recv })
+impl<T> Default for AudioSPSC<T> {
+    fn default() -> Self {
+        Self {
+            inner: RingBuffer::new(AUDIO_BUFFER_LEN * CHANNEL_COUNT),
+        }
     }
 }
 
-#[derive(Debug, Clone)]
-pub struct AudioSender<T> {
-    inner: Sender<T>,
-}
+impl<T> AudioSPSC<T> {
+    pub fn new(capacity: usize) -> Self {
+        Self {
+            inner: RingBuffer::new(capacity),
+        }
+    }
 
-impl<T> AudioSender<T> {
-    pub(crate) fn send_samples(&self, left: T, right: T) -> Result<(), SendError<T>> {
-        self.inner.send(left).and(self.inner.send(right))?;
-        Ok(())
+    pub fn init(self) -> (SampleProducer<T>, SampleConsumer<T>) {
+        let (prod, cons) = self.inner.split();
+
+        (
+            SampleProducer { inner: prod },
+            SampleConsumer { inner: cons },
+        )
     }
 }
 
-pub struct AudioReceiver<T> {
-    inner: Receiver<T>,
+pub struct SampleProducer<T> {
+    inner: Producer<T>,
 }
 
-impl<T> Iterator for AudioReceiver<T> {
-    type Item = T;
+impl<T> SampleProducer<T> {
+    pub(crate) fn push(&mut self, value: T) -> Result<(), PushError<T>> {
+        self.inner.push(value)
+    }
+
+    pub(crate) fn is_full(&self) -> bool {
+        self.inner.is_full()
+    }
+}
+
+impl<T> std::fmt::Debug for SampleProducer<T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct(&format!("SampleProducer<{}>", std::any::type_name::<T>()))
+            .finish_non_exhaustive()
+    }
+}
+
+pub struct SampleConsumer<T> {
+    inner: Consumer<T>,
+}
+
+impl Iterator for SampleConsumer<f32> {
+    type Item = f32;
 
     fn next(&mut self) -> Option<Self::Item> {
-        // TODO: Should this never return none?
-        self.inner.recv().ok()
+        // As of 2021-07-28, PopError can only be Empty
+        Some(self.inner.pop().unwrap_or_default())
     }
 }
 
-impl<T: rodio::Sample> Source for AudioReceiver<T> {
+impl Source for SampleConsumer<f32> {
     fn current_frame_len(&self) -> Option<usize> {
         // A frame changes when the samples rate or
         // number of channels change. This will never happen, so
@@ -63,30 +90,3 @@ impl<T: rodio::Sample> Source for AudioReceiver<T> {
         None
     }
 }
-
-#[derive(Debug, Clone)]
-pub(crate) struct AudioBuffer<T> {
-    inner: VecDeque<T>,
-}
-
-impl<T> Default for AudioBuffer<T> {
-    fn default() -> Self {
-        Self {
-            inner: VecDeque::with_capacity(AUDIO_BUFFER_LEN * CHANNEL_COUNT),
-        }
-    }
-}
-
-impl<T> AudioBuffer<T> {
-    pub(crate) fn push_back(&mut self, value: T) {
-        self.inner.push_back(value)
-    }
-
-    pub(crate) fn pop_front(&mut self) -> Option<T> {
-        self.inner.pop_front()
-    }
-
-    pub(crate) fn len(&self) -> usize {
-        self.inner.len()
-    }
-}

src/bus.rs

@@ -11,7 +11,7 @@ use std::{fs::File, io::Read};
 
 const BOOT_ROM_SIZE: usize = 0x100;
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct Bus {
     boot: Option<[u8; BOOT_ROM_SIZE]>, // Boot ROM is 256b long
     cartridge: Option<Cartridge>,
@@ -70,10 +70,6 @@ impl Bus {
         &self.apu
     }
 
-    pub(crate) fn apu_mut(&mut self) -> &mut Apu {
-        &mut self.apu
-    }
-
     pub(crate) fn clock(&mut self) {
         self.ppu.clock();
         self.timer.clock();

src/cartridge.rs

@@ -8,7 +8,7 @@ const RAM_SIZE_ADDRESS: usize = 0x0149;
 const ROM_SIZE_ADDRESS: usize = 0x0148;
 const MBC_TYPE_ADDRESS: usize = 0x0147;
 
-#[derive(Debug, Clone, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Cartridge {
     memory: Vec<u8>,
     title: Option<String>,
@@ -144,7 +144,7 @@ impl BusIo for Cartridge {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 struct Mbc1 {
     /// 5-bit number
     rom_bank: u8,
@@ -293,7 +293,7 @@ enum MBC3Device {
     RealTimeClock,
 }
 
-#[derive(Debug, Clone, Default)]
+#[derive(Debug, Default)]
 struct MBC3 {
     /// 7-bit Number
     rom_bank: u8,
@@ -361,7 +361,7 @@ impl MemoryBankController for MBC3 {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug)]
 struct NoMbc {}
 
 impl MemoryBankController for NoMbc {
@@ -374,24 +374,11 @@ impl MemoryBankController for NoMbc {
     }
 }
 
-trait MemoryBankController: CloneMbc {
+trait MemoryBankController {
     fn handle_read(&self, addr: u16) -> MbcResult;
     fn handle_write(&mut self, addr: u16, byte: u8);
 }
 
-trait CloneMbc {
-    fn clone_mbc(&self) -> Box<dyn MemoryBankController>;
-}
-
-impl<T> CloneMbc for T
-where
-    T: MemoryBankController + Clone + 'static,
-{
-    fn clone_mbc<'a>(&self) -> Box<dyn MemoryBankController> {
-        Box::new(self.clone())
-    }
-}
-
 #[derive(Debug, Clone, Copy)]
 enum MbcResult {
     Address(usize),
@@ -485,7 +472,7 @@ impl Default for BankCount {
 
 impl BankCount {
     // https://hacktix.github.io/GBEDG/mbcs/#rom-size
-    fn size(self) -> u32 {
+    fn size(&self) -> u32 {
         use BankCount::*;
 
         match self {
@@ -533,12 +520,6 @@ impl std::fmt::Debug for Box<dyn MemoryBankController> {
     }
 }
 
-impl std::clone::Clone for Box<dyn MemoryBankController> {
-    fn clone(&self) -> Self {
-        self.clone_mbc()
-    }
-}
-
 impl Default for Box<dyn MemoryBankController> {
     fn default() -> Self {
         Box::new(Mbc1::default())

src/cpu.rs

@@ -4,11 +4,10 @@ use crate::instruction::{Cycle, Instruction};
 use crate::interrupt::{InterruptEnable, InterruptFlag};
 use crate::joypad::Joypad;
 use crate::ppu::Ppu;
-use crate::timer::Timer;
 use bitfield::bitfield;
 use std::fmt::{Display, Formatter, Result as FmtResult};
 
-#[derive(Debug, Clone, Default)]
+#[derive(Debug, Default)]
 pub struct Cpu {
     pub bus: Bus,
     reg: Registers,
@@ -45,8 +44,8 @@ impl Cpu {
         })
     }
 
-    pub(crate) fn ime(&self) -> ImeState {
-        self.ime
+    pub(crate) fn ime(&self) -> &ImeState {
+        &self.ime
     }
 
     pub(crate) fn set_ime(&mut self, state: ImeState) {
@@ -61,8 +60,8 @@ impl Cpu {
         self.halted = None;
     }
 
-    pub(crate) fn halted(&self) -> Option<HaltState> {
-        self.halted
+    pub(crate) fn halted(&self) -> Option<&HaltState> {
+        self.halted.as_ref()
     }
 
     fn inc_pc(&mut self) {
@@ -127,8 +126,6 @@ impl Cpu {
             if !self.bus.apu().is_full() {
                 self.bus.clock();
                 elapsed += 1;
-            } else {
-                self.bus.apu_mut().flush_samples();
             }
         }
 
@@ -178,10 +175,6 @@ impl Cpu {
         &mut self.bus.joypad
     }
 
-    pub(crate) fn timer(&self) -> &Timer {
-        &self.bus.timer
-    }
-
     fn check_ime(&mut self) {
         match self.ime {
             ImeState::Pending => {
@@ -267,7 +260,7 @@ impl Cpu {
     }
 }
 
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Clone, Copy)]
 enum State {
     Execute,
     // Halt,
@@ -292,7 +285,6 @@ impl Cpu {
             E => self.reg.e = value,
             H => self.reg.h = value,
             L => self.reg.l = value,
-            Flag => self.flags = value.into(),
         }
     }
 
@@ -307,7 +299,6 @@ impl Cpu {
             E => self.reg.e,
             H => self.reg.h,
             L => self.reg.l,
-            Flag => self.flags.into(),
         }
     }
 
@@ -400,7 +391,7 @@ impl Cpu {
     }
 }
 
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Clone, Copy)]
 pub(crate) enum Register {
     A,
     B,
@@ -409,10 +400,9 @@ pub(crate) enum Register {
     E,
     H,
     L,
-    Flag,
 }
 
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Clone, Copy)]
 pub(crate) enum RegisterPair {
     AF,
     BC,
@@ -422,7 +412,7 @@ pub(crate) enum RegisterPair {
     PC,
 }
 
-#[derive(Debug, Copy, Clone, Default)]
+#[derive(Debug, Default)]
 struct Registers {
     a: u8,
     b: u8,

src/instruction.rs

@@ -2,7 +2,7 @@ use crate::bus::BusIo;
 use crate::cpu::{Cpu, Flags, HaltState, ImeState, Register, RegisterPair};
 use std::{convert::TryFrom, fmt::Debug};
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 #[allow(clippy::upper_case_acronyms)]
 pub(crate) enum Instruction {
     NOP,
@@ -51,26 +51,26 @@ pub(crate) enum Instruction {
     SET(u8, InstrRegister),
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum JPTarget {
     RegisterPair(RegisterPair),
     ImmediateWord(u16),
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum Registers {
     Byte(InstrRegister),
     Word(RegisterPair),
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum MATHTarget {
     Register(InstrRegister),
     RegisterPair(RegisterPair),
     ImmediateByte(u8),
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum LDTarget {
     IndirectC,
     Register(InstrRegister),
@@ -82,7 +82,7 @@ pub(crate) enum LDTarget {
     ByteAtAddressWithOffset(u8),
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum InstrRegisterPair {
     AF,
     BC,
@@ -94,7 +94,7 @@ pub(crate) enum InstrRegisterPair {
     DecrementHL,
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum InstrRegister {
     A,
     B,
@@ -106,7 +106,7 @@ pub(crate) enum InstrRegister {
     IndirectHL, // (HL)
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum JumpCondition {
     NotZero,
     Zero,
@@ -115,7 +115,7 @@ pub(crate) enum JumpCondition {
     Always,
 }
 
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug)]
 struct Table;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Default)]
@@ -1887,7 +1887,6 @@ impl TryFrom<Register> for InstrRegister {
             Register::E => Ok(Self::E),
             Register::H => Ok(Self::H),
             Register::L => Ok(Self::L),
-            Register::Flag => Err("Can not convert Register::Flag to InstrRegister"),
         }
     }
 }

src/interrupt.rs

@@ -1,6 +1,6 @@
 use bitfield::bitfield;
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Interrupt {
     pub(crate) flag: InterruptFlag,
     pub(crate) enable: InterruptEnable,

src/joypad.rs

@@ -1,7 +1,7 @@
 use gilrs::{Button, Event as GamepadEvent, EventType as GamepadEventType};
 use winit_input_helper::WinitInputHelper;
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug)]
 pub struct Joypad {
     /// 0xFF00 | P1/JOYP - Player 1 Joypad
     pub(crate) p1: u8,
@@ -48,7 +48,7 @@ impl Joypad {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct JoypadState {
     // Direction Row
     dpad_down: ButtonEvent,

src/lib.rs

@@ -1,4 +1,4 @@
-pub use apu::gen::AudioMPSC;
+pub use apu::gen::AudioSPSC;
 pub use instruction::Cycle;
 
 pub const GB_WIDTH: usize = 160;

src/main.rs

@@ -1,9 +1,9 @@
 use anyhow::{anyhow, Result};
 use clap::{crate_authors, crate_description, crate_name, crate_version, App, Arg};
-use gb::{AudioMPSC, Cycle, GB_HEIGHT, GB_WIDTH};
+use gb::{AudioSPSC, Cycle, GB_HEIGHT, GB_WIDTH};
 use gilrs::Gilrs;
 use pixels::{PixelsBuilder, SurfaceTexture};
-use rodio::OutputStream;
+use rodio::{OutputStream, Sink};
 use std::time::Instant;
 use winit::dpi::LogicalSize;
 use winit::event::{Event, VirtualKeyCode};
@@ -65,16 +65,18 @@ fn main() -> Result<()> {
             .build()?
     };
 
-    let (send, recv) = AudioMPSC::init();
-    game_boy.apu_mut().set_audio_sender(send);
+    let spsc: AudioSPSC<f32> = Default::default();
+    let (prod, cons) = spsc.init();
+
+    game_boy.apu_mut().set_producer(prod);
 
     // Initialize Audio
     let (_stream, stream_handle) = OutputStream::try_default().expect("Initialized Audio");
+    let sink = Sink::try_new(&stream_handle)?;
+    sink.append(cons);
 
     std::thread::spawn(move || {
-        stream_handle
-            .play_raw(recv)
-            .expect("Failed to play Audio Source");
+        sink.sleep_until_end();
     });
 
     let mut now = Instant::now();

src/ppu.rs

@@ -33,7 +33,7 @@ const LIGHT_GRAY: [u8; 4] = 0xAEBA89FFu32.to_be_bytes();
 const DARK_GRAY: [u8; 4] = 0x5E6745FFu32.to_be_bytes();
 const BLACK: [u8; 4] = 0x202020FFu32.to_be_bytes();
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct Ppu {
     pub(crate) int: Interrupt,
     /// 0xFF40 | LCDC - LCD Control
@@ -489,7 +489,7 @@ impl Default for Ppu {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Interrupt {
     _vblank: bool,
     _lcd_stat: bool,
@@ -513,7 +513,7 @@ impl Interrupt {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct ScreenPosition {
     /// 0xFF42 | SCY - Scroll Y
     pub(crate) scroll_y: u8,
@@ -529,7 +529,7 @@ pub(crate) struct ScreenPosition {
     pub(crate) window_x: u8,
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Monochrome {
     /// 0xFF47 | BGP - Background Palette Data
     pub(crate) bg_palette: BackgroundPalette,
@@ -539,7 +539,7 @@ pub(crate) struct Monochrome {
     pub(crate) obj_palette_1: ObjectPalette,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub(crate) struct ObjectAttributeTable {
     buf: Box<[u8; OAM_SIZE]>,
 }
@@ -606,7 +606,7 @@ impl<'a> From<&'a [u8; 4]> for ObjectAttribute {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug)]
 struct ObjectBuffer {
     buf: [Option<ObjectAttribute>; OBJECT_LIMIT],
     len: usize,
@@ -674,7 +674,7 @@ impl Default for ObjectBuffer {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct PixelFetcher {
     x_pos: u8,
     back: BackgroundFetcher,
@@ -793,7 +793,7 @@ trait Fetcher {
     fn hblank_reset(&mut self);
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug)]
 struct BackgroundFetcher {
     state: FetcherState,
     tile: TileBuilder,
@@ -859,7 +859,7 @@ impl Default for BackgroundFetcher {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct ObjectFetcher {
     state: FetcherState,
     tile: TileBuilder,
@@ -881,7 +881,7 @@ impl Fetcher for ObjectFetcher {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct WindowLineCounter {
     count: u8,
 }
@@ -918,12 +918,12 @@ impl Default for FetcherState {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct BgPixelProperty {
     shade_id: u8,
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct ObjPixelProperty {
     shade_id: u8,
     palette_kind: ObjectPaletteKind,
@@ -932,7 +932,7 @@ struct ObjPixelProperty {
 
 // FIXME: Fifo Registers have a known size. Are heap allocations
 // really necessary here?
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 struct PixelFifo {
     back: VecDeque<BgPixelProperty>,
     obj: VecDeque<ObjPixelProperty>,
@@ -963,7 +963,7 @@ impl Default for PixelFifo {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct TileBuilder {
     id: Option<u8>,
     low: Option<u8>,
@@ -988,7 +988,7 @@ impl TileBuilder {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct OamScanState {
     count: u8,
     mode: OamScanMode,
@@ -1009,8 +1009,8 @@ impl OamScanState {
         self.count
     }
 
-    fn mode(&self) -> OamScanMode {
-        self.mode
+    fn mode(&self) -> &OamScanMode {
+        &self.mode
     }
 
     fn next(&mut self) {
@@ -1035,7 +1035,7 @@ impl Default for OamScanMode {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 struct WindowStatus {
     /// This will be true if WY == LY at any point in the frame thus far
     coincidence: bool,

src/ppu/dma.rs

@@ -1,6 +1,6 @@
 use crate::instruction::Cycle;
 
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Default)]
 pub(crate) struct DirectMemoryAccess {
     pub(crate) state: DmaState,
     cycle: Cycle,
@@ -28,7 +28,7 @@ impl DirectMemoryAccess {
 
                 let src_addr = self
                     .start
-                    .addr
+                    .0
                     .as_mut()
                     .expect("Source Address present during DMA Transfer");
 
@@ -58,7 +58,7 @@ impl DirectMemoryAccess {
     fn reset(&mut self) {
         self.cycle = Cycle::new(0);
         self.state = DmaState::Disabled;
-        self.start.addr = None;
+        self.start.0 = None;
     }
 }
 
@@ -75,26 +75,21 @@ impl Default for DmaState {
     }
 }
 
-#[derive(Debug, Default, Clone, Copy)]
-pub(crate) struct DmaAddress {
-    /// The current *source* address of the DMA Transfer
-    ///
-    /// NB: Will be None if no DMA Transfer is in progress
-    addr: Option<u16>,
-}
+#[derive(Debug, Clone, Copy, Default)]
+pub(crate) struct DmaAddress(Option<u16>);
 
 impl DmaAddress {
     pub(crate) fn update(&mut self, byte: u8, state: &mut DmaState) {
         let start = (byte as u16) << 8;
 
-        self.addr = Some(start);
+        self.0 = Some(start);
         *state = DmaState::Pending;
     }
 }
 
 impl From<DmaAddress> for u8 {
     fn from(ctrl: DmaAddress) -> Self {
-        match ctrl.addr {
+        match ctrl.0 {
             Some(addr) => (addr >> 8) as u8,
             None => 0xFF, // TODO: What garbage value should be here?
         }

src/serial.rs

@@ -1,6 +1,6 @@
 use bitfield::bitfield;
 
-#[derive(Debug, Clone, Copy, Default)]
+#[derive(Debug, Default)]
 pub(crate) struct Serial {
     /// 0xFF01 | SB - Serial Transfer Data
     pub(crate) next: u8,

src/timer.rs

@@ -1,6 +1,6 @@
 use bitfield::bitfield;
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug)]
 pub(crate) struct Timer {
     /// 0xFF07 | TAC - Timer Control
     pub(crate) ctrl: TimerControl,

src/work_ram.rs

@@ -5,7 +5,7 @@ const VARIABLE_WORK_RAM_SIZE: usize = WORK_RAM_SIZE;
 const WORK_RAM_START_ADDRESS: usize = 0xC000;
 const VARIABLE_WORK_RAM_START_ADDRESS: usize = 0xD000;
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub(crate) struct WorkRam {
     bank: Box<[u8; WORK_RAM_SIZE]>,
 }
@@ -39,7 +39,7 @@ enum BankNumber {
     Seven = 7,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub(crate) struct VariableWorkRam {
     current: BankNumber,
     bank_n: Box<[[u8; VARIABLE_WORK_RAM_SIZE]; 7]>, // 4K for Variable amount of Banks (Banks 1 -> 7) in Game Boy Colour
@@ -59,8 +59,8 @@ impl VariableWorkRam {
         self.current = bank;
     }
 
-    fn get_current_bank(&self) -> BankNumber {
-        self.current
+    fn get_current_bank(&self) -> &BankNumber {
+        &self.current
     }
 }