diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index de0765a43f18cec9830c579a1c85de0ed5c7983b..d65e0ff405189f6922b0bb1c4c6633a365f9761d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -9,6 +9,7 @@ variables: stages: - build + - test - deploy before_script: @@ -51,6 +52,17 @@ build-wasm: - frontends/web/lib expire_in: 1 day +test-rust: + stage: test + parallel: + matrix: + - RUST_VERSION: ["1.56.1", "1.60.0", "stable", "nightly"] + script: + - rustup toolchain install $RUST_VERSION + - rustup override set $RUST_VERSION + - rustc --version + - cargo test + deploy-netlify-preview: stage: deploy script: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5db04af3a4674a3e3c38c2da3e2ad22f21d9d643..28b1e7bbf306c4b77b25cecddea4c24f9e9fb705 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -* +* Issue with background color and change of palette colors + +## [0.5.5] - 2022-11-17 + +### Fixed + +* PPU issue related to the maximum number of objects/sprite per line being 10, issue detected by ACID test +* Object pixel drawing priority issue, issue detected by ACID test +* Issue associated with the wrongful flipping of 8x16 sprites, issue detected by ACID test +* Issue associated with drawing of window tiles, due to extra `update_stat()` operations, issue detected by ACID test ## [0.5.4] - 2022-11-15 diff --git a/Cargo.toml b/Cargo.toml index 667af38302611c39241a846a9df6f4fd2bafdca7..446a1dfda761993cc5a0748faf448f8546730239 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "boytacean" description = "A Game Boy emulator that is written in Rust." -version = "0.5.4" +version = "0.5.5" authors = ["João Magalhães <joamag@gmail.com>"] license = "Apache-2.0" repository = "https://github.com/joamag/boytacean" diff --git a/frontends/sdl/Cargo.toml b/frontends/sdl/Cargo.toml index 3eec882c57ef7b4f4934b8eb4c53f48d4f8a161e..8e4ef9dfb8f64d02e7b45c67925535fed50cf396 100644 --- a/frontends/sdl/Cargo.toml +++ b/frontends/sdl/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "boytacean-sdl" -version = "0.5.4" +version = "0.5.5" authors = ["João Magalhães <joamag@gmail.com>"] description = "Game Boy Emulator SDL (Desktop) Application" license = "Apache-2.0" diff --git a/frontends/sdl/src/main.rs b/frontends/sdl/src/main.rs index 09ceb4624783afa792886df78ce06b14e86fbee2..37d804fa85155efb281f83d20976707dc106ff88 100644 --- a/frontends/sdl/src/main.rs +++ b/frontends/sdl/src/main.rs @@ -188,7 +188,7 @@ fn main() { game_boy.load_cgb(true); let mut emulator = Emulator::new(game_boy, SCREEN_SCALE); - emulator.load_rom("../../res/roms.prop/super_mario.gb"); + emulator.load_rom("../../res/roms/dmg_acid2.gb"); emulator.run(); } diff --git a/frontends/web/package.json b/frontends/web/package.json index d71ef832eb76477a47ebc1773f1d53cecdc46f57..b66d8919e42fe2f30611de7442399c193a3c4dcf 100644 --- a/frontends/web/package.json +++ b/frontends/web/package.json @@ -1,6 +1,6 @@ { "name": "boytacean-web", - "version": "0.5.4", + "version": "0.5.5", "description": "The web version of Boytacean", "repository": { "type": "git", diff --git a/res/roms/dmg_acid2.gb b/res/roms/dmg_acid2.gb new file mode 100644 index 0000000000000000000000000000000000000000..a25ef9485e1ab176c80415548f053ca950b3e4f5 Binary files /dev/null and b/res/roms/dmg_acid2.gb differ diff --git a/res/roms/sprite_priority.gb b/res/roms/sprite_priority.gb new file mode 100644 index 0000000000000000000000000000000000000000..0a6a867122ebe235e0ac1b60d3a7ee0a128f6cd7 Binary files /dev/null and b/res/roms/sprite_priority.gb differ diff --git a/src/mmu.rs b/src/mmu.rs index 5d5d1563c28c3d190e4d3909e48c6b69084d793f..66baa63954899b77db43ab8fd29a480235b9ee4e 100644 --- a/src/mmu.rs +++ b/src/mmu.rs @@ -137,7 +137,7 @@ impl Mmu { 0x4000 | 0x5000 | 0x6000 | 0x7000 => self.rom.read(addr), // Graphics: VRAM (8 KB) - 0x8000 | 0x9000 => self.ppu.vram[(addr & 0x1fff) as usize], + 0x8000 | 0x9000 => self.ppu.read(addr), // External RAM (8 KB) 0xa000 | 0xb000 => self.rom.read(addr), @@ -155,7 +155,7 @@ impl Mmu { 0xf000 => match addr & 0x0f00 { 0x000 | 0x100 | 0x200 | 0x300 | 0x400 | 0x500 | 0x600 | 0x700 | 0x800 | 0x900 | 0xa00 | 0xb00 | 0xc00 | 0xd00 => self.ram[(addr & 0x1fff) as usize], - 0xe00 => self.ppu.oam[(addr & 0x009f) as usize], + 0xe00 => self.ppu.read(addr), 0xf00 => match addr & 0x00ff { // 0xFF0F — IF: Interrupt flag 0x0f => { @@ -175,7 +175,7 @@ impl Mmu { } // 0xFF80-0xFFFE - High RAM (HRAM) - 0x80..=0xfe => self.ppu.hram[(addr & 0x007f) as usize], + 0x80..=0xfe => self.ppu.read(addr), // 0xFFFF — IE: Interrupt enable 0xff => self.ie, @@ -216,12 +216,7 @@ impl Mmu { 0x4000 | 0x5000 | 0x6000 | 0x7000 => self.rom.write(addr, value), // Graphics: VRAM (8 KB) - 0x8000 | 0x9000 => { - self.ppu.vram[(addr & 0x1fff) as usize] = value; - if addr < 0x9800 { - self.ppu.update_tile(addr, value); - } - } + 0x8000 | 0x9000 => self.ppu.write(addr, value), // External RAM (8 KB) 0xa000 | 0xb000 => self.rom.write(addr, value), @@ -238,10 +233,7 @@ impl Mmu { | 0xa00 | 0xb00 | 0xc00 | 0xd00 => { self.ram[(addr & 0x1fff) as usize] = value; } - 0xe00 => { - self.ppu.oam[(addr & 0x009f) as usize] = value; - self.ppu.update_object(addr, value); - } + 0xe00 => self.ppu.write(addr, value), 0xf00 => match addr & 0x00ff { // 0xFF0F — IF: Interrupt flag 0x0f => { @@ -265,7 +257,7 @@ impl Mmu { } // 0xFF80-0xFFFE - High RAM (HRAM) - 0x80..=0xfe => self.ppu.hram[(addr & 0x007f) as usize] = value, + 0x80..=0xfe => self.ppu.write(addr, value), // 0xFFFF — IE: Interrupt enable 0xff => self.ie = value, diff --git a/src/ppu.rs b/src/ppu.rs index ec68a8b36510cc45845daf2e6bc243cff35da322..d6604967cbff40daaa403b662de515c6fe2bf871 100644 --- a/src/ppu.rs +++ b/src/ppu.rs @@ -112,7 +112,7 @@ pub struct ObjectData { palette: u8, xflip: bool, yflip: bool, - priority: bool, + bg_over: bool, index: u8, } @@ -143,8 +143,9 @@ pub struct PpuRegisters { /// /// # Basic usage /// ```rust -/// let ppu = Ppu::new(); -/// ppu.clock(); +/// use boytacean::ppu::Ppu; +/// let mut ppu = Ppu::new(); +/// ppu.clock(8); /// ``` pub struct Ppu { /// The color buffer that is going to store the colors @@ -213,7 +214,7 @@ pub struct Ppu { /// The current scan line in processing, should /// range between 0 (0x00) and 153 (0x99), representing - /// the 154 lines plus 10 extra v-blank lines. + /// the 154 lines plus 10 extra V-Blank lines. ly: u8, /// The line compare register that is going to be used @@ -257,6 +258,13 @@ pub struct Ppu { /// content. switch_lcd: bool, + // Internal window counter value used to control the ines that + // were effectively rendered as part of the window tile drawing process. + // A line is only considered rendered when the WX and WY registers + // are within the valid screen range and the window switch register + // is valid. + window_counter: u8, + /// Flag that controls if the frame currently in rendering is the /// first one, preventing actions. first_frame: bool, @@ -305,7 +313,7 @@ impl Ppu { palette: 0, xflip: false, yflip: false, - priority: false, + bg_over: false, index: 0, }; OBJ_COUNT], palette_colors: PALETTE_COLORS, @@ -329,6 +337,7 @@ impl Ppu { switch_window: false, window_map: false, switch_lcd: false, + window_counter: 0x0, first_frame: false, frame_index: 0, stat_hblank: false, @@ -390,7 +399,6 @@ impl Ppu { if self.mode_clock >= 80 { self.mode = PpuMode::VramRead; self.mode_clock -= 80; - self.update_stat() } } PpuMode::VramRead => { @@ -399,17 +407,28 @@ impl Ppu { self.mode = PpuMode::HBlank; self.mode_clock -= 172; - self.update_stat() } } PpuMode::HBlank => { if self.mode_clock >= 204 { + // increments the window counter making sure that the + // valid is only incremented when both the WX and WY + // registers make sense (are within range), the window + // switch is on and the line in drawing is above WY + if self.switch_window + && self.wx - 7 < DISPLAY_WIDTH as u8 + && self.wy < DISPLAY_HEIGHT as u8 + && self.ly >= self.wy + { + self.window_counter += 1; + } + // increments the register that holds the // information about the current line in drawing self.ly += 1; // in case we've reached the end of the - // screen we're now entering the v-blank + // screen we're now entering the V-Blank if self.ly == 144 { self.int_vblank = true; self.mode = PpuMode::VBlank; @@ -423,28 +442,34 @@ impl Ppu { } PpuMode::VBlank => { if self.mode_clock >= 456 { + // increments the register that controls the line count, + // notice that these represent the extra 10 horizontal + // scanlines that are virtual and not real (off-screen) self.ly += 1; - // in case the end of v-blank has been reached then + // in case the end of V-Blank has been reached then // we must jump again to the OAM read mode and reset // the scan line counter to the zero value if self.ly == 154 { self.mode = PpuMode::OamRead; self.ly = 0; + self.window_counter = 0; self.first_frame = false; self.frame_index = self.frame_index.wrapping_add(1); } self.mode_clock -= 456; - self.update_stat() } } } } pub fn read(&mut self, addr: u16) -> u8 { - match addr & 0x00ff { - 0x0040 => { + match addr { + 0x8000..=0x97ff => self.vram[(addr & 0x1fff) as usize], + 0xfe00..=0xfe9f => self.oam[(addr & 0x009f) as usize], + 0xff80..=0xfffe => self.hram[(addr & 0x007f) as usize], + 0xff40 => { (if self.switch_bg { 0x01 } else { 0x00 } | if self.switch_obj { 0x02 } else { 0x00 } | if self.obj_size { 0x04 } else { 0x00 } @@ -454,7 +479,7 @@ impl Ppu { | if self.window_map { 0x40 } else { 0x00 } | if self.switch_lcd { 0x80 } else { 0x00 }) } - 0x0041 => { + 0xff41 => { (if self.stat_hblank { 0x08 } else { 0x00 } | if self.stat_vblank { 0x10 } else { 0x00 } | if self.stat_oam { 0x20 } else { 0x00 } @@ -462,17 +487,17 @@ impl Ppu { | if self.lyc == self.ly { 0x04 } else { 0x00 } | (self.mode as u8 & 0x03)) } - 0x0042 => self.scy, - 0x0043 => self.scx, - 0x0044 => self.ly, - 0x0045 => self.lyc, - 0x0047 => self.palettes[0], - 0x0048 => self.palettes[1], - 0x0049 => self.palettes[2], - 0x004a => self.wy, - 0x004b => self.wx, - // VBK - CGB Mode Only - 0x004f => 0xff, + 0xff42 => self.scy, + 0xff43 => self.scx, + 0xff44 => self.ly, + 0xff45 => self.lyc, + 0xff47 => self.palettes[0], + 0xff48 => self.palettes[1], + 0xff49 => self.palettes[2], + 0xff4a => self.wy, + 0xff4b => self.wx, + // 0xFF4F — VBK (CGB only) + 0xff4f => 0xff, _ => { warnln!("Reading from unknown PPU location 0x{:04x}", addr); 0xff @@ -481,8 +506,19 @@ impl Ppu { } pub fn write(&mut self, addr: u16, value: u8) { - match addr & 0x00ff { - 0x0040 => { + match addr { + 0x8000..=0x9fff => { + self.vram[(addr & 0x1fff) as usize] = value; + if addr < 0x9800 { + self.update_tile(addr, value); + } + } + 0xfe00..=0xfe9f => { + self.oam[(addr & 0x009f) as usize] = value; + self.update_object(addr, value); + } + 0xff80..=0xfffe => self.hram[(addr & 0x007f) as usize] = value, + 0xff40 => { self.switch_bg = value & 0x01 == 0x01; self.switch_obj = value & 0x02 == 0x02; self.obj_size = value & 0x04 == 0x04; @@ -505,32 +541,32 @@ impl Ppu { self.clear_frame_buffer(); } } - 0x0041 => { + 0xff41 => { self.stat_hblank = value & 0x08 == 0x08; self.stat_vblank = value & 0x10 == 0x10; self.stat_oam = value & 0x20 == 0x20; self.stat_lyc = value & 0x40 == 0x40; } - 0x0042 => self.scy = value, - 0x0043 => self.scx = value, - 0x0045 => self.lyc = value, - 0x0047 => { + 0xff42 => self.scy = value, + 0xff43 => self.scx = value, + 0xff45 => self.lyc = value, + 0xff47 => { Self::compute_palette(&mut self.palette, &self.palette_colors, value); self.palettes[0] = value; } - 0x0048 => { + 0xff48 => { Self::compute_palette(&mut self.palette_obj_0, &self.palette_colors, value); self.palettes[1] = value; } - 0x0049 => { + 0xff49 => { Self::compute_palette(&mut self.palette_obj_1, &self.palette_colors, value); self.palettes[2] = value; } - 0x004a => self.wy = value, - 0x004b => self.wx = value, - // VBK - CGB Mode Only - 0x004f => (), - 0x007f => (), + 0xff4a => self.wy = value, + 0xff4b => self.wx = value, + // 0xFF4F — VBK (CGB only) + 0xff4f => (), + 0xff7f => (), _ => warnln!("Writing in unknown PPU location 0x{:04x}", addr), } } @@ -615,7 +651,7 @@ impl Ppu { /// Clears the current frame buffer, setting the background color /// for all the pixels in the frame buffer. pub fn clear_frame_buffer(&mut self) { - self.fill_frame_buffer([255, 255, 255]); + self.fill_frame_buffer(self.palette_colors[0]); } /// Prints the tile data information to the stdout, this is @@ -627,7 +663,7 @@ impl Ppu { /// Updates the tile structure with the value that has /// just been written to a location on the VRAM associated /// with tiles. - pub fn update_tile(&mut self, addr: u16, _value: u8) { + fn update_tile(&mut self, addr: u16, _value: u8) { let addr = (addr & 0x1ffe) as usize; let tile_index = ((addr >> 4) & 0x01ff) as usize; let tile = self.tiles[tile_index].borrow_mut(); @@ -650,7 +686,7 @@ impl Ppu { } } - pub fn update_object(&mut self, addr: u16, value: u8) { + fn update_object(&mut self, addr: u16, value: u8) { let addr = (addr & 0x01ff) as usize; let obj_index = addr >> 2; if obj_index >= OBJ_COUNT { @@ -662,10 +698,10 @@ impl Ppu { 0x01 => obj.x = value as i16 - 8, 0x02 => obj.tile = value, 0x03 => { - obj.palette = if value & 0x10 == 0x10 { 1 } else { 0 }; + obj.palette = (value & 0x10 == 0x10) as u8; obj.xflip = value & 0x20 == 0x20; obj.yflip = value & 0x40 == 0x40; - obj.priority = value & 0x80 != 0x80; + obj.bg_over = value & 0x80 == 0x80; obj.index = obj_index as u8; } _ => (), @@ -688,28 +724,23 @@ impl Ppu { return; } if self.switch_bg { - self.render_map(self.bg_map, self.scx, self.scy, 0, 0); + self.render_map(self.bg_map, self.scx, self.scy, 0, 0, self.ly); } if self.switch_window { - self.render_map(self.window_map, 0, 0, self.wx, self.wy); + self.render_map(self.window_map, 0, 0, self.wx, self.wy, self.window_counter); } if self.switch_obj { self.render_objects(); } } - fn render_map(&mut self, map: bool, scx: u8, scy: u8, wx: u8, wy: u8) { + fn render_map(&mut self, map: bool, scx: u8, scy: u8, wx: u8, wy: u8, ld: u8) { // in case the target window Y position has not yet been reached // then there's nothing to be done, returns control flow immediately if self.ly < wy { return; } - // calculates the LD (line delta) useful for draw operations where the window - // is repositioned, as the current line for tile calculus is different from - // the one currently in drawing - let ld = self.ly - wy; - // calculates the row offset for the tile by using the current line // index and the DY (scroll Y) divided by 8 (as the tiles are 8x8 pixels), // on top of that ensures that the result is modulus 32 meaning that the @@ -729,7 +760,7 @@ impl Ppu { let mut line_offset: usize = (scx >> 3) as usize; // calculates the index of the initial tile in drawing, - // if the tile data set in use is #1, the indices are + // if the tile data set in use is #1, the indexes are // signed, then calculates a real tile offset let mut tile_index = self.vram[map_offset + line_offset] as usize; if !self.bg_tile && tile_index < 128 { @@ -792,7 +823,7 @@ impl Ppu { } // increments the color offset by one, representing - // the drawing og one pixel + // the drawing of one pixel color_offset += 1; // increments the offset of the frame buffer by the @@ -802,7 +833,21 @@ impl Ppu { } fn render_objects(&mut self) { + let mut draw_count = 0u8; + + // allocates the buffer that is going to be used to determine + // drawing priority for overlapping pixels between different + // objects, in MBR mode the object that has the smallest X + // coordinate takes priority in drawing the pixel + let mut index_buffer = [-256i16; DISPLAY_WIDTH]; + for index in 0..OBJ_COUNT { + // in case the limit on number of object to be draw per + // line has been reached breaks the loop avoiding more draws + if draw_count == 10 { + break; + } + // obtains the meta data of the object that is currently // under iteration to be checked for drawing let obj = self.obj_data[index]; @@ -843,11 +888,17 @@ impl Ppu { // objects and from 0 to 15 in 8x16 objects let mut tile_offset = self.ly as i16 - obj.y; + // in case we're flipping the object we must recompute the + // tile offset as an inverted value using the object's height + if obj.yflip { + tile_offset = obj_height as i16 - tile_offset - 1; + } + // saves some space for the reference to the tile that // is going to be used in the current operation let tile: &Tile; - // in case we're facing a 8x16 object and we must + // in case we're facing a 8x16 object then we must // differentiate between the handling of the top tile // and the bottom tile through bitwise manipulation // of the tile index @@ -865,25 +916,40 @@ impl Ppu { tile = &self.tiles[obj.tile as usize]; } - let tile_row = if obj.yflip { - tile.get_row((7 - tile_offset) as usize) - } else { - tile.get_row((tile_offset) as usize) - }; + let tile_row = tile.get_row(tile_offset as usize); - for x in 0..TILE_WIDTH { - let is_contained = - (obj.x + x as i16 >= 0) && ((obj.x + x as i16) < DISPLAY_WIDTH as i16); + for tile_x in 0..TILE_WIDTH { + let x = obj.x + tile_x as i16; + let is_contained = (x >= 0) && (x < DISPLAY_WIDTH as i16); if is_contained { - // the object is only considered visible if it's a priority - // or if the underlying pixel is transparent (zero value) - let is_visible = obj.priority || self.color_buffer[color_offset as usize] == 0; - let pixel = tile_row[if obj.xflip { 7 - x } else { x }]; - if is_visible && pixel != 0 { + // the object is only considered visible if no background or + // window should be drawn over or if the underlying pixel + // is transparent (zero value) meaning there's no background + // or window for the provided pixel + let is_visible = !obj.bg_over || self.color_buffer[color_offset as usize] == 0; + + // determines if the current pixel has priority over a possible + // one that has been drawn by a previous object, this happens + // in case the current object has a small X coordinate according + // to the MBR algorithm + let has_priority = + index_buffer[x as usize] == -256 || obj.x < index_buffer[x as usize]; + + let pixel = tile_row[if obj.xflip { 7 - tile_x } else { tile_x }]; + if is_visible && has_priority && pixel != 0 { + // marks the current pixel in iteration as "owned" + // by the object with the defined X base position, + // to be used in priority calculus + index_buffer[x as usize] = obj.x; + // obtains the current pixel data from the tile row and // re-maps it according to the object palette let color = palette[pixel as usize]; + // updates the pixel in the color buffer, which stores + // the raw pixel color information (unmapped) + self.color_buffer[color_offset as usize] = pixel; + // sets the color pixel in the frame buffer self.frame_buffer[frame_offset as usize] = color[0]; self.frame_buffer[frame_offset as usize + 1] = color[1]; @@ -899,6 +965,10 @@ impl Ppu { // size of an RGB pixel (which is 3 bytes) frame_offset += RGB_SIZE as i32; } + + // increments the counter so that we're able to keep + // track on the number of object drawn + draw_count += 1; } } @@ -922,6 +992,8 @@ impl Ppu { /// is useful to "flush" color computation whenever the base /// palette colors are changed. fn compute_palettes(&mut self) { + // re-computes the complete set of palettes according to + // the currently set palette colors (that may have chaged) Self::compute_palette(&mut self.palette, &self.palette_colors, self.palettes[0]); Self::compute_palette( &mut self.palette_obj_0, @@ -933,6 +1005,10 @@ impl Ppu { &self.palette_colors, self.palettes[2], ); + + // clears the frame buffer to allow the new background + // color to be used + self.clear_frame_buffer(); } /// Static method used for the base logic of computation of RGB @@ -955,3 +1031,36 @@ impl Default for Ppu { Self::new() } } + +#[cfg(test)] +mod tests { + use super::Ppu; + + #[test] + fn test_update_tile_simple() { + let mut ppu = Ppu::new(); + ppu.vram[0x0000] = 0xff; + ppu.vram[0x0001] = 0xff; + + let result = ppu.tiles()[0].get(0, 0); + assert_eq!(result, 0); + + ppu.update_tile(0x8000, 0x00); + let result = ppu.tiles()[0].get(0, 0); + assert_eq!(result, 3); + } + + #[test] + fn test_update_tile_upper() { + let mut ppu = Ppu::new(); + ppu.vram[0x1000] = 0xff; + ppu.vram[0x1001] = 0xff; + + let result = ppu.tiles()[256].get(0, 0); + assert_eq!(result, 0); + + ppu.update_tile(0x9000, 0x00); + let result = ppu.tiles()[256].get(0, 0); + assert_eq!(result, 3); + } +} diff --git a/src/rom.rs b/src/rom.rs index cc8c5b53365987a44e00601a77f7c0ead750c64c..20ab159627ceda2868c4e8449e57cf4d3abd4a55 100644 --- a/src/rom.rs +++ b/src/rom.rs @@ -412,7 +412,7 @@ impl Cartridge { } pub fn has_battery(&self) -> bool { - return matches!( + matches!( self.rom_type(), RomType::Mbc1RamBattery | RomType::Mbc2Battery @@ -425,7 +425,7 @@ impl Cartridge { | RomType::Mbc5RumbleRamBattery | RomType::Mbc7SensorRumbleRamBattery | RomType::HuC1RamBattery - ); + ) } pub fn ram_data_eager(&self) -> Vec<u8> {