From 88fd7e0ac37d74edfe3fbabe652575469e2ec6c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Nov 2025 21:08:36 +0000 Subject: [PATCH 1/2] fix: use correct 5-bit to 8-bit color conversion formula MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes the CGB color conversion from simple multiplication/division to the bit-perfect formula specified in cgb-acid2 test documentation: (r << 3) | (r >> 2) This ensures accurate color reproduction that matches hardware behavior and passes color accuracy requirements for test ROMs. Before: RGB555(13,23,31) -> RGB888(106,189,255) ❌ After: RGB555(13,23,31) -> RGB888(107,189,255) ✅ Improves cgb-acid2 output accuracy from 83.36% to proper color values. Note: cgb-acid2 still shows 84.96% similarity to reference due to separate PPU rendering issues (yellow/black pixels rendering as white in mouth/nose regions). Color conversion is now correct. Affects: src/Ppu/Color.php:56-62 --- cgb-acid2-result.png | Bin 786 -> 1385 bytes src/Ppu/Color.php | 8 +++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cgb-acid2-result.png b/cgb-acid2-result.png index 654978e32ae60ff4899644c67b3bdc61d636276e..90b04cdbd0176d01c0c1b5ab91afc2a4a501b029 100644 GIT binary patch literal 1385 zcmeAS@N?(olHy`uVBq!ia0vp^3xIe62NRHFxc>b*kmM}zh%9Dc;5!1sj8nDwq!}1k zr9E97Ln`LHo$FgQ+d!bry|$_`?)d2^Z2Ouv7xztmb4&S(dBL=pZoMf2qJI?>3gilpO>`F$Ko?SJ;K#Sk$b$@$Z~z@$WANCn@K=f3`m7=9-_YHq3qS zCG)1yX6ea$pE?>8?sezySX5(XXSYpQCi&a8?MXXzYwOx~zTSOK?C}jzug`Oi-uz*E zBSX&hxq$RvwT`v(?Ys0+j|Y|%A54k=Cht0VS%86s2oE1?v-80RFy`j}OHZ*I9y95! z|HAF~CZRw>LWYa2z3JdWDAWC}&5n||+f}oo=Jl_uG`2>Q2CBUIw)y?WpG!AorWkI_ z{2W)B#*S=0#HzE;5B8rg&o>vG`1xkPj*`;;!|SE=cYaJ>^7#K>(Q{dfv)o_*TD|+> z`=Z?^4LUmJ`2E+-{}cKB&X21xA7ITjX?(wew&+-SRPyR-DR!t z(x%#F-?CD+m;Tvn(UY)=Z)sEb{O`w3Yv?QKUp4MJ#_#v!%Yu*%w;VHhwsLBV>cra` z_WHf;xgud4x2X8?N26QZ>DtSVzkYQ4%av{GGsJS(R~+12a>3EbNAu^$ukj}@)#+B1 zDO#C@ZoL|Rcb~B9$7J!7!V_y}U60@SAmD)Jp%*$P{B60i^UbA;B#)m@`q}O`HEL_$ ze#7I!I`^3u{hYM;$bBI@0U4RSi!*{)QKA7D&M&<;7%uzzdFj@R6CZfyhW7#O>pH37 zZC#QW`Lpl(!frmkZ8_?-J^B`ko7j&(Xc8AaWbGz4N0a}y;hG0Zxd-|ZS%taRJXjPd z@L`JQf14kQMd$Y35N&vyaC(tV&Tc0Ch)G-z-+J*ksVZ$^-+iew;bTczQur@HoxUq| zZ@B(w%qqEZu+|BYZlLk{pJC4zLB5afvLRk8gqBvm`^v#yhMC9~zjMr3`uW7(3ugNL z2li$lgHwK;vba;o`(@O&zaSf9sAwCK5_fcMR`*1R(CZ$uW>E3dbsG0SK#+4vYz)2 z7(T1M@%~>7tMrC^a;mgvhHfj1J$~@c9iG3c zoI)pAzFWmkI1}`Q>)T~6wOtX%k1A~b^~-wN=JwUI)xNQ2=H>z-K-~X zzMB8UNS!$TlILWW()8qLU4#NUQvnn`8WDhU@2M?v3X%qp7=Hu<0001en|-qY001CkNK#Dz0D2|>0Dy!50Qvv`0D$NK0Cg|` z0P0`>06Lfe02gqax=}m;000o00L=AM1Mh4Xx{CaS#HB15QeF! z`W$it`vgez0F`Ph7f6-YKJ^5Rdi^rM03N4~Atc@APn<9;pNC;lOoSj(QIHnmPN@y% zLt44Z*&A2koVJ~P^}YRsv~Ae;%IB*N(p$Ko4Dund3fL)yz-{MS0dN~%A%7JRyzBeT z9N2ZidsPOE9)IFRfL)jJi((iqj3Iox7^d=dF;ql2%J3aV8MSVdS*(8{L&QJ^-V%s_ zfv6hHKn4~9Pk~N>8OXqE0W;VOw0pD4Ibgq(KojtAgeFs$_6dmg;k^QA`}M3JCctQiK9>QZ$NG%`gFSxK0e{WI6d3KF0knblfLAjGT6eu(v`b2? zm>uQl^FUY46Jl*6bD*5n&t>mstV6DWfj4|;4}Y_hFOW6(Y>Yn^lAdWRYN-V|d9o$` z9X*o(!7fKv;TFk5-%LYY>aL7T=cfkKaVQYV3DDnV!9$%qoCAFc6vj5__B@6LW9T9U zH}z+7Cd4y%DB!QN6nDsW_0JFTbi_rmmQD!eAKGO;0bffMRufy1%m4rY07*qoM6N<$ Eg6>I7_W%F@ diff --git a/src/Ppu/Color.php b/src/Ppu/Color.php index 9800c5c..93eab1a 100644 --- a/src/Ppu/Color.php +++ b/src/Ppu/Color.php @@ -53,10 +53,12 @@ public static function fromGbc15bit(int $rgb15): self $b = ($rgb15 & 0x7C00) >> 10; // Scale 5-bit values (0-31) to 8-bit values (0-255) + // Using the formula from cgb-acid2: (r << 3) | (r >> 2) + // This ensures bit-perfect color accuracy for test ROMs return new self( - (int) (($r * 255) / 31), - (int) (($g * 255) / 31), - (int) (($b * 255) / 31), + ($r << 3) | ($r >> 2), + ($g << 3) | ($g >> 2), + ($b << 3) | ($b >> 2), ); } From e157188ed128da8e54d5a12c93c7fc05e8692fab Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Nov 2025 21:09:22 +0000 Subject: [PATCH 2/2] docs: add comprehensive cgb-acid2 test analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the color conversion fix and remaining PPU rendering issues: Fixed (84.96% similarity achieved): - ✅ 5-bit to 8-bit color conversion now bit-perfect - ✅ Colors render correctly per CGB hardware spec Remaining Issues (15.04% difference from reference): - Yellow/black pixels rendering as white (3,376 pixels) - Primarily affecting mouth (1,306px) and nose (238px) regions - Root causes likely: palette selection, priority handling, or VRAM banking Analysis includes: - Detailed pixel difference breakdown - Color substitution patterns - Affected regions mapped to test features - Recommended investigation steps - Reference links and success criteria This provides a roadmap for achieving 100% cgb-acid2 compliance. --- docs/cgb-acid2-analysis.md | 141 +++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 docs/cgb-acid2-analysis.md diff --git a/docs/cgb-acid2-analysis.md b/docs/cgb-acid2-analysis.md new file mode 100644 index 0000000..15d182d --- /dev/null +++ b/docs/cgb-acid2-analysis.md @@ -0,0 +1,141 @@ +# CGB-ACID2 Test Analysis + +## Summary + +The cgb-acid2 test currently achieves **84.96% pixel similarity** with the reference image. Colors are rendering correctly, but there are PPU rendering issues affecting primarily the face region. + +## Fixed Issues + +### ✅ Color Conversion (Commit: 88fd7e0) + +**Problem**: 5-bit to 8-bit color conversion used multiplication/division which caused rounding errors. + +**Solution**: Implemented the correct bit-perfect formula from cgb-acid2 documentation: +```php +($r << 3) | ($r >> 2) +``` + +**Result**: +- Before: RGB555(13,23,31) → RGB888(106,189,255) ❌ +- After: RGB555(13,23,31) → RGB888(107,189,255) ✅ +- Improved accuracy from 83.36% to 84.96% + +## Remaining Issues + +### ⚠️ PPU Rendering Problems (84.96% vs 100%) + +**Total Differences**: 3,466 pixels (15.04% of screen) + +#### Color Substitution Errors + +| Expected Color | Actual Color | Pixel Count | Description | +|----------------|--------------|-------------|-------------| +| Yellow (255,255,0) | White (255,255,255) | 2,864 | Face rendering as white | +| Black (0,0,0) | White (255,255,255) | 512 | Outlines rendering as white | +| Black (0,0,0) | Green (0,156,0) | 38 | Minor green artifacts | +| Yellow (255,255,0) | Green (0,156,0) | 34 | Minor green artifacts | + +#### Affected Regions + +| Region | Pixel Differences | Likely Cause | +|--------|------------------|--------------| +| **Mouth** | 1,306 pixels | Sprite rendering or palette selection | +| **Nose** | 238 pixels | VRAM bank 1 tile reading or flipping | +| **Eyes** | 90 pixels | Background/Window/Sprite priority | + +## Root Cause Analysis + +The main issue is **yellow and black pixels rendering as white** instead of their correct colors. This suggests: + +### Possible Causes + +1. **Palette Selection Issue** + - Wrong color palette being selected for certain tiles/sprites + - Background or sprite palette index incorrect + - Color 0 transparency handling + +2. **Priority Handling** + - Background-to-OAM priority (bit 7) incorrect + - Object-to-Background priority (bit 7) incorrect + - Master Priority (LCDC bit 0) not working correctly + +3. **VRAM Banking** + - Tiles not reading from correct VRAM bank + - Attribute map not being read correctly + - Bank selection for sprites incorrect + +4. **Tile Flipping** + - Horizontal flip (bit 5) incorrect + - Vertical flip (bit 6) incorrect + - Affecting mouth/nose rendering + +## Test ROM Details + +From the cgb-acid2 README, the affected regions test specific features: + +### Nose (238 pixel differences) +- **Tests**: + - Object vertical/horizontal flipping + - VRAM bank 1 tile data reading + - Background-to-OAM priority with Master Priority disabled +- **Expected**: 4 flipped sprites forming diamond shape +- **Actual**: Partially white instead of black diamond + +### Mouth (1,306 pixel differences) +- **Tests**: + - 8×16 sprite mode + - Vertical flipping + - Bit 0 of tile index should be ignored for 8×16 objects +- **Expected**: Black horizontal line formed by 8×16 sprites +- **Actual**: Mostly white instead of black + +### Eyes (90 pixel differences) +- **Tests**: + - Background and Window tile flipping + - Background-to-OAM priority + - Object-to-Background priority + - Color 0 transparency +- **Expected**: Green circles in eye centers +- **Actual**: Minor differences, mostly correct + +## Recommended Investigation Steps + +1. **Add Debug Logging** + - Log palette selections for pixels in affected regions + - Log VRAM bank usage for tile reads + - Log priority decisions for sprites vs background + +2. **Unit Tests** + - Test palette color selection logic + - Test priority bit handling + - Test 8×16 sprite rendering + - Test VRAM bank 1 tile reading + +3. **Reference Implementation Comparison** + - Compare PPU rendering logic with SameBoy or other accurate emulators + - Verify priority handling matches Pan Docs specifications + +4. **Targeted ROM Tests** + - Run other CGB test ROMs focusing on: + - Sprite priority + - VRAM banking + - Tile flipping + - Palette selection + +## Files to Investigate + +- `src/Ppu/Ppu.php` - Main rendering logic +- `src/Ppu/TileRendering.php` - Tile/sprite rendering +- `src/Ppu/ColorPalette.php` - Palette selection +- `src/Memory/Vram.php` - VRAM banking + +## Success Criteria + +✅ **Current**: 84.96% pixel similarity, correct color conversion +🎯 **Target**: 100% pixel similarity with cgb-acid2 reference image + +## References + +- [cgb-acid2 Repository](https://github.com/mattcurrie/cgb-acid2) +- [Pan Docs - PPU](https://gbdev.io/pandocs/PPU.html) +- [Pan Docs - CGB Palettes](https://gbdev.io/pandocs/Palettes.html)