Fix libretro frontend bug (#147)

The problem is the core outputs the data as RGB555 and libretro will expect RGB565 (it can accept ARGB1555 but it is not recommended)

This involved some more fixes as Im really confused between RGB and BGR I hope Im correct with the comments and names but im not sure.

Now Pixel is a RGB565 since this format is more popular.

Also fixed some typos

.gitignore

@@ -6,7 +6,7 @@ Dependencies/
 *.wav
 *.bmp
 *.img
-config.txt
+*.txt
 
 ## Ignore Visual Studio temporary files, build results, and
 ## files generated by popular Visual Studio add-ons.

common/src/initialization.rs

@@ -60,7 +60,7 @@ pub fn init_and_run_gameboy(
                 mode
             }
             else{
-                let mode = mbc.detect_prefered_mode();
+                let mode = mbc.detect_preferred_mode();
                 log::info!("Could not find a mode flag, auto detected {}", <Mode as Into<&str>>::into(mode));
                 mode
             };

core/src/machine/mod.rs

@@ -35,7 +35,7 @@ impl From<Mode> for &str{
 // for some reason the lifetime is important here for the
 // compiler to accept this call on any Mbc lifetine and not just 'static
 impl<'a> dyn Mbc + 'a{
-    pub fn detect_prefered_mode(&self)->Mode{
+    pub fn detect_preferred_mode(&self)->Mode{
         let cart_compatibility_reg = self.read_bank0(CGB_FLAG_ADDRESS as u16);
         return if cart_compatibility_reg & BIT_7_MASK == 0 {Mode::DMG} else{Mode::CGB};
     }

core/src/mmu/gb_mmu.rs

@@ -1,5 +1,5 @@
 use super::{access_bus::AccessBus, carts::{Mbc, CGB_FLAG_ADDRESS}, external_memory_bus::{Bootrom, ExternalMemoryBus}, interrupts_handler::InterruptRequest, io_bus::IoBus, Memory};
-use crate::{apu::{audio_device::AudioDevice, gb_apu::GbApu}, keypad::joypad_provider::JoypadProvider, machine::Mode, ppu::{color::Color, gfx_device::GfxDevice, ppu_state::PpuState}, utils::{bit_masks::{flip_bit_u8, BIT_7_MASK}, memory_registers::*}, Pixel};
+use crate::{apu::{audio_device::AudioDevice, gb_apu::GbApu}, keypad::joypad_provider::JoypadProvider, machine::Mode, ppu::{color::Color, gfx_device::GfxDevice, ppu_state::PpuState}, utils::{bit_masks::{flip_bit_u8, BIT_7_MASK}, memory_registers::*}};
 
 const HRAM_SIZE:usize = 0x7F;
 
@@ -177,17 +177,18 @@ impl<'a, D:AudioDevice, G:GfxDevice, J:JoypadProvider> GbMmu<'a, D, G, J>{
                     mmu.write(KEY0_REGISTER_ADDRESS, cgb_reg, 0);
                 }
                 else{
-                    mmu.write(KEY0_REGISTER_ADDRESS, 0x4, 0);   // Set bit 2 that indicates DMG compatability mode 
+                    mmu.write(KEY0_REGISTER_ADDRESS, 0x4, 0);   // Set bit 2 that indicates DMG compatibility mode 
                     mmu.write(OPRI_REGISTER_ADDRESS, 1, 0);     // Set DMG priority mode
-
-                    // Setup the default BG palletes
+
+                    // Default colors are from here - https://tcrf.net/Notes:Game_Boy_Color_Bootstrap_ROM
+                    // Setup the default BG palettes
                     mmu.write(BGPI_REGISTER_ADDRESS, BIT_7_MASK, 0);    // Set to auto increment
                     mmu.write_color_ram(BGPD_REGISTER_ADDRESS, Color::from(0xFFFFFF as u32));
                     mmu.write_color_ram(BGPD_REGISTER_ADDRESS, Color::from(0x7BFF31 as u32));
                     mmu.write_color_ram(BGPD_REGISTER_ADDRESS, Color::from(0x0063C5 as u32));
                     mmu.write_color_ram(BGPD_REGISTER_ADDRESS, Color::from(0x000000 as u32));
 
-                    // Setup the default OBJ palletes
+                    // Setup the default OBJ palettes
                     mmu.write(OBPI_REGISTER_ADDRESS, BIT_7_MASK, 0);    // Set to auto increment
                     // Fill the first 2 object palettes with the same palette
                     for _ in 0..2{
@@ -251,10 +252,10 @@ impl<'a, D:AudioDevice, G:GfxDevice, J:JoypadProvider> GbMmu<'a, D, G, J>{
     }
 
     fn write_color_ram(&mut self, address:u16, color: Color){
-        let pixel:Pixel = color.into();
+        let bgr555_value = ((color.r >> 3) as u16 & 0b1_1111) | (((color.g >> 3) as u16 & 0b1_1111) << 5)  | (((color.b >> 3) as u16 & 0b1_1111) << 10);
 
         // The value is little endian in memory so writing the low bits first and then the high bits
-        self.write(address, (pixel & 0xFF) as u8, 0);
-        self.write(address, ((pixel >> 8) & 0xFF) as u8, 0);
+        self.write(address, (bgr555_value & 0xFF) as u8, 0);
+        self.write(address, ((bgr555_value >> 8) & 0xFF) as u8, 0);
     }
 }

core/src/ppu/color.rs

@@ -14,11 +14,12 @@ pub struct Color{
 
 impl From<Color> for Pixel{
     #[inline]
-    fn from(color: Color) -> Self {(((color.b >> 3) as u16) << 10) | (((color.g >> 3) as u16) << 5) | ((color.r >> 3) as u16)}
+    // Notice that Pixel is RGB565, for more see Pixel doc
+    fn from(color: Color) -> Self {(((color.r >> 3) as u16) << 11) | (((color.g >> 2) as u16) << 5) | ((color.b >> 3) as u16)}
 }
 
 impl From<u16> for Color{
-    /// color is BGR555 u16 value
+    /// color is BGR555 u16 value (Red - low bits, Blue - High bits)
     fn from(color:u16)->Color{
         Color{
             r: ((color & 0b1_1111) as u8) << 3, 
@@ -29,7 +30,7 @@ impl From<u16> for Color{
 }
 
 impl From<u32> for Color{
-    /// Color is RGB RGB888 value where Red is the high bits and Blue the low bits
+    /// Color is RGB888 value (Red - high bits, Blue - low bits)
     fn from(color: u32) -> Self {
         Self{ r: ((color >> 16) & 0xFF) as u8, g: ((color >> 8) & 0xFF) as u8, b: (color & 0xFF) as u8 }
     }

core/src/ppu/gfx_device.rs

@@ -1,6 +1,7 @@
 use super::gb_ppu::{SCREEN_HEIGHT, SCREEN_WIDTH};
 
-/// Pixel is in the format of RGB555 as in the gbdev docs which is low bits (Red) -> high bits (Blue)
+/// Pixel is in the format of RGB565 even though the CGB stores pixels as BGR555 as the gbdev docs indicates, RGB565 is much more used format now days
+/// The bits are represented as: RGB565 (low bits (BLue) -> high bits (Red))
 pub type Pixel = u16;
 
 pub trait GfxDevice{

core/tests/integration_tests.rs

@@ -1,6 +1,6 @@
 use std::{collections::hash_map::DefaultHasher, convert::TryInto, hash::{Hash, Hasher}, io::Read, sync::atomic::AtomicBool};
 
-use magenboy_core::{keypad::{joypad::Joypad, joypad_provider::JoypadProvider}, machine::{Mode, gameboy::GameBoy, mbc_initializer::initialize_mbc}, mmu::{external_memory_bus::Bootrom, carts::Mbc}, ppu::{gb_ppu::{SCREEN_HEIGHT, SCREEN_WIDTH},color::*, gfx_device::*}, apu::audio_device::*};
+use magenboy_core::{keypad::{joypad::Joypad, joypad_provider::JoypadProvider}, machine::{Mode, gameboy::GameBoy, mbc_initializer::initialize_mbc}, mmu::{external_memory_bus::Bootrom, carts::Mbc}, ppu::{gb_ppu::{SCREEN_HEIGHT, SCREEN_WIDTH}, gfx_device::*}, apu::audio_device::*};
 
 struct CheckHashGfxDevice<'a>{
     hash: u64,
@@ -32,85 +32,85 @@ impl JoypadProvider for StubJoypadProvider{
 #[test]
 fn test_cpu_instrs(){
     let file_url = "https://raw.githubusercontent.com/retrio/gb-test-roms/master/cpu_instrs/cpu_instrs.gb";
-    run_integration_test_from_url(file_url, 3200, 8045249190936210527, Some(Mode::DMG));
+    run_integration_test_from_url(file_url, 3200, 15560803699908721371, Some(Mode::DMG));
 }
 
 #[test]
 fn test_cpu_instrs_timing(){
     let file_url = "https://raw.githubusercontent.com/retrio/gb-test-roms/master/instr_timing/instr_timing.gb";
-    run_integration_test_from_url(file_url, 100, 5992502146430994882, Some(Mode::DMG));
+    run_integration_test_from_url(file_url, 100, 6688493151528556732, Some(Mode::DMG));
 }
 
 #[test]
 fn test_dmg_acid_dmg_mode(){
     let file_url = "https://github.com/mattcurrie/dmg-acid2/releases/download/v1.0/dmg-acid2.gb";
-    run_integration_test_from_url(file_url, 60, 14652376974750987946, Some(Mode::DMG));
+    run_integration_test_from_url(file_url, 60, 1467713036241655344, Some(Mode::DMG));
 }
 
 #[test]
 fn test_dmg_acid_cgb_mode(){
     let file_url = "https://github.com/mattcurrie/dmg-acid2/releases/download/v1.0/dmg-acid2.gb";
-    run_integration_test_from_url(file_url, 60, 18113135055643582129, Some(Mode::CGB));
+    run_integration_test_from_url(file_url, 60, 18025850858500536480, Some(Mode::CGB));
 }
 
 #[test]
 fn test_turtle_window_y_trigger(){
-    run_turtle_integration_test("window_y_trigger.gb", 7485875088720750776);
+    run_turtle_integration_test("window_y_trigger.gb", 6465875958237578550);
 }
 
 #[test]
 fn test_turtle_window_y_trigger_wx_offscreen(){
-    run_turtle_integration_test("window_y_trigger_wx_offscreen.gb", 141491602507137088);
+    run_turtle_integration_test("window_y_trigger_wx_offscreen.gb", 18187429968502985545);
 }
 
 #[test]
 fn test_mooneye_acceptance_ppu_intr_2_0_timing(){
-    run_mooneye_test_suite_test("acceptance/ppu/intr_2_0_timing.gb", 11247474121545571329);
+    run_mooneye_test_suite_test("acceptance/ppu/intr_2_0_timing.gb", 7475320393161591745);
 }
 
 #[test]
 fn test_mooneye_acceptance_ppu_intr_2_mode0_timing(){
-    run_mooneye_test_suite_test("acceptance/ppu/intr_2_mode0_timing.gb", 6031731779359060542);
+    run_mooneye_test_suite_test("acceptance/ppu/intr_2_mode0_timing.gb", 9052326526940620337);
 }
 
 #[test]
 fn test_mooneye_acceptance_ppu_intr_2_mode3_timing(){
-    run_mooneye_test_suite_test("acceptance/ppu/intr_2_mode3_timing.gb", 656080116664351641);
+    run_mooneye_test_suite_test("acceptance/ppu/intr_2_mode3_timing.gb", 14127472135696903085);
 }
 
 #[test]
 fn test_mooneye_acceptance_ppu_intr_2_oam_ok_timing(){
-    run_mooneye_test_suite_test("acceptance/ppu/intr_2_oam_ok_timing.gb", 1944677356369736172);
+    run_mooneye_test_suite_test("acceptance/ppu/intr_2_oam_ok_timing.gb", 14374012711624871933);
 }
 
 #[test]
 fn test_magentests_bg_oam_priority(){
     let file_url = "https://github.com/alloncm/MagenTests/releases/download/0.3.0/bg_oam_priority.gbc";
-    run_integration_test_from_url(file_url, 60, 6516853904884538463, Some(Mode::CGB));
+    run_integration_test_from_url(file_url, 60, 10888561623649800478, Some(Mode::CGB));
 }
 
 #[test]
 fn test_magentests_oam_internal_priority(){
     let file_url = "https://github.com/alloncm/MagenTests/releases/download/0.2.0/oam_internal_priority.gbc";
-    run_integration_test_from_url(file_url, 60, 6761868656389238011, Some(Mode::CGB));
+    run_integration_test_from_url(file_url, 60, 3314422793898507891, Some(Mode::CGB));
 }
 
 #[test]
 fn test_magentests_hblank_vram_dma(){
     let file_url = "https://github.com/alloncm/MagenTests/releases/download/0.3.0/hblank_vram_dma.gbc";
-    run_integration_test_from_url(file_url, 60, 2706871350915036708, Some(Mode::CGB));
+    run_integration_test_from_url(file_url, 60, 6410113756445583331, Some(Mode::CGB));
 }
 
 #[test]
 fn test_magentests_key0_lock_after_boot(){
     let file_url = "https://github.com/alloncm/MagenTests/releases/download/0.4.0/key0_lock_after_boot.gbc";
-    run_integration_test_from_url(file_url, 60, 2706871350915036708, Some(Mode::CGB));
+    run_integration_test_from_url(file_url, 60, 6410113756445583331, Some(Mode::CGB));
 }
 
 #[test]
 fn test_cgb_acid2(){
     let file_url = "https://github.com/mattcurrie/cgb-acid2/releases/download/v1.1/cgb-acid2.gbc";
-    run_integration_test_from_url(file_url, 60, 2979852716992493573, Some(Mode::CGB));
+    run_integration_test_from_url(file_url, 60, 1123147979104076695, Some(Mode::CGB));
 }
 
 fn run_turtle_integration_test(program_name:&str, hash:u64){
@@ -208,11 +208,10 @@ fn calc_hash(rom_path:&str, boot_rom_path:Option<&str>, mode:Option<Mode>){
                 self.last_hash_counter += 1;
                 if self.last_hash_counter > 600{
                     std::fs::write("calc_hash_output.txt", hash.to_string().as_bytes()).unwrap();
-                    let buf = buffer
-                        .map(Color::from)
-                        .map(|c|[c.r, c.g, c.b])
+                    let buffer = buffer
+                        .map(|c|[(((c >> 11) & 0b1_1111) as u8) << 3, (((c >> 5) & 0b11_1111) as u8) << 2, (((c) & 0b1_1111) as u8) << 3])
                         .concat();
-                    image::save_buffer("calc_hash_output.bmp", &buf, SCREEN_WIDTH as u32, SCREEN_HEIGHT as u32, image::ColorType::Rgb8).unwrap();
+                    image::save_buffer("calc_hash_output.bmp", &buffer, SCREEN_WIDTH as u32, SCREEN_HEIGHT as u32, image::ColorType::Rgb8).unwrap();
                     std::process::exit(0);
                 }
             }

libretro/src/lib.rs

@@ -86,7 +86,7 @@ pub unsafe extern "C" fn retro_load_game(game_info: *const GameInfo)->bool{
     if mbc.has_battery(){
         RETRO_CORE_CTX.save_data_fat_ptr = Some((mbc.get_ram().as_mut_ptr(), mbc.get_ram().len()));
     }
-    let mode = mbc.detect_prefered_mode();
+    let mode = mbc.detect_preferred_mode();
     RETRO_CORE_CTX.gameboy = Some(GameBoy::new_with_mode(mbc, RetroJoypadProvider, RetroAudioDevice::default(), RetroGfxDevice, mode));
 
     let mut pixel_format = PixelFormat::RGB565.to_uint();

rpi/src/bin/baremetal/main.rs

@@ -47,7 +47,7 @@ pub extern "C" fn main()->!{
     let rom = unsafe{&mut ROM_BUFFER};
     fs.read_file(selected_rom, rom);
     let mbc = initialize_mbc(&rom[0..selected_rom.size as usize], None);
-    let mode = mbc.detect_prefered_mode();
+    let mode = mbc.detect_preferred_mode();
 
     let mut gameboy = GameBoy::new_with_mode(mbc, joypad_provider, magenboy_rpi::BlankAudioDevice, gfx, mode);
     log::info!("Initialized gameboy!");

rpi/src/drivers/ili9341_gfx_device.rs

@@ -96,7 +96,7 @@ impl Ili9341Contoller{
         controller.spi.write(Ili9341Command::VcomControl2 as u8, &[0x86]);
 
         // Configuring the screen
-        controller.spi.write(Ili9341Command::MemoryAccessControl as u8, &[0x20]); // This command tlit the screen 90 degree and set pixel to RGB order (low bits (Red) -> High bits (Blue))
+        controller.spi.write(Ili9341Command::MemoryAccessControl as u8, &[0x28]); // This command tilt the screen 90 degree and set pixel to RGB order (low bits (Blue) -> High bits (Red))
         controller.spi.write(Ili9341Command::PixelFormatSet as u8, &[0x55]);      // set pixel format to 16 bit per pixel;
         controller.spi.write(Ili9341Command::FrameRateControl as u8, &[0x0, 0x10 /*According to the docs this is 119 hrz, setting this option in order to avoid screen tearing on rpi zero2 */]);
         controller.spi.write(Ili9341Command::DisplayFunctionControl as u8, &[0x8, 0x82, 0x27]);

sdl/src/sdl_gfx_device.rs

@@ -3,8 +3,9 @@ use sdl2::sys::*;
 use magenboy_core::{ppu::gb_ppu::{SCREEN_HEIGHT, SCREEN_WIDTH}, utils::vec2::Vec2, GfxDevice, Pixel};
 use super::utils::get_sdl_error_message;
 
-// The bit order is high bits -> low bits as opposed to RGB555 in the gbdev docs which is low -> high 
-const SDL_PIXEL_FORMAT:u32 = SDL_PixelFormatEnum::SDL_PIXELFORMAT_BGR555 as u32;
+// The bit order is high bits -> low bits as opposed to RGB555 in the gbdev docs which is low -> high.
+// Using 565 since Pixel also uses this more convenient format
+const SDL_PIXEL_FORMAT:u32 = SDL_PixelFormatEnum::SDL_PIXELFORMAT_RGB565 as u32;
 
 struct SdlWindow{
     _window_name: CString,