Fix some sdl unsound usage and allowed the ppu layer windows to be cl…

…osed

First of all allow the ppu layer windows to be closed by managing them in the main thread

Also have been calling SDL_PumpEvents in the incorrect thread + managed the windows not in the video subsystem initializing thread.

common/src/joypad_menu/mod.rs

@@ -52,10 +52,9 @@ impl<'a, T, S: AsRef<str>, MR:MenuRenderer<T, S>> JoypadMenu<'a, T, S, MR>{
                 }
             }
         }
-        // Busy wait untill A is released in order to not leak the button press to the emulation
-        while joypad.buttons[Button::A as usize]{
-            provider.provide(&mut joypad);
-        }
+
+        // TODO: used to busy wait, now SDL knows how to not to leak the confirm press, verify on RPI.
+
         return &self.options[self.selection].value;
     }
 }

core/src/ppu/vram.rs

@@ -1,5 +1,3 @@
-use core::convert::TryInto;
-
 const VRAM_SIZE:usize = 0x4000;
 const VRAM_BANK_SIZE:usize = 0x2000;
 

sdl/Cargo.toml

@@ -20,7 +20,7 @@ cfg-if = "1.0"
 wav = "1.0"
 
 [features]
-default = ["static-sdl", "dbg"]
+default = ["static-sdl"]
 sdl-resample = []
 push-audio = []
 static-sdl = ["sdl2/bundled", "sdl2/static-link"]

sdl/src/main.rs

@@ -31,30 +31,29 @@ cfg_if::cfg_if!{
 
 use crate::audio::*;
 use magenboy_common::{audio::ResampledAudioDevice, joypad_menu::*, mbc_handler::*, menu::*, mpmc_gfx_device::*};
-use magenboy_core::{keypad::button::Button, apu::audio_device::*, machine::{gameboy::GameBoy, Mode}, ppu::{gb_ppu::{BUFFERS_NUMBER, SCREEN_HEIGHT, SCREEN_WIDTH}, gfx_device::{GfxDevice, Pixel}}, mmu::{GBC_BOOT_ROM_SIZE, external_memory_bus::Bootrom, GB_BOOT_ROM_SIZE}};
+use magenboy_core::{apu::audio_device::*, keypad::{joypad::NUM_OF_KEYS}, machine::{gameboy::GameBoy, Mode}, mmu::{external_memory_bus::Bootrom, GBC_BOOT_ROM_SIZE, GB_BOOT_ROM_SIZE}, ppu::{gb_ppu::{BUFFERS_NUMBER, SCREEN_HEIGHT, SCREEN_WIDTH}, gfx_device::{GfxDevice, Pixel}}};
 use std::{fs, env, result::Result, vec::Vec, convert::TryInto};
 use log::info;
 use magenboy_core::GB_FREQUENCY;
 use sdl2::sys::*;
 #[cfg(feature = "dbg")]
 use crate::terminal_debugger::TerminalDebugger;
+use crate::sdl::sdl_gfx_device::SdlGfxDevice;
 
 const TURBO_MUL:u8 = 1;
 
 const SCREEN_SCALE:usize = 4;
 use sdl2::sys::SDL_Scancode;
-fn buttons_mapper(button:&Button)->SDL_Scancode{
-    match button{
-        Button::A       => SDL_Scancode::SDL_SCANCODE_X,
-        Button::B       => SDL_Scancode::SDL_SCANCODE_Z,
-        Button::Start   => SDL_Scancode::SDL_SCANCODE_S,
-        Button::Select  => SDL_Scancode::SDL_SCANCODE_A,
-        Button::Up      => SDL_Scancode::SDL_SCANCODE_UP,
-        Button::Down    => SDL_Scancode::SDL_SCANCODE_DOWN,
-        Button::Right   => SDL_Scancode::SDL_SCANCODE_RIGHT,
-        Button::Left    => SDL_Scancode::SDL_SCANCODE_LEFT
-    }
-}
+const KEYBOARD_MAPPING:[SDL_Scancode; NUM_OF_KEYS] = [
+    SDL_Scancode::SDL_SCANCODE_X,
+    SDL_Scancode::SDL_SCANCODE_Z,
+    SDL_Scancode::SDL_SCANCODE_S,
+    SDL_Scancode::SDL_SCANCODE_A,
+    SDL_Scancode::SDL_SCANCODE_UP,
+    SDL_Scancode::SDL_SCANCODE_DOWN,
+    SDL_Scancode::SDL_SCANCODE_RIGHT,
+    SDL_Scancode::SDL_SCANCODE_LEFT
+];
 
 
 fn check_for_terminal_feature_flag(args:&Vec::<String>, flag:&str)->bool{
@@ -79,11 +78,11 @@ fn main() {
     }
 
     // Initialize the gfx first cause it initialize both the screen and the sdl context for the joypad
-    let mut gfx_device = sdl::sdl_gfx_device::SdlGfxDevice::new(header.as_str(), SCREEN_SCALE, TURBO_MUL,
+    let mut gfx_device: SdlGfxDevice = SdlGfxDevice::new(header.as_str(), SCREEN_SCALE, TURBO_MUL,
     check_for_terminal_feature_flag(&args, "--no-vsync"), check_for_terminal_feature_flag(&args, "--full-screen"));
 
     while !(EMULATOR_STATE.exit.load(std::sync::atomic::Ordering::Relaxed)){
-        let mut provider = sdl::sdl_joypad_provider::SdlJoypadProvider::new(buttons_mapper);
+        let mut provider = sdl::sdl_joypad_provider::SdlJoypadProvider::new(KEYBOARD_MAPPING);
 
         let program_name = if check_for_terminal_feature_flag(&args, "--rom-menu"){
             let roms_path = get_terminal_feature_flag_value(&args, "--rom-menu", "Error! no roms folder specified");
@@ -99,20 +98,19 @@ fn main() {
         let (s,r) = crossbeam_channel::bounded(BUFFERS_NUMBER - 1);
         let mpmc_device = MpmcGfxDevice::new(s);
 
+        #[cfg(feature = "dbg")]
+        let (debugger_s, debugger_r) = crossbeam_channel::bounded::<terminal_debugger::PpuLayerResult>(0);
+
         let args_clone = args.clone();
         let emualation_thread = std::thread::Builder::new()
             .name("Emualtion Thread".to_string())
             .stack_size(0x100_0000)
-            .spawn(move || emulation_thread_main(args_clone, program_name, mpmc_device))
+            .spawn(move || emulation_thread_main(args_clone, program_name, mpmc_device, #[cfg(feature = "dbg")]debugger_s))
             .unwrap();
 
-
         unsafe{
-            let mut event: std::mem::MaybeUninit<SDL_Event> = std::mem::MaybeUninit::uninit();
-
             loop{
-                if SDL_PollEvent(event.as_mut_ptr()) != 0{
-                    let event: SDL_Event = event.assume_init();
+                if let Some(event) = gfx_device.poll_event(){
                     if event.type_ == SDL_EventType::SDL_QUIT as u32{
                         EMULATOR_STATE.exit.store(true, std::sync::atomic::Ordering::Relaxed);
                         break;
@@ -122,10 +120,22 @@ fn main() {
                     }
                 }
 
-                match r.recv() {
-                    Result::Ok(buffer) => gfx_device.swap_buffer(&*(buffer as *const [Pixel; SCREEN_WIDTH * SCREEN_HEIGHT])),
-                    Result::Err(_) => break,
-                }
+                cfg_if::cfg_if! {if #[cfg(feature = "dbg")] {
+                    crossbeam_channel::select! {
+                        recv(r) -> msg => {
+                            let Ok(buffer) = msg else {break};
+                            gfx_device.swap_buffer(&*(buffer as *const [Pixel; SCREEN_WIDTH * SCREEN_HEIGHT]));
+                        },
+                        recv(debugger_r)-> msg => {
+                            let Ok(result) = msg else {break};
+                            let mut window = sdl::sdl_gfx_device::PpuLayerWindow::new(result.1);
+                            window.run(result.0);
+                        }
+                    }
+                }else{
+                    let Ok(buffer) = r.recv() else {break};
+                    gfx_device.swap_buffer(&*(buffer as *const [Pixel; SCREEN_WIDTH * SCREEN_HEIGHT]));
+                }}
             }
 
             drop(r);
@@ -140,7 +150,7 @@ fn main() {
 }
 
 // Receiving usize and not raw ptr cause in rust you cant pass a raw ptr to another thread
-fn emulation_thread_main(args: Vec<String>, program_name: String, spsc_gfx_device: MpmcGfxDevice) {
+fn emulation_thread_main(args: Vec<String>, program_name: String, spsc_gfx_device: MpmcGfxDevice, #[cfg(feature = "dbg")] debugger_sender: crossbeam_channel::Sender<terminal_debugger::PpuLayerResult>) {
     let mut devices: Vec::<Box::<dyn AudioDevice>> = Vec::new();
     let audio_device = sdl::ChosenAudioDevice::<ChosenResampler>::new(44100, TURBO_MUL);
     devices.push(Box::new(audio_device));
@@ -152,7 +162,7 @@ fn emulation_thread_main(args: Vec<String>, program_name: String, spsc_gfx_devic
     }
 
     let audio_devices = MultiAudioDevice::new(devices);
-    let joypad_provider = sdl::sdl_joypad_provider::SdlJoypadProvider::new(buttons_mapper);
+    let joypad_provider = sdl::sdl_joypad_provider::SdlJoypadProvider::new(KEYBOARD_MAPPING);
 
     let bootrom_path = if check_for_terminal_feature_flag(&args, "--bootrom"){
         get_terminal_feature_flag_value(&args, "--bootrom", "Error! you must specify a value for the --bootrom parameter")
@@ -198,7 +208,7 @@ fn emulation_thread_main(args: Vec<String>, program_name: String, spsc_gfx_devic
     let mut gameboy = GameBoy::new(
         mbc, joypad_provider, audio_devices, spsc_gfx_device, 
         #[cfg(feature = "dbg")]
-        TerminalDebugger::new(),
+        TerminalDebugger::new(debugger_sender),
         bootrom, mode);
 
     info!("initialized gameboy successfully!");

sdl/src/sdl/sdl_gfx_device.rs

@@ -1,6 +1,6 @@
 use std::ffi::{CString, c_void};
 use sdl2::sys::*;
-use magenboy_core::{ppu::gb_ppu::{SCREEN_HEIGHT, SCREEN_WIDTH}, {Pixel, GfxDevice}, debugger::PpuLayer};
+use magenboy_core::{ppu::gb_ppu::{SCREEN_HEIGHT, SCREEN_WIDTH}, GfxDevice, Pixel};
 use crate::sdl::utils::get_sdl_error_message;
 
 pub struct SdlGfxDevice{
@@ -63,6 +63,18 @@ impl SdlGfxDevice{
             turbo_mul
         }
     }
+
+    pub fn poll_event(&self)->Option<SDL_Event>{
+        unsafe{
+            let mut event: std::mem::MaybeUninit<SDL_Event> = std::mem::MaybeUninit::uninit();
+            // updating the events for the whole app
+            SDL_PumpEvents();
+            if SDL_PollEvent(event.as_mut_ptr()) != 0{
+                return Option::Some(event.assume_init());
+            }
+            return Option::None;
+        }
+    }
 }
 
 impl GfxDevice for SdlGfxDevice{
@@ -87,46 +99,78 @@ impl GfxDevice for SdlGfxDevice{
     }
 }
 
-cfg_if::cfg_if!{ if #[cfg(feature = "dbg")]{
-    pub struct PpuLayerWindow{
-        _window_name: CString,
-        renderer: *mut SDL_Renderer,
-        texture: *mut SDL_Texture,
+#[cfg(feature = "dbg")]
+pub struct PpuLayerWindow{
+    _window_name: CString,
+    window: *mut SDL_Window,
+    renderer: *mut SDL_Renderer,
+    texture: *mut SDL_Texture,
+}
+
+#[cfg(feature = "dbg")]
+impl PpuLayerWindow{
+    pub fn new(layer:magenboy_core::debugger::PpuLayer)->Self{
+        use magenboy_core::debugger::PpuLayer;
+
+        let layer_name = match layer{
+            PpuLayer::Background => "Background",
+            PpuLayer::Window => "Window",
+            PpuLayer::Sprites => "Sprites"
+        };
+        let name = std::format!("Ppu {} debugger", layer_name);
+        let c_name = CString::new(name).unwrap();
+        unsafe{
+            let window:*mut SDL_Window = SDL_CreateWindow(
+                c_name.as_ptr(),
+                SDL_WINDOWPOS_UNDEFINED_MASK as i32, SDL_WINDOWPOS_UNDEFINED_MASK as i32,
+                0x100, 0x100, 0);
+            let renderer: *mut SDL_Renderer = SDL_CreateRenderer(window, -1, 0);
+            let texture: *mut SDL_Texture = SDL_CreateTexture(renderer, SDL_PIXEL_FORMAT,
+                SDL_TextureAccess::SDL_TEXTUREACCESS_STREAMING as i32, 0x100, 0x100);
+
+            return PpuLayerWindow { _window_name: c_name, window, renderer, texture};
+        }
     }
 
-    impl PpuLayerWindow{
-        pub fn new(layer:PpuLayer)->Self{
-            let layer_name = match layer{
-                PpuLayer::Background => "Background",
-                PpuLayer::Window => "Window",
-                PpuLayer::Sprites => "Sprites"
-            };
-            let name = std::format!("Ppu {} debugger", layer_name);
-            let c_name = CString::new(name).unwrap();
-            unsafe{
-                let window:*mut SDL_Window = SDL_CreateWindow(
-                    c_name.as_ptr(),
-                    SDL_WINDOWPOS_UNDEFINED_MASK as i32, SDL_WINDOWPOS_UNDEFINED_MASK as i32,
-                    0x100, 0x100, 0);
-                let renderer: *mut SDL_Renderer = SDL_CreateRenderer(window, -1, 0);
-                let texture: *mut SDL_Texture = SDL_CreateTexture(renderer, SDL_PIXEL_FORMAT,
-                    SDL_TextureAccess::SDL_TEXTUREACCESS_STREAMING as i32, 0x100, 0x100);
-
-                return PpuLayerWindow { _window_name: c_name, renderer, texture};
-            }
+
+
+    pub fn run(&mut self, buffer:[Pixel;magenboy_core::debugger::PPU_BUFFER_SIZE]){
+        unsafe{
+            let mut event: std::mem::MaybeUninit<SDL_Event> = std::mem::MaybeUninit::uninit();
+            self.render(buffer);
+            loop{
+                SDL_PumpEvents();
+                if SDL_PollEvent(event.as_mut_ptr()) != 0{
+                    let event: SDL_Event = event.assume_init();
+                    if event.type_ == SDL_EventType::SDL_WINDOWEVENT as u32 && event.window.event == SDL_WindowEventID::SDL_WINDOWEVENT_CLOSE as u8{
+                        break;
+                    }
+                }
+            }   
         }
+    }
+
+    fn render(&mut self, buffer:[Pixel;magenboy_core::debugger::PPU_BUFFER_SIZE]){
+        unsafe{
+            let mut pixels: *mut c_void = std::ptr::null_mut();
+            let mut length: std::os::raw::c_int = 0;
+            SDL_LockTexture(self.texture, std::ptr::null(), &mut pixels, &mut length);
+            std::ptr::copy_nonoverlapping(buffer.as_ptr(),pixels as *mut Pixel,  buffer.len());
+            SDL_UnlockTexture(self.texture);
 
-        pub fn render(&mut self, buffer:[Pixel;0x100*0x100]){
-            unsafe{
-                let mut pixels: *mut c_void = std::ptr::null_mut();
-                let mut length: std::os::raw::c_int = 0;
-                SDL_LockTexture(self.texture, std::ptr::null(), &mut pixels, &mut length);
-                std::ptr::copy_nonoverlapping(buffer.as_ptr(),pixels as *mut Pixel,  buffer.len());
-                SDL_UnlockTexture(self.texture);
+            SDL_RenderCopy(self.renderer, self.texture, std::ptr::null(), std::ptr::null());
+            SDL_RenderPresent(self.renderer);
+        }
+    }
+}
 
-                SDL_RenderCopy(self.renderer, self.texture, std::ptr::null(), std::ptr::null());
-                SDL_RenderPresent(self.renderer);
-            }
+#[cfg(feature = "dbg")]
+impl Drop for PpuLayerWindow{
+    fn drop(&mut self) {
+        unsafe{
+            SDL_DestroyTexture(self.texture);
+            SDL_DestroyRenderer(self.renderer);
+            SDL_DestroyWindow(self.window);
         }
     }
-}}
+}

sdl/src/sdl/sdl_joypad_provider.rs

@@ -1,55 +1,49 @@
 use sdl2::sys::*;
-use magenboy_core::{keypad::{joypad::{Joypad, NUM_OF_KEYS}, joypad_provider::JoypadProvider, button::Button}, utils::create_array};
+use magenboy_core::keypad::{joypad::{Joypad, NUM_OF_KEYS}, joypad_provider::JoypadProvider};
 use magenboy_common::joypad_menu::MenuJoypadProvider;
 use super::utils::get_sdl_error_message;
 
 
 pub struct SdlJoypadProvider{
-    keyborad_state: [*const u8;NUM_OF_KEYS],
+    mapping: [SDL_Scancode; NUM_OF_KEYS]
 }
 
 impl SdlJoypadProvider{
-    pub fn new<F:Fn(&Button)->SDL_Scancode>(mapper:F)->Self{
-        let keyboard_ptr = unsafe{SDL_GetKeyboardState(std::ptr::null_mut())};
-        let mut counter:u8 = 0;
-        let init_lambda = ||{
-            let button:Button = unsafe{std::mem::transmute(counter)};
-            let result = unsafe{keyboard_ptr.offset(mapper(&button) as isize)};
-            counter += 1;
-            return result;
-        };
-        let state:[*const u8; NUM_OF_KEYS] = create_array(init_lambda);
-
-        return Self{keyborad_state: state}
+    pub fn new(mapping: [SDL_Scancode; NUM_OF_KEYS])->Self{
+        Self{mapping}
     }
 }
 
 impl JoypadProvider for SdlJoypadProvider{
+    // Events are pumped from the main thread (the thread that initializes SDL)
+    // Its unsound to pump them from other threads
     fn provide(&mut self, joypad:&mut Joypad) {
         unsafe{
-            SDL_PumpEvents();
-
+            let state = SDL_GetKeyboardState(std::ptr::null_mut());
             for i in 0..NUM_OF_KEYS{
-                joypad.buttons[i] = *self.keyborad_state[i] != 0;
+                joypad.buttons[i] = *state.add(self.mapping[i] as usize) != 0;
             }
         }
     }
 }
 
 impl MenuJoypadProvider for SdlJoypadProvider{
-    fn poll(&mut self, mut joypad:&mut Joypad) {
+    fn poll(&mut self, joypad:&mut Joypad) {
+        joypad.buttons.fill(false);
         unsafe{
+            let mut event = std::mem::MaybeUninit::<SDL_Event>::uninit();
             loop{
-                let mut event = std::mem::MaybeUninit::<SDL_Event>::uninit();
                 if SDL_WaitEvent(event.as_mut_ptr()) == 0{
                     std::panic!("SDL_Error: {}", get_sdl_error_message());
                 }
                 let event = event.assume_init();
-                if event.type_ == SDL_EventType::SDL_KEYDOWN as u32 || event.type_ == SDL_EventType::SDL_KEYUP as u32 {
+                if event.type_ == SDL_EventType::SDL_KEYUP as u32{
+                    if let Some(index) = self.mapping.iter().position(|key|*key == event.key.keysym.scancode){
+                        joypad.buttons[index] = true;
+                    }
                     break;
                 }
             }
         }
-        self.provide(&mut joypad);
     }
 }