From c477191f5bd78c4866b867f452429711432f6ffc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Magalh=C3=A3es?= <joamag@gmail.com>
Date: Mon, 19 Jun 2023 00:09:09 +0100
Subject: [PATCH] fix: support for the Bit 7 BG attributes This allows
 background to take precedence over objects. This completes the CGB ACID 2
 test compliance.

---
 README.md                            |   2 +-
 frontends/sdl/res/test/cgb_acid2.png | Bin 0 -> 3138 bytes
 frontends/sdl/src/test.rs            |  15 +++++++++++++++
 src/ppu.rs                           |  26 +++++++++++++++++++++++++-
 src/test.rs                          |  17 +++++++++++------
 5 files changed, 52 insertions(+), 8 deletions(-)
 create mode 100644 frontends/sdl/res/test/cgb_acid2.png

diff --git a/README.md b/README.md
index 3d55f45a..784a120a 100644
--- a/README.md
+++ b/README.md
@@ -16,7 +16,7 @@ A Game Boy emulator that is written in Rust 🦀.
 * Game Boy Printer emulation
 * Support for multiple MBCs: MBC1, MBC2, MBC3, and MBC5
 * Variable CPU clock speed
-* Accurate PPU - passes [dmg-acid2](https://github.com/mattcurrie/dmg-acid2) tests
+* Accurate PPU - passes [dmg-acid2](https://github.com/mattcurrie/dmg-acid2) and [cgb-acid2](https://github.com/mattcurrie/cgb-acid2) tests
 
 For the Web front-end...
 
diff --git a/frontends/sdl/res/test/cgb_acid2.png b/frontends/sdl/res/test/cgb_acid2.png
new file mode 100644
index 0000000000000000000000000000000000000000..6d23ed4468bb4e3fc45019b1ac6fb7aea4642067
GIT binary patch
literal 3138
zcmeAS@N?(olHy`uVBq!ia0y~yU|7Jwz%YS>iGhKE;rjRM3=BM+o-U3d6^w5mFiueM
zoTTC@)V*rvZg&@#w6wIa(9qD(auE1_h8Ca%WOR5q$d<G;H#e7--Lq$Mo%cx#FE1@G
z_dgG^(kCf)ZP=gmUoY-IyZ>f&{Ot6fFYZ5!-1lp1<%0fyx${nk?>Eyw;r`$3{Ppnt
zX7Yb;+e}|ymuvTHdu3ew-&-|b@;~du|Gi~%8l>X&>Ue2}A5iT-&$~ahzi+<Z@BsUN
zqXS6v`KQhGk`HFzmw3=zKlAygv-vz94(>n0T>B-Tx#qL<XC3)Jr$NSEU4NDvCMkOV
z&#{^h`kycE=lyUt|IB72Hghdp9BfYU&u8Y(8tr~e2YF+4{A}%gKYJNLTz$t6_un?x
z&*cA?3gSM9e|G;F)DisuQXBH?lK<tx)INx3`vEfs%69#DaQ_*oPyW}g_!ADY=jX3y
z=FhtL|E2ExwH0LF{xjzL4dwrw-qW$~XYbBO&Gj??+k%ur&DaTYw+<)_FMxvUy8C~#
z|6f6ld1n3$6c%4E?myf84y4N|<lpXEXkgf!Hs5b}{?ozzXEwvahXE8upQS&a%|9aw
zvaSHm=K+Ne`+uWwkX2{%LGA%FV<6VB{df@nEOOt^-pXnG|5E2Yoqd0Xw%w2El?(Fg
zk{kH{rT+6?a5d{_{j&alx&MM|zh-~Fxc_YN&sW!<wSuB`XU*4aW>DP3K>d6FJ!*`%
zhpiS3|2yCH>8k&ySHyg+mz=u#pZ?;atN&9&*8SSwb1L-z^uV33{u^n9|BZKhTEFvW
z&AIwzSF?`(Pr@dTV#p`^Gc|V4<CC{OvcC#pM18d9z-vB8{~z<2=bziyuNM8kcum-!
z)n}d;&zleOAv44YASF25`P1KQ{`tzfFzq-O`@feN^8a45nf`N5dgR(4{O0pN>#P<%
z|8#l1)PuG6XFcB(^|9tG%w#34|IL5D_<ufL&-3Bb`ZLUDpW96TDU-1Ek^PaHbHy{y
zGlpr)z3Bfx!~S1#+|Q|%ueU#|*V6hQjHD%czv+3S`Nc=Gwmz!+{8MJe`N*|D%4eP5
znY1;kaR1L5zBA9)gvETkme2d)c>Njq|Aq&m_nZD#4*WU)RsVm92UFu`Zm-Lpmuklz
zu{P|$pWV;z<Zq2){QR@VE`95x`#D&JySCS5H`xD6{`aFF>>!n<{}bzeEw7CGY<VDS
zE7P<4w$B-#-M3{ATP<oZ|8u_aeC9P_+6n*e7R!N%1d#cb=U*?cpY?wt$3OR9kL}qi
zo`ydI1&!(dcb4aq|J^PA*L-Gw^|{Y8>c7u<{_$G=S?&Es2ju@7{#O?Kqi+Z@Z}rde
z^OYc5;@5^9s5!U)S`FWZsI?%gS!dLLuL0Zf-#Y-5e474GjQcs2@!yaBpYz{C{S3Cb
z3KW{tx7TI=b65H?|JBrZ$p_2pXK9CNH~g&GSM^?0|G=xg;h%rXM6CUBJ!&n(v(G<Y
z?G4|<u=;!4+2_RxTbUT@el2H=`#JUJ{H_P}tG3r=gV>MlpH;5CSAFzz#NO{+{}on$
zkNap7_I~f9%0D0FKc8BE1{C_i3qgr$+0UADpEpN+T=e7Re71_$+u0bN6*pYXVg>p9
zZ`k|2OwT@BZixDrZwB%&IQ-siuX-<PF#mkx)h=z2-S>X2tqeT#+$MhY)vo_88C#=1
z&d;=)|M}0Fus^rYJdfEJwRXq-8oTG4u6ETg|2aoHY_;h3+2^0fY>WDsZ#@5V&Dt>S
zf5L%3@271|vSXid{<+Qat6lZ`uXg>ENwAy$c}|%2zwHY@?Z1%4Y5-2b{~JLe15VTT
zIYD}rA^D?gbzH~KInT4VMtziTK+Ck4Ip&`|I8pz*ukfQj+3tDQ&pH3iIsVnpIA6Ko
z=bZoY9RK8JoCl?(|NH`f;?3uOZaVY)PrUnw`dc;UxXtH(u9rIaKlZbwK1kngj(`2L
z&R1SM`}~hQxDes^*B`aE;^Nw{Kk}|0>aS-V?M~YI=)Ydm|I1<8aXYSd{lCodZ+-OI
z3db<*f9sWh)F*6xRJk_nkG;c(|A(`VGOrE$^WWja|6^H4MOTad&ky+dKV|Eq|B{6B
zKL4huwH0;GK3kqYpLO)JDL82V^c#U`oA}R`=dWjNePl04xcK4U3@#h~^lys#xSx@b
z`U_cGAMHO}WB2@L<k}zcxpvQgMy(C|W4-`a2?~#l|7xqRc0EtGoBw(J#;A|;v+SP7
zr*0*#faKpCwf4vL8Rwt>Ne5Y`??$lj{(pLHnD)A=Ml2=#1N(C|cF*k()!03+JoCI*
zZ}$1;pps|)XQngHi~oIn_Sq7YUiUD3{yFEl$^7$`3!j0d&p-bYefD|rzupyp&To!d
zTfzA3^UtiUk2d|R*%!6;N5Ap>^Wm#Or9Q~?__bj%W*`C4|5dB6cGW9u>2La3b1rrD
z)vnXeK3g)Z3Dahn0nQVU7@WQ~OdI5mm~ByO8-CXCt$}cD4rFb8<Z3YgJR`XH`dPCz
zs_<rwUHsZG8~fE)v+U-t4_p1ShL7R%PZ<Vqv3I@^qB+5CJ~Oxo3ELB}`YP9q^Uqgb
z<pLEYpn?*Vzt=K=lt83FmcU$BFF95GEZ7E!`>%r83<hAoY=~OBBL`%u_Ppb8pCgAt
z7&rhRAp%lad=p#_WF6(+5CyhzeVBG!%Id3Kpkg0l=+&&Pvp}xv`n?w97f?`^L!1jH
z!G6?E09m^FYL+P|BtUF%V1WXbacceEt64|8)9mIK?+2A)&x_+4u4WzGp8{6)4^)}{
z<6ax4Etjx0>Z8rG&z7K|{+Al^?<>UNt{bA(R)9)gh~GZXfe6?4fTI-@N-!a4e6!95
zJGUNGL+@vrDt;F10dT~D9a!o2`RAO|VXH;YCvS~9`@9%r@!99adeNX7!;T$neAL=7
zXyV#=KWpowpEJ)tXV?(6wgGIw&v2;pmZ-JhvUu%|9I#1XA3%fnY8LAnP<Zh%fI{J9
z4T`}a+y5JFiCX)k9OBsjykKh|{pZ&D&j(E`|2IQj^~b&`4ACO`;hxz3Yv$sptMQ<|
Nfv2mV%Q~loCIH)9M+*P|

literal 0
HcmV?d00001

diff --git a/frontends/sdl/src/test.rs b/frontends/sdl/src/test.rs
index 154a5a34..7fa014f8 100644
--- a/frontends/sdl/src/test.rs
+++ b/frontends/sdl/src/test.rs
@@ -26,6 +26,7 @@ pub fn compare_images(source_pixels: &[u8], target_path: &str) -> bool {
 #[cfg(test)]
 mod tests {
     use boytacean::{
+        gb::GameBoyMode,
         ppu::FRAME_BUFFER_SIZE,
         test::{run_image_test, TestOptions},
     };
@@ -54,6 +55,20 @@ mod tests {
         assert_eq!(image_result, true);
     }
 
+    #[test]
+    fn test_cgb_acid2() {
+        let result: [u8; FRAME_BUFFER_SIZE] = run_image_test(
+            "../../res/roms/test/cgb_acid2.gbc",
+            Some(50000000),
+            TestOptions {
+                mode: Some(GameBoyMode::Cgb),
+                ..Default::default()
+            },
+        );
+        let image_result = compare_images(&result, "res/test/cgb_acid2.png");
+        assert_eq!(image_result, true);
+    }
+
     #[test]
     fn test_firstwhite() {
         let result: [u8; FRAME_BUFFER_SIZE] = run_image_test(
diff --git a/src/ppu.rs b/src/ppu.rs
index 9e20dffe..24fbe4f5 100644
--- a/src/ppu.rs
+++ b/src/ppu.rs
@@ -285,6 +285,11 @@ pub struct Ppu {
     /// processed set of pixels ready to be displayed on screen.
     pub frame_buffer: Box<[u8; FRAME_BUFFER_SIZE]>,
 
+    /// The buffer that will control the background to OAM
+    /// priority, allowing the background to be drawn over
+    /// the sprites/objects if necessary.
+    priority_buffer: Box<[bool; COLOR_BUFFER_SIZE]>,
+
     /// Video dedicated memory (VRAM) where both the tiles and
     /// the sprites/objects are going to be stored.
     vram: [u8; VRAM_SIZE],
@@ -499,6 +504,7 @@ impl Ppu {
         Self {
             color_buffer: Box::new([0u8; COLOR_BUFFER_SIZE]),
             frame_buffer: Box::new([0u8; FRAME_BUFFER_SIZE]),
+            priority_buffer: Box::new([false; COLOR_BUFFER_SIZE]),
             vram: [0u8; VRAM_SIZE],
             hram: [0u8; HRAM_SIZE],
             oam: [0u8; OAM_SIZE],
@@ -555,6 +561,7 @@ impl Ppu {
     pub fn reset(&mut self) {
         self.color_buffer = Box::new([0u8; COLOR_BUFFER_SIZE]);
         self.frame_buffer = Box::new([0u8; FRAME_BUFFER_SIZE]);
+        self.priority_buffer = Box::new([false; COLOR_BUFFER_SIZE]);
         self.vram = [0u8; VRAM_SIZE_CGB];
         self.hram = [0u8; HRAM_SIZE];
         self.vram_bank = 0x0;
@@ -1196,6 +1203,10 @@ impl Ppu {
         let mut xflip = tile_attr.xflip;
         let mut yflip = tile_attr.yflip;
 
+        // obtains the value the BG-to-OAM priority to be used in the computation
+        // of the final pixel value (CGB only)
+        let mut priority = tile_attr.priority;
+
         // increments the tile index value by the required offset for the VRAM
         // bank in which the tile is stored, this is only required for CGB mode
         tile_index += tile_attr.vram_bank as usize * TILE_COUNT_DMG;
@@ -1240,6 +1251,12 @@ impl Ppu {
             self.frame_buffer[frame_offset + 1] = color[1];
             self.frame_buffer[frame_offset + 2] = color[2];
 
+            // updates the priority buffer with the current pixel
+            // the priority is only set in case the priority of
+            // the background (over OAM) is set in the attributes
+            // and the pixel is not transparent
+            self.priority_buffer[color_offset] = priority && pixel > 0;
+
             // increments the current tile X position in drawing
             x += 1;
 
@@ -1269,6 +1286,7 @@ impl Ppu {
                     palette = &self.palettes_color_bg[tile_attr.palette as usize];
                     xflip = tile_attr.xflip;
                     yflip = tile_attr.yflip;
+                    priority = tile_attr.priority;
                     tile_index += tile_attr.vram_bank as usize * TILE_COUNT_DMG;
                 }
 
@@ -1540,7 +1558,13 @@ impl Ppu {
                     // 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_over || self.color_buffer[color_offset as usize] == 0;
+                    let mut is_visible = obj_over || self.color_buffer[color_offset as usize] == 0;
+
+                    // additionally (in CCG mode) the object is only considered to
+                    // be visible if the priority buffer is not set for the current
+                    // pixel, this means that the background is capturing priority
+                    // by having the BG-to-OAM priority bit set in the bg map attributes
+                    is_visible &= always_over || !self.priority_buffer[color_offset as usize];
 
                     // determines if the current pixel has priority over a possible
                     // one that has been drawn by a previous object, this happens
diff --git a/src/test.rs b/src/test.rs
index c9e03698..ac1cd497 100644
--- a/src/test.rs
+++ b/src/test.rs
@@ -1,16 +1,21 @@
-use crate::{devices::buffer::BufferDevice, gb::GameBoy, ppu::FRAME_BUFFER_SIZE};
+use crate::{
+    devices::buffer::BufferDevice,
+    gb::{GameBoy, GameBoyMode},
+    ppu::FRAME_BUFFER_SIZE,
+};
 
 #[derive(Default)]
 pub struct TestOptions {
-    ppu_enabled: Option<bool>,
-    apu_enabled: Option<bool>,
-    dma_enabled: Option<bool>,
-    timer_enabled: Option<bool>,
+    pub mode: Option<GameBoyMode>,
+    pub ppu_enabled: Option<bool>,
+    pub apu_enabled: Option<bool>,
+    pub dma_enabled: Option<bool>,
+    pub timer_enabled: Option<bool>,
 }
 
 pub fn build_test(options: TestOptions) -> GameBoy {
     let device = Box::<BufferDevice>::default();
-    let mut game_boy = GameBoy::new(None);
+    let mut game_boy = GameBoy::new(options.mode);
     game_boy.set_ppu_enabled(options.ppu_enabled.unwrap_or(true));
     game_boy.set_apu_enabled(options.apu_enabled.unwrap_or(true));
     game_boy.set_dma_enabled(options.dma_enabled.unwrap_or(true));
-- 
GitLab