Fix rendering bug in the window

When window is full screen the ppu would still trim the fifo according to scx even though the window is rendering and not the background. Also improve the fetchers code
alloncm · Feb 10, 2023 · 5881b67 · 5881b67
1 parent fc3dfd4
commit 5881b67
Show file tree

Hide file tree

Showing 8 changed files with 53 additions and 62 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -7,5 +7,5 @@ members = [
 ]
 
 [workspace.package]
-version = "2.3.1"
+version = "2.3.2"
 authors = ["alloncm <[email protected]>"]
diff --git a/lib_gb/src/ppu/fifo/background_fetcher.rs b/lib_gb/src/ppu/fifo/background_fetcher.rs
@@ -1,15 +1,15 @@
 use crate::{mmu::vram::VRam, utils::{bit_masks::*, fixed_size_queue::FixedSizeQueue, vec2::Vec2}};
-use super::{FIFO_SIZE, SPRITE_WIDTH, fetcher_state_machine::FetcherStateMachine, fetching_state::*};
+use super::{FIFO_SIZE, SPRITE_WIDTH, fetching_state::*};
 
 const EMPTY_FIFO_BUFFER:[u8;FIFO_SIZE] = [0;FIFO_SIZE];
 
 pub struct BackgroundFetcher{
     pub fifo:FixedSizeQueue<u8, FIFO_SIZE>,
     pub window_line_counter:u8,
     pub has_wy_reached_ly:bool,
+    pub rendering_window:bool,
 
     current_x_pos:u8,
-    rendering_window:bool,
     fetcher_state_machine:FetcherStateMachine,
     scanline_rendering_started:bool,
 }
@@ -66,19 +66,15 @@ impl BackgroundFetcher{
 
                 self.fetcher_state_machine.data.reset();
                 self.fetcher_state_machine.data.tile_data = tile_num;
+                // Calculating once per fetching cycle might be inaccurate (not sure), but could improve perf
+                self.fetcher_state_machine.data.tile_data_address = self.get_tila_data_address(lcd_control, bg_pos.y, ly_register, tile_num);
             }
             FetchingState::FetchLowTile=>{
-                let tile_num = self.fetcher_state_machine.data.tile_data;
-                let address = self.get_tila_data_address(lcd_control, bg_pos, ly_register, tile_num);
-                let low_data = vram.read_current_bank(address);
-
+                let low_data = vram.read_current_bank(self.fetcher_state_machine.data.tile_data_address);
                 self.fetcher_state_machine.data.low_tile_data = low_data;
             }
             FetchingState::FetchHighTile=>{
-                let tile_num= self.fetcher_state_machine.data.tile_data;
-                let address = self.get_tila_data_address(lcd_control, bg_pos, ly_register, tile_num);
-                let high_data = vram.read_current_bank(address + 1);
-
+                let high_data = vram.read_current_bank(self.fetcher_state_machine.data.tile_data_address + 1);
                 self.fetcher_state_machine.data.high_tile_data = high_data;
 
                 // The gameboy has this quirk that in the first fetch of the scanline it reset itself after reaching the fetch high tile step
@@ -122,13 +118,13 @@ impl BackgroundFetcher{
         self.fetcher_state_machine.advance();
     }
 
-    fn get_tila_data_address(&self, lcd_control:u8, bg_pos:&Vec2<u8>, ly_register:u8, tile_num:u8)->u16{
+    fn get_tila_data_address(&self, lcd_control:u8, scy:u8, ly_register:u8, tile_num:u8)->u16{
         let current_tile_base_data_address = if (lcd_control & BIT_4_MASK) == 0 && (tile_num & BIT_7_MASK) == 0 {0x1000} else {0};
         let current_tile_data_address = current_tile_base_data_address + (tile_num  as u16 * 16);
         return if self.rendering_window{
             current_tile_data_address + (2 * (self.window_line_counter % SPRITE_WIDTH)) as u16
         } else{
-            current_tile_data_address + (2 * ((bg_pos.y as u16 + ly_register as u16) % SPRITE_WIDTH as u16))
+            current_tile_data_address + (2 * ((scy as u16 + ly_register as u16) % SPRITE_WIDTH as u16))
         };
     }
 

diff --git a/lib_gb/src/ppu/fifo/fetcher_state_machine.rs b/lib_gb/src/ppu/fifo/fetcher_state_machine.rs
diff --git a/lib_gb/src/ppu/fifo/fetching_state.rs b/lib_gb/src/ppu/fifo/fetching_state.rs
@@ -7,18 +7,47 @@ pub enum FetchingState{
     Sleep
 }
 
-
-
 pub struct FetchingStateData{
+    pub tile_data_address:u16,
     pub tile_data:u8,
     pub low_tile_data:u8,
     pub high_tile_data:u8,
 }
 
 impl FetchingStateData{
     pub fn reset(&mut self){
+        self.tile_data_address = 0;
         self.high_tile_data = 0;
         self.low_tile_data = 0;
         self.tile_data = 0;
     }
+}
+
+pub struct FetcherStateMachine{
+    pub data:FetchingStateData,
+    state:usize,
+    state_machine:[FetchingState;8]
+}
+
+impl FetcherStateMachine{
+    pub fn advance(&mut self){
+        self.state = (self.state + 1) % 8;
+    }
+
+    pub fn new(state_machine:[FetchingState;8])->Self{
+        Self{
+            data:FetchingStateData{tile_data_address:0, high_tile_data:0, low_tile_data:0, tile_data:0},
+            state:0,
+            state_machine
+        }
+    }
+
+    pub fn reset(&mut self){
+        self.state = 0;
+        self.data.reset();
+    }
+
+    pub fn current_state(&self)->&FetchingState{
+        &self.state_machine[self.state]
+    }
 }
diff --git a/lib_gb/src/ppu/fifo/mod.rs b/lib_gb/src/ppu/fifo/mod.rs
@@ -1,7 +1,6 @@
 pub mod background_fetcher;
 pub mod sprite_fetcher;
 mod fetching_state;
-mod fetcher_state_machine;
 
 pub const FIFO_SIZE:usize = 8;
 pub const SPRITE_WIDTH:u8 = 8;
diff --git a/lib_gb/src/ppu/fifo/sprite_fetcher.rs b/lib_gb/src/ppu/fifo/sprite_fetcher.rs
@@ -1,5 +1,5 @@
 use crate::{mmu::vram::VRam, ppu::sprite_attribute::SpriteAttribute, utils::{self, bit_masks::{BIT_0_MASK, BIT_2_MASK}, fixed_size_queue::FixedSizeQueue}};
-use super::{FIFO_SIZE, SPRITE_WIDTH, fetcher_state_machine::FetcherStateMachine, fetching_state::*};
+use super::{FIFO_SIZE, SPRITE_WIDTH, fetching_state::*};
 
 pub const NORMAL_SPRITE_HIGHT:u8 = 8;
 pub const EXTENDED_SPRITE_HIGHT:u8 = 16;
@@ -43,21 +43,15 @@ impl SpriteFetcher{
 
         match self.fetcher_state_machine.current_state(){
             FetchingState::FetchTileNumber=>{
-                self.try_fetch_tile_number(current_x_pos, lcd_control);
+                self.try_fetch_tile_number(current_x_pos, lcd_control, sprite_size, ly_register);
             }
             FetchingState::FetchLowTile=>{
-                let tile_num = self.fetcher_state_machine.data.tile_data;
-                let oam_attribute = &self.oam_entries[self.current_oam_entry as usize];
-                let current_tile_data_address = Self::get_current_tile_data_address(ly_register, oam_attribute, sprite_size, tile_num);
-                let low_data = vram.read_current_bank(current_tile_data_address);
+                let low_data = vram.read_current_bank(self.fetcher_state_machine.data.tile_data_address);
                 self.fetcher_state_machine.data.low_tile_data = low_data;
                 self.fetcher_state_machine.advance();
             }
             FetchingState::FetchHighTile=>{
-                let tile_num= self.fetcher_state_machine.data.tile_data;
-                let oam_attribute = &self.oam_entries[self.current_oam_entry as usize];
-                let current_tile_data_address = Self::get_current_tile_data_address(ly_register, oam_attribute, sprite_size, tile_num);
-                let high_data = vram.read_current_bank(current_tile_data_address + 1);
+                let high_data = vram.read_current_bank(self.fetcher_state_machine.data.tile_data_address + 1);
                 self.fetcher_state_machine.data.high_tile_data = high_data;
                 self.fetcher_state_machine.advance();
             }
@@ -100,7 +94,7 @@ impl SpriteFetcher{
     }
 
     //This is a function on order to abort if rendering
-    fn try_fetch_tile_number(&mut self, current_x_pos: u8, lcd_control: u8) {
+    fn try_fetch_tile_number(&mut self, current_x_pos: u8, lcd_control: u8, sprite_size:u8, ly_register:u8) {
         if self.oam_entries_len > self.current_oam_entry{
             let oam_entry = &self.oam_entries[self.current_oam_entry as usize];
             if oam_entry.x <= current_x_pos + SPRITE_WIDTH && current_x_pos < oam_entry.x{
@@ -111,6 +105,8 @@ impl SpriteFetcher{
                 self.rendering = true;
                 self.fetcher_state_machine.data.reset();
                 self.fetcher_state_machine.data.tile_data = tile_number;
+                self.fetcher_state_machine.data.tile_data_address = Self::get_current_tile_data_address(ly_register, 
+                    &self.oam_entries[self.current_oam_entry as usize], sprite_size, tile_number);
                 self.fetcher_state_machine.advance();
                 return;
             }

diff --git a/lib_gb/src/ppu/gb_ppu.rs b/lib_gb/src/ppu/gb_ppu.rs
@@ -315,9 +315,10 @@ impl<GFX:GfxDevice> GbPpu<GFX>{
             return;
         }
         if !self.scanline_started{
+            let screen_x_indicator = if self.bg_fetcher.rendering_window{self.window_pos.x}else{self.bg_pos.x};
             // discard the next pixel in the bg fifo
             // the bg fifo should start with 8 pixels and not push more untill its empty again
-            if FIFO_SIZE as usize - self.bg_fetcher.fifo.len() >= self.bg_pos.x as usize % FIFO_SIZE as usize{
+            if FIFO_SIZE as usize - self.bg_fetcher.fifo.len() >= screen_x_indicator as usize % FIFO_SIZE as usize{
                 self.scanline_started = true;
             }
             else{