PPU implementation with sprites

STATE.md

@@ -45,7 +45,12 @@
 - PPU 2a done (src/palette.rs + src/render.rs): 64-color 2C02 SYSTEM_PALETTE in index order, index_to_rgb with & 0x3F mask. Renderer trait (the only hardware trait) + PpmRenderer writing numbered frames to renders/ (frame_0000.ppm etc.) + pure ppm_bytes for testability. 4 unit tests pass. 2C02 chosen over composite palettes - we render raw PPU output
 - PPU 2b done (src/ppu.rs struct + registers + VRAM routing + src/bus.rs open-bus): Ppu has ctrl/mask/status/oam/oamaddr, shared write_latch (W bit for $2005/$2006), vram_addr (14-bit), data_buffer ($2007 read-buffer), vram[0x1000] (4KB for FourScreen), palette[32]. read/write handle $2000-$2007 with open-bus for write-only regs ($2000/$2001/$2003/$2005/$2006), $2002 read = (status & 0xE0) | (open_bus & 0x1F) + clears vblank + resets write latch, $2004 write increments oamaddr, $2007 palette reads bypass the read-buffer. VRAM routing: $0000-$1FFF = CHR via cart, $2000-$3EFF = nametables via cart.mirroring() (H/V/OneScreenLow/High/FourScreen), $3F00-$3FFF = palette with 10/14/18/1C mirror. Bus tracks open_bus (last byte on CPU data bus, updated on every read/write) and passes it into ppu.read; PPU regs routed via disjoint-field borrows; APU/expansion stubs now return open_bus instead of 0. cartridge.rs mirroring honors the four-screen header bit
 - PPU 2c implementation done (src/ppu.rs full background renderer): scanline/cycle timing (3 PPU dots per CPU cycle, 341 dots/scanline, 262 scanlines/frame, odd-frame 340-dot pre-render line), vblank+NMI at scanline 241 dot 0, clear at 261 dot 0, frame_complete at 261 end. Loopy scroll $2005/$2006 (t/x/w, shared write latch reset on $2002 read). Background fetch pipeline (4-fetch/8-dot: nametable at %8==0, attribute at 2, pattern lo at 4, pattern hi at 6, reload shift regs + increment coarse X at 7), two attribute shift registers (0xFF/0x00 per bit), horizontal copy at dot 255, vertical increment at 256, vertical copy at 280. Fine-X pre-shift at cycle 0 of visible scanlines. Transparent pixel uses palette[0]. Fixed mirroring swap (Horizontal=>a>>11, Vertical=>a>>10). Bus frame-driving methods (tick_ppu/frame_done/take_nmi/framebuffer/begin_frame). main.rs run_cart boots any ROM, renders frames to renders/ with PpmRenderer
-- CURRENT BUG: SMB renders ALL GREY. The framebuffer is all 0x00 (palette index 0 = dark grey 0x5D5D5D) - every frame renders identical grey boxes (180/3600 frames tested). Diagnosis: either the framebuffer is never written (rendering never enabled / render_bg never true) or every pixel computes transparent. SMB ROM confirmed to write $2000 5x, $2001 3x, poll $2002 6x, reset vector $8000 points at real SEI init code. NMI forwarding fix applied (take_nmi/set_nmi moved inside the frame loop) but did not change the grey output. Next step is machine-state diagnosis
+- PPU 2c FIXED - SMB BACKGROUND RENDERS CORRECTLY. Three bugs found and fixed, in order:
+  1. NMI fired unconditionally at vblank (src/ppu.rs dot()): the PPU asserted nmi_pending on every vblank regardless of the $2000 NMI-enable bit (ctrl & 0x80). Game boots with NMI off, so every frame the CPU got yanked into the NMI handler mid-boot, pushing stack and spinning in the RAM-clear loop at $90CC (SP dropped 8 bytes/frame, PC pinned at $90DE, palette/nametable never written = all grey). Fixed by gating nmi_pending on ctrl & 0x80. Verified with debug_out/state.txt + pctrace.txt (src/dump.rs dbg mode)
+  2. Attribute table fetch wrong (src/ppu.rs bg_fetch): column term used (v & 0x07) instead of ((v >> 2) & 0x07), and quadrant selector used coarseX bit 0 / coarseY bit 0 instead of bit 1 ((v & 0x40) >> 4 | (v & 0x02)). Produced green/brown logo colors, invisible letters (blank lines through P/R), and wrong-palette garbage on the right edge. Fixed both formulas per nesdev
+  3. Background pipeline timing (src/ppu.rs dot/bg_fetch, modeled after olcNES Part 4 reference): shifters must shift ONLY during fetch windows (cycles 2-257 and 321-337), with LoadBackgroundShifters + TransferAddressX at cycle 257 (preloads the NEXT scanline's first tile), IncrementScrollY at 256, idle NT fetches at 338/340, TransferAddressY on the pre-render line 280-304. Our code shifted every dot and copied horizontal at 255 -> each scanline started mid-tile, image shifted showing page 1 + page 2. Rewrote dot() with windowed shifters, (cycle-1)%8 fetch phases, load_shifters/update_shifters helpers, bit-mux fine-X (0x8000 >> x) in render_bg_pixel. Frame now renders pixel-correct with correct colors
+- ACCEPTANCE MET: SMB renders the full frame correctly (background). Mario himself is a sprite and is NOT rendered yet - that's the next milestone (PPU-2 sprites + $4014 OAM DMA)
+- PPU-2 3a SPRITES DONE: $4014 OAM DMA (bus.rs routes to Ppu::oam_dma, copies 256 bytes from CPU RAM page through the 2KB mirror), sprite evaluation at cycle 257 (8 sprites per scanline, overflow flag bit 5, sprite-0 tracking), sprite pattern fetch at cycle 340 (8x8 via ctrl bit 3, 8x16 via tile id bit 0, V-flip with 7-row inversion, H-flip via reverse_bits, hi plane +8), per-sprite shifters with X countdown (decrement X until 0 then shift, cycles 1-257), and render_pixel compositing (bg vs sprite priority via attr bit 5, sprites use palettes 4-7, sprite-0 hit sets status bit 6 at cycle>=9). Fixed one integration bug: the old bg-only pipeline block was left in dot() duplicating the new combined block (shifters shifted/fetched twice per dot) - deleted the old block. ACCEPTANCE MET: Mario visible and the coin sprite animates across frames
 - Backup library mapper audit: mappers 0 (NROM) and 1 (MMC1) cover many games; still need mapper 2 (UNROM: Castlevania, Contra, Megaman 1), mapper 4 (MMC3: SMB2, SMB3, Lolo 2), mapper 7 (AOROM: Who Framed Roger Rabbit)
 - Finding: real commercial games rarely use illegal opcodes; none of the backup library needs them. Official-only CPU is sufficient for the goal of playing these games
 
@@ -58,13 +63,25 @@ Phases in order:
 1. NMI/IRQ servicing - DONE (see Done section). cpu_interrupts.nes verification deferred to the last milestone (needs APU frame-counter IRQ)
 2. PPU-1 rendering core - PARTIAL: 2a (palette/render) and 2b (registers/VRAM/open-bus) DONE (see Done section). Remaining: full background rendering in ONE merged step - timing + scroll + background pipeline together, no intermediate gating (we do not test until the full PPU exists). Acceptance: SMB title screen renders to PPM via main.rs run_cart. NEXT. Confirmed decisions: full dot-accurate scanline model, loopy v/t/x/w scroll system, Cartridge passed as parameter, open-bus tracking in the bus, hardware-accurate throughout (same philosophy as the CPU). Merged step:
 
-   2c. Full background renderer - IMPLEMENTED but BROKEN (grey screen, see Done section). Code complete and reviewed: timing, loopy scroll, background pipeline, fine-X, mirroring fix, transparent pixel fix, NMI forwarding fix all in. Acceptance NOT met yet: SMB renders uniform grey (framebuffer all 0x00). Diagnostics pending
+   2c. Full background renderer - DONE. Acceptance MET: SMB renders correctly (colors correct, no shift, no holes). Mario missing = sprites not yet implemented
-2d. CURRENT TASK - diagnose the grey screen. The framebuffer is all 0 (dark grey), meaning rendering is either never enabled or every pixel is transparent. Two-step plan:
+2d. DONE - diagnosis + fix. Built src/dump.rs dbg mode (state.txt + pctrace.txt per frame). Found 3 bugs (see Done section): unconditional NMI, attribute address/quadrant formulas, and pipeline timing (shift windows + cycle-257 preload). All fixed, verified by renders/ output
-   - Step 1 (quick inline probe): temporarily print per-frame state from run_cart - CPU PC (stuck loop?), PPU mask bit 3 (rendering enabled?), status, palette[0..4] (game wrote colors?), framebuffer non-zero pixel count (pipeline producing?). Needs small read accessors on Cpu (pc/a/x/y/sp/p) and Ppu (ctrl/mask/status/scanline/cycle/palette/framebuffer_nonzero) and NesBus (ram/ppu)
+3. NEXT: PPU-2 sprites + input + GUI window - makes SMB actually playable (Mario + enemies visible AND moveable). Detailed plan below. Four phases, sprites first (verifiable headless via PPM + dbg), then input logic (testable in isolation), then the window (first external deps):
-   - Step 2 (full dump module): new src/dump.rs writing machine state to text files, recreated fresh each run (clear + new files). Write debug_out/state.txt with CPU regs + PPU regs + palette + nametable + zero-page + framebuffer non-zero count, plus debug_out/pctrace.txt appending CPU PC each frame (identical lines = infinite loop). cargo run -- dbg <rom> mode clears debug_out/ and renders/, runs N frames, writes snapshot
+
-   - Expected outcomes: PC trace identical every frame = stuck loop (look at what the CPU polls); mask never gets bit 3 + palette stays zero = PPU writes not landing (check Ppu::write masking / vram_write routing); regs set but framebuffer 0 = pipeline/render_bg bug
+   3a. SPRITES - DONE (see Done section). Acceptance MET: Mario + coin sprite visible, coin animates
-   - After diagnosis, fix and re-run until SMB title screen appears in renders/frame_XXXX.ppm
+   3b. NEXT: INPUT (new src/input.rs, pure logic, testable without GUI):
-3. PPU-2 sprites + scrolling + $4014 OAM DMA: makes SMB actually playable
+       - InputSource trait (the ONLY new trait): fn buttons(&mut self) -> u8, NES button bit order. Impls: KeyboardInput (winit later), BotInput for automated tests (e.g. "hold right 120 frames" to verify scrolling headlessly)
+       - Controller struct (concrete, NOT a trait - fixed hardware protocol): holds Box<dyn InputSource>, strobe latch + shift register for $4016/$4017 semantics. read/write protocol implemented once here
+       - Keyboard defaults: Z=A, X=B, Shift=Select, Enter=Start, Arrows=D-pad. Pure mapping fn, unit-testable
+       - One Controller per player ($4016 = P1, $4017 = P2); SMB1 is single-controller, two-player support for SMB2 later
+
+   3c. BUS WIRING (src/bus.rs): add Controller(s) to NesBus, $4016 write = strobe, $4016/$4017 read = shift out one bit (open-bus for unused bits). Bus stays a trait; controller wiring lives in the concrete NesBus impl like everything else
+
+   3d. GUI WINDOW (src/render.rs + new deps): first external crates (egui + egui-winit + winit). main.rs owns the frame loop for ALL modes (nestest/blargg/cart/dbg/gui) - window is passive, renderer is dumb:
+       - Renderer trait gains two default no-op methods (PPM inherits them): fn should_close(&self) -> bool { false }, fn poll_input(&mut self, controller: &mut Controller) {}
+       - WindowRenderer: egui + winit impl of Renderer. present() converts palette indices -> RGB via index_to_rgb, uploads as texture, draws. poll_input() maps winit key events -> Controller bits. should_close() from winit close request
+       - PPM stays as headless/dbg output; renders/ no longer written for normal GUI runs (was ~1MB/frame, heavy)
+       - New run_gui <rom> mode: event pump -> one emulated frame -> present, 60fps
+       - Acceptance: walk Mario in the window, screen scrolls correctly between nametables (this is how scrolling gets verified - the scroll logic itself is already fixed and correct)
 4. GUI (egui + winit, first external deps): display framebuffer, 60fps loop, keyboard -> controller ($4016/$4017). First "games running on screen" moment
 5. Mappers: 2 (UNROM), 7 (AOROM) simple; 4 (MMC3) complex - includes scanline IRQ counter required for SMB3 status bar. Verified by running the real backups
 6. APU audio: 5 channels, frame counter -> IRQ, mixing. Verified with SMB
@@ -77,7 +94,8 @@ Blargg/illegal background: ~76 illegal opcodes in the nestest log (23 undocument
 ## Decisions
 
 - No external dependencies yet, keeping it pure std until the GUI milestone
-- GUI choice: egui + winit (decided, not yet used)
+- GUI choice: egui + winit (decided, not yet used); main.rs owns the frame loop for all modes, window is passive
+- InputSource is a trait (Box<dyn InputSource>) so input sources are pluggable: keyboard, gamepad, bot/test harness. Controller (shift-register protocol) is a concrete struct - the protocol is fixed hardware, only the button source varies
 - Scope: cartridge + CPU first, verified headless before graphics
 - Bus is a trait (impl Bus) so the CPU works against any memory layout (real bus, test bus, debug bus)
 - Flags stored as a raw u8 with named bit-mask constants (FLAG_CARRY etc.)

src/bus.rs

@@ -32,7 +32,7 @@ impl NesBus {
     pub fn begin_frame(&mut self) {
         self.ppu.begin_frame();
     }
-    
+
     pub fn ppu(&self) -> &Ppu { &self.ppu }
     pub fn ram(&self) -> &[u8; 0x800] { &self.ram }
 }
@@ -55,6 +55,7 @@ impl Bus for NesBus {
         match address {
             0x0000..=0x1FFF => self.ram[(address & 0x07FF) as usize] = value,
             0x2000..=0x3FFF => self.ppu.write(&mut self.cart, address, value),
+            0x4014 => self.ppu.oam_dma(&self.ram, value),
             0x6000..=0x7FFF => self.cart.write_prg_ram(address, value),
             0x8000..=0xFFFF => self.cart.write_prg(address, value),
             _ => {} // Everything else dropped for now

src/ppu.rs

@@ -1,12 +1,20 @@
 use crate::cartridge::Cartridge;
 use crate::mapper::Mirroring;
 
+// 4-byte OAM entry: Y, tile, attribute, X
+#[derive(Clone, Copy, Default)]
+struct Sprite {
+    y: u8,
+    tile: u8,
+    attr: u8,
+    x: u8,
+}
 pub struct Ppu {
-    ctrl: u8,          // $2000
+    ctrl: u8, // $2000
-    mask: u8,          // $2001
+    mask: u8, // $2001
-    status: u8,        // $2002
+    status: u8, // $2002
-    oamaddr: u8,       // $2003
+    oamaddr: u8, // $2003
-    oam: [u8; 256],    // $2004
+    oam: [u8; 256], // $2004
     scanline: u32,
     cycle: u32,
     framebuffer: [u8; 256 * 240],
@@ -24,10 +32,16 @@ pub struct Ppu {
     tile_latch: u8,
     attr_latch: u8,
     write_latch: bool, // shared W toggle for $2005/$2006, reset by $2002 read
-    vram_addr: u16,    // 14-bit
+    vram_addr: u16, // 14-bit
-    data_buffer: u8,   // $2007 read-buffer
+    data_buffer: u8, // $2007 read-buffer
     vram: [u8; 0x1000], // 4KB nametables (FourScreen support)
     palette: [u8; 32],
+    sprite_scanline: [Sprite; 8], // 8 sprites selected for the current scanline
+    sprite_count: u8, // how many were selected (up to 9 for overflow)
+    sprite_shifter_lo: [u8; 8], // per-sprite pattern bitplanes
+    sprite_shifter_hi: [u8; 8],
+    sprite_zero_hit_possible: bool, // sprite 0 is in this scanline's set
+    sprite_zero_being_rendered: bool, // sprite 0 has a non-transparent pixel here
 }
 
 impl Ppu {
@@ -50,9 +64,16 @@ impl Ppu {
             self.status &= !0x80;
         }
 
-        let rendering = self.mask & 0x08 != 0;
+        let rendering = self.mask & 0x18 != 0;   // bg OR sprites
 
-        // Background pipeline: visible + pre-render scanlines only
+        // Pre-render line start: clear sprite flags + shifters (reference: -1, cycle 1)
+        if self.scanline == 261 && self.cycle == 1 {
+            self.status &= !0x60;       // sprite overflow + sprite zero hit
+            self.sprite_shifter_lo = [0; 8];
+            self.sprite_shifter_hi = [0; 8];
+        }
+
+        // Background + sprite pipeline: visible + pre-render scanlines only
         if rendering && (self.scanline < 240 || self.scanline == 261) {
             // Shifters shift ONLY during the fetch windows (reference timing)
             if (self.cycle >= 2 && self.cycle < 258) || (self.cycle >= 321 && self.cycle < 338) {
@@ -60,6 +81,11 @@ impl Ppu {
                 self.bg_fetch(cart);
             }
 
+            // Sprite X countdown + shifter shifting (reference: cycle 1-257)
+            if self.cycle >= 1 && self.cycle < 258 {
+                self.sprite_shift();
+            }
+
             // End of visible scanline: advance vertical position
             if self.cycle == 256 && (self.scanline < 240 || self.scanline == 261) {
                 self.increment_vertical();
@@ -68,6 +94,7 @@ impl Ppu {
             if self.cycle == 257 && (self.scanline < 240 || self.scanline == 261) {
                 self.load_shifters();
                 self.copy_horizontal();
+                self.sprite_evaluate();   // pick sprites for the next scanline
             }
             // Idle nametable fetches at the end of the line
             if (self.cycle == 338 || self.cycle == 340) && (self.scanline < 240 || self.scanline == 261) {
@@ -77,11 +104,15 @@ impl Ppu {
             if self.scanline == 261 && self.cycle >= 280 && self.cycle < 305 {
                 self.copy_vertical();
             }
+            // Load the 8 selected sprites' patterns into shifters for the next scanline
+            if self.cycle == 340 && (self.scanline < 240 || self.scanline == 261) {
+                self.sprite_fetch_patterns(cart);
+            }
         }
 
         // Compose + store pixel (visible only, x = cycle - 1)
         if rendering && self.scanline < 240 && self.cycle >= 1 && self.cycle <= 256 {
-            self.render_bg_pixel(cart);
+            self.render_pixel(cart);   // was render_bg_pixel
         }
 
         // advance timing
@@ -112,6 +143,12 @@ impl Ppu {
             tile_latch: 0, attr_latch: 0,
             write_latch: false, vram_addr: 0, data_buffer: 0,
             vram: [0; 0x1000], palette: [0; 32],
+            sprite_scanline: [Sprite::default(); 8],
+            sprite_count: 0,
+            sprite_shifter_lo: [0; 8],
+            sprite_shifter_hi: [0; 8],
+            sprite_zero_hit_possible: false,
+            sprite_zero_being_rendered: false,
         }
     }
 
@@ -271,6 +308,107 @@ impl Ppu {
         }
     }
 
+    pub fn oam_dma(&mut self, ram: &[u8; 0x800], page: u8) {
+        for i in 0..256u16 {
+            let src = (((page as u16) << 8) + i) & 0x7FF;   // CPU RAM mirrors every 2KB
+            self.oam[i as usize] = ram[src as usize];
+        }
+    }
+
+    // Pick the up-to-8 sprites that overlap the NEXT scanline (cycle 257).
+    fn sprite_evaluate(&mut self) {
+        self.sprite_count = 0;
+        self.sprite_zero_hit_possible = false;
+        self.sprite_scanline = [Sprite::default(); 8];
+
+        // The scanline these sprites will be drawn on (pre-render -> scanline 0)
+        let target = if self.scanline == 261 { 0 } else { self.scanline as i32 + 1 };
+        let height: i32 = if self.ctrl & 0x20 != 0 { 16 } else { 8 };
+
+        let mut entry: usize = 0;
+        while entry < 64 && self.sprite_count < 9 {
+            let y = self.oam[entry * 4] as i32;
+            let diff = target - y;
+            if diff >= 0 && diff < height {
+                if self.sprite_count < 8 {
+                    if entry == 0 {
+                        self.sprite_zero_hit_possible = true;
+                    }
+                    let i = self.sprite_count as usize;
+                    self.sprite_scanline[i] = Sprite {
+                        y: self.oam[entry * 4],
+                        tile: self.oam[entry * 4 + 1],
+                        attr: self.oam[entry * 4 + 2],
+                        x: self.oam[entry * 4 + 3],
+                    };
+                }
+                self.sprite_count += 1;
+            }
+            entry += 1;
+        }
+        // sprite overflow flag (status bit 5)
+        self.status = (self.status & !0x20) | if self.sprite_count > 8 { 0x20 } else { 0 };
+    }
+
+    // Decrement sprite X until the sprite's screen position, then shift its pattern.
+    fn sprite_shift(&mut self) {
+        if self.mask & 0x10 == 0 { return; }   // sprite rendering disabled
+        for i in 0..(self.sprite_count.min(8) as usize) {
+            if self.sprite_scanline[i].x > 0 {
+                self.sprite_scanline[i].x -= 1;
+            } else {
+                self.sprite_shifter_lo[i] <<= 1;
+                self.sprite_shifter_hi[i] <<= 1;
+            }
+        }
+    }
+
+    // Fetch the pattern rows for the 8 selected sprites (cycle 340).
+    fn sprite_fetch_patterns(&mut self, cart: &mut Cartridge) {
+        let n = self.sprite_count.min(8) as usize;
+        for i in 0..n {
+            let s = self.sprite_scanline[i];
+            let row = (self.scanline as i32 + 1 - s.y as i32) & 0x0F;   // row within sprite (0-15)
+
+            let addr_lo: u16 = if self.ctrl & 0x20 == 0 {
+                // 8x8 sprite: pattern table from ctrl bit 3
+                let base = ((self.ctrl & 0x08) as u16) << 12;
+                let tile = s.tile as u16;
+                if s.attr & 0x80 != 0 {
+                    base | (tile << 4) | (7 - (row & 7)) as u16       // V-flipped
+                } else {
+                    base | (tile << 4) | (row & 7) as u16
+                }
+            } else {
+                // 8x16 sprite: pattern table from tile id bit 0, two tiles stacked
+                let table = (s.tile & 0x01) as u16;
+                let tile_even = (s.tile & 0xFE) as u16;
+                let row_lo = (row & 7) as u16;
+                if s.attr & 0x80 != 0 {
+                    // V-flipped: bottom tile first
+                    if row < 8 {
+                        (table << 12) | ((tile_even + 1) << 4) | (7 - row_lo)
+                    } else {
+                        (table << 12) | (tile_even << 4) | (7 - row_lo)
+                    }
+                } else if row < 8 {
+                    (table << 12) | (tile_even << 4) | row_lo
+                } else {
+                    (table << 12) | ((tile_even + 1) << 4) | row_lo
+                }
+            };
+
+            let mut lo = cart.read_chr(addr_lo);
+            let mut hi = cart.read_chr(addr_lo + 8);   // hi plane always +8
+            if s.attr & 0x40 != 0 {                    // H-flip
+                lo = lo.reverse_bits();
+                hi = hi.reverse_bits();
+            }
+            self.sprite_shifter_lo[i] = lo;
+            self.sprite_shifter_hi[i] = hi;
+        }
+    }
+
     fn increment_coarse_x(&mut self) {
         if self.vram_addr & 0x001F == 0x001F {      // coarse X wraps 31 -> 0
             self.vram_addr &= !0x001F;
@@ -306,22 +444,72 @@ impl Ppu {
         self.vram_addr = (self.vram_addr & !0x7BE0) | (self.t & 0x7BE0);
     }
 
-    fn render_bg_pixel(&mut self, _cart: &mut Cartridge) {
+    fn render_pixel(&mut self, _cart: &mut Cartridge) {
-        let bit_mux = 0x8000u16 >> self.x;
+        // ---- background pixel (mask bit 3) ----
-        let pattern_lo_bit = (self.bg_pattern_lo & bit_mux) != 0;
+        let mut bg_pixel = 0u8;
-        let pattern_hi_bit = (self.bg_pattern_hi & bit_mux) != 0;
+        let mut bg_palette = 0u8;
-        let attr_lo_bit = (self.bg_attr_lo & bit_mux) != 0;
+        if self.mask & 0x08 != 0 {
-        let attr_hi_bit = (self.bg_attr_hi & bit_mux) != 0;
+            let bit_mux = 0x8000u16 >> self.x;
+            let lo = (self.bg_pattern_lo & bit_mux) != 0;
+            let hi = (self.bg_pattern_hi & bit_mux) != 0;
+            let al = (self.bg_attr_lo & bit_mux) != 0;
+            let ah = (self.bg_attr_hi & bit_mux) != 0;
+            bg_pixel = (hi as u8) << 1 | lo as u8;
+            bg_palette = (ah as u8) << 1 | al as u8;
+        }
 
-        let pixel = (pattern_hi_bit as u8) << 1 | pattern_lo_bit as u8;
+        // ---- foreground (sprite) pixel (mask bit 4) ----
-        let palette_bits = (attr_hi_bit as u8) << 1 | attr_lo_bit as u8;
+        let mut fg_pixel = 0u8;
+        let mut fg_palette = 0u8;
+        let mut fg_priority = false;
+        self.sprite_zero_being_rendered = false;
+        if self.mask & 0x10 != 0 {
+            for i in 0..(self.sprite_count.min(8) as usize) {
+                if self.sprite_scanline[i].x == 0 {
+                    let lo = (self.sprite_shifter_lo[i] & 0x80) != 0;
+                    let hi = (self.sprite_shifter_hi[i] & 0x80) != 0;
+                    let pixel = (hi as u8) << 1 | lo as u8;
+                    if pixel != 0 {
+                        fg_pixel = pixel;
+                        fg_palette = (self.sprite_scanline[i].attr & 0x03) + 0x04;  // sprites use palettes 4-7
+                        fg_priority = self.sprite_scanline[i].attr & 0x20 == 0;     // bit5=0 -> in front
+                        if i == 0 {
+                            self.sprite_zero_being_rendered = true;
+                        }
+                        break;   // first non-transparent sprite wins (priority order)
+                    }
+                }
+            }
+        }
 
+        // ---- composite ----
+        let (pixel, palette) = match (bg_pixel, fg_pixel) {
+            (0, 0) => (0, 0),
+            (0, f) => (f, fg_palette),
+            (b, 0) => (b, bg_palette),
+            (b, f) if fg_priority => (f, fg_palette),
+            (b, _) => (b, bg_palette),
+        };
+
+        // ---- sprite zero hit (status bit 6) ----
+        if self.sprite_zero_hit_possible
+            && self.sprite_zero_being_rendered
+            && bg_pixel != 0
+            && fg_pixel != 0
+            && self.mask & 0x08 != 0
+            && self.mask & 0x10 != 0
+        {
+            if self.cycle >= 9 && self.cycle < 258 {   // skip leftmost 8px, both enabled
+                self.status |= 0x40;
+            }
+        }
+
+        // ---- write framebuffer ----
         let color = if pixel == 0 {
             self.palette[0]
         } else {
-            self.palette[(palette_bits * 4 + pixel) as usize]
+            self.palette[(palette * 4 + pixel) as usize]
         };
-
         let idx = (self.scanline as usize) * 256 + (self.cycle as usize - 1);
         self.framebuffer[idx] = color;
     }