Fix split Y scrolling

Makefile

@@ -1,11 +1,12 @@
 CC = gcc
 LD = $(CC)
 PFLAGS = -g
+#PFLAGS = -O3
 #PFLAGS += -DDEBUG_MAPPER
 #PFLAGS += -DDEBUG_RENDER
-#PFLAGS += -DDEBUG_PPU
+#PFLAGS += -DDEBUG_PPU -DDEBUG_VRAM
 #PFLAGS += -DE6502_DEBUG
-CFLAGS = $(PFLAGS) -Wall -Werror -Wshadow -I..
+CFLAGS = $(PFLAGS) -Wall -Werror -Wshadow -Wunused -I..
 LDFLAGS = $(PFLAGS)
 
 OBJDIR = obj

src/ppu.c

@@ -64,17 +64,38 @@ uint8_t nes_ppu_read(nes_ppu* ppu, uint16_t addr) {
                      32 : 1;
     }
 
-//    fprintf(stdout, "PPU: <-R $%04x %02x\n", addr, val);
+    PPU_LOG("PPU: <-R $%04x %02x\n", addr, val);
 
     return val;
 }
 
+static inline void nes_ppu_internal_copy_x(nes_ppu* ppu) {
+    ppu->control &= ~(1U);
+    ppu->control |= ((ppu->t >> 10) & 1U);
+
+    ppu->scroll_x = ((ppu->t & 0b11111U) << 3) |
+                    (ppu->x & 0b111U);
+}
+
+static inline void nes_ppu_internal_copy_y(nes_ppu* ppu) {
+    // Copy t to v (decoded into scroll_x, scroll_y, ctrl)
+
+    ppu->control &= ~(0b10U);
+    ppu->control |= ((ppu->t >> 10) & 0b10U);
+
+    ppu->scroll_y = ((ppu->t & 0x03E0U) >> 2) |
+                    ((ppu->t & 0x7000U) >> 12);
+}
+
 void nes_ppu_write(nes_ppu* ppu, uint16_t addr, uint8_t val) {
-//    fprintf(stdout, "PPU: W-> $%04x %02x\n", addr, val);
+    PPU_LOG("PPU: W-> $%04x %02x\n", addr, val);
 
     if (ppu_reg_ctrl == addr) {
         PPU_LOG("PPU: CTRL %02x\n", val);
-        ppu->control = val;
+        ppu->control &= ppu_Control_Nametable_Mask;
+        ppu->control |= (val & ~ppu_Control_Nametable_Mask);
+        ppu->t &= ~(0xC00U);
+        ppu->t |= (uint16_t)(val & ppu_Control_Nametable_Mask) << 10;
 
     } else if (oam_reg_addr == addr) {
         OAM_LOG("PPU: OAM ADDR %02x\n", val);
@@ -85,34 +106,53 @@ void nes_ppu_write(nes_ppu* ppu, uint16_t addr, uint8_t val) {
         ((uint8_t*)ppu->oam)[ppu->oam_addr++] = val;
 
     } else if (ppu_reg_mask == addr) {
+        PPU_LOG("PPU: Mask %02x\n", val);
         ppu->mask = val;
 
     } else if (ppu_reg_scroll == addr) {
         if (ppu->latch) {
             PPU_LOG("PPU: Scroll Y %02x\n", val);
-            ppu->scroll_y = val;
+//            ppu->scroll_y = val;
+            ppu->t &= 0b0000110000011111U;
+            ppu->t |= (uint16_t)(val & 0b00000111U) << 12;
+            ppu->t |= (uint16_t)(val & 0b11111000U) << 2;
+
         } else {
             PPU_LOG("PPU: Scroll X %02x\n", val);
-            ppu->scroll_x = val;
+//            ppu->scroll_x = val;
+            ppu->t &= ~(0b11111U);
+            ppu->t |= (val & 0b11111000U) >> 3;
+            ppu->x = (val & 0b111U);
         }
         ppu->latch = !ppu->latch;
 
     } else if (ppu_reg_addr == addr) {
-        if (ppu->latch) {
-            ppu->addr &= 0x3F00U;
-            ppu->addr |= val;
-            VRAM_LOG("PPU: VRAM ADDR %04x\n", ppu->addr);
+        VRAM_LOG("PPU: ADDR %02x\n", val);
 
+        if (ppu->latch) {
+            ppu->t &= 0xFF00U;
+            ppu->t |= val;
+/*
             // Take advantage of the quick split quirk
             ppu->scroll_x &= 0b00000111;
             ppu->scroll_x |= (val & 0b00011111);
             ppu->scroll_y &= 0b11000111;
             ppu->scroll_y |= (val & 0b11100000) >> 2;
+*/
+            ppu->addr = (ppu->t & 0x3FFFU);
+            VRAM_LOG("PPU: VRAM ADDR %04x\n", ppu->addr);
 
-        } else {
-            ppu->addr &= 0x00FFU;
-            ppu->addr |= (uint16_t)val << 8;
+            nes_ppu_internal_copy_x(ppu);
+            nes_ppu_internal_copy_y(ppu);
+
+            PPU_LOG("PPU: Scroll N, X, Y = %d, %d, %d\n",
+                    ppu->control & ppu_Control_Nametable_Mask,
+                    ppu->scroll_x, ppu->scroll_y);
 
+        } else {
+            ppu->t &= 0x00FFU;
+            ppu->t |= (uint16_t)(0x3FU & val) << 8;
+/*
             // Take advantage of the quick split quirk
             ppu->control &= ~ppu_Control_Nametable_Mask;
             ppu->control |= (val & 0b1100) >> 2;
@@ -120,6 +160,9 @@ void nes_ppu_write(nes_ppu* ppu, uint16_t addr, uint8_t val) {
             ppu->scroll_y |= (val & 0b11) << 6;
             ppu->scroll_y |= (val & 0b110000) >> 4;
 
+            PPU_LOG("PPU: Scroll X, Y = %d, %d\n",
+                    ppu->scroll_x, ppu->scroll_y);
+*/
         }
         ppu->latch = !ppu->latch;
 
@@ -189,6 +232,7 @@ int nes_ppu_init(nes_ppu* ppu, const nes_cart* cart) {
     ppu->status   = 0;
     ppu->oam_addr = 0;
     ppu->addr     = 0;
+    ppu->t        = 0;
     nes_ppu_reset(ppu);
     return 0;
 }
@@ -198,6 +242,14 @@ int nes_ppu_run(nes_ppu* ppu, int cycles) {
 
     int next_cycle = ppu->cycle + cycles;
 
+    if (     ppu->scanline < nes_ppu_render &&
+            ppu->cycle < 257 && next_cycle >= 257) {
+        nes_ppu_internal_copy_x(ppu);
+        if (ppu->scanline == 0) {
+            nes_ppu_internal_copy_y(ppu);
+        }
+    }
+
     if (    NULL != ppu->mapper->scanline &&
             ppu->scanline < nes_ppu_render &&
             (ppu->mask & (ppu_Mask_Back | ppu_Mask_Sprite)) &&

src/ppu.h

@@ -138,9 +138,12 @@ typedef struct {
     uint8_t control;
     uint8_t mask;
     uint8_t status;
+    uint16_t t;
+    uint8_t x;
+    // We decode scroll_x and scroll_y in lieu of v
     uint8_t scroll_x;
     uint8_t scroll_y;
-    uint16_t addr;
+    uint16_t addr; // Not quite the same as internal register t
     uint8_t data;
     uint8_t oam_addr;
 

src/sdl_render.c

@@ -324,17 +324,17 @@ static inline void render_bg_scanline_area(
     }
 }
 
-static void render_bg_scanline(const nes_ppu* ppu, int scanline,
+static void render_bg_scanline(const nes_ppu* ppu,/* int scanline,*/
                                uint8_t* dst) {
     int page = (ppu->control & ppu_Control_Nametable_Mask);
     int x = ppu->scroll_x;
-    int y = ppu->scroll_y + scanline;
-
+    int y = ppu->scroll_y /*+ scanline*/;
+/*
     if (y >= nes_ppu_render_h) {
         y -= nes_ppu_render_h;
         page ^= 0b10;
     }
-
+*/
     int w = (nes_ppu_render_w - x);
     if (!(ppu->mask & ppu_Mask_Left_Back)) {
         // Handle column 0 flag - need to fill with transparency
@@ -413,32 +413,26 @@ static void render_sprite(nes_ppu* ppu, int index,
 */
 
 static void render_line_sprites(nes_ppu* ppu, uint8_t* dst_line,
-                                int scanline, int background) {
-    int bank = (ppu->control & ppu_Control_Sprite_Bank) ?
-               0x100 : 0;
-    for (   int i_sprite = nes_ppu_oam_sprite_count - 1;
-            i_sprite >= 0; --i_sprite) {
-        const oam_sprite* sprite = &ppu->oam[i_sprite];
+                                int scanline, int background,
+                                const oam_sprite* sprites,
+                                int n_sprites) {
+    for (int i_sprite = n_sprites - 1; i_sprite >= 0; --i_sprite) {
+        const oam_sprite* sprite = &sprites[i_sprite];
+
         if ((sprite->attr & oam_Attr_Background) ^ background) {
             continue;
         }
-        if (    !(ppu->mask & ppu_Mask_Left_Sprite) &&
-                sprite->x < 8) {
-            continue;
-        }
-        int y_pos = (sprite->y + 1);
-        int y = scanline - y_pos;
-        if (0 > y) continue;
-        int h = (ppu->control & ppu_Control_Sprite_Size) ? 16 : 8;
-        if (y >= h) continue;
 
         int index = sprite->index;
+        int bank = (ppu->control & ppu_Control_Sprite_Bank) ?
+                   0x100 : 0;
         if (ppu->control & ppu_Control_Sprite_Size) {
             bank = (index & 1) ? 0x100 : 0;
             index &= 0xFEU;
         }
         index += bank;
 
+        int y = scanline - (sprite->y + 1);
         if (ppu->control & ppu_Control_Sprite_Size) {
             if (y >= 8) {
                 index ^= 1;
@@ -455,12 +449,19 @@ static void render_line_sprites(nes_ppu* ppu, uint8_t* dst_line,
         if (sprite->attr & oam_Attr_Flip_Y) y = 7 - y;
         int end = nes_ppu_render_w - sprite->x;
         if (end > 8) end = 8;
+        int start = 0;
+        if (    !(ppu->mask & ppu_Mask_Left_Sprite) &&
+                sprite->x < 8) {
+            start = 8 - sprite->x;
+        }
         if (sprite->attr & oam_Attr_Flip_X) {
             render_sprite_line_flip(ppu, index, y, pal,
-                                    dst_line + sprite->x, 0, end);
+                                    dst_line + sprite->x + start,
+                                    start, end);
         } else {
             render_sprite_line(ppu, index, y, pal,
-                               dst_line + sprite->x, 0, end);
+                               dst_line + sprite->x + start,
+                               start, end);
         }
     }
 }
@@ -711,8 +712,48 @@ static void update_scanline_hit(nes_ppu* ppu, uint8_t* back_line,
     }
 }
 
+static int select_line_sprites(const nes_ppu* ppu, int scanline,
+                               oam_sprite* sprites, int max) {
+    int n_sprites = 0;
+
+    for (   int i_sprite = 0;
+            i_sprite < nes_ppu_oam_sprite_count && n_sprites < max;
+            ++i_sprite) {
+        const oam_sprite* sprite = &ppu->oam[i_sprite];
+        int y_pos = (sprite->y + 1);
+        int y = scanline - y_pos;
+        if (0 > y) continue;
+        int h = (ppu->control & ppu_Control_Sprite_Size) ? 16 : 8;
+        if (y >= h) continue;
+
+        *sprites = *sprite;
+        ++sprites;
+        ++n_sprites;
+    }
+
+    return n_sprites;
+}
+
 static void render_scanline(nes_ppu* ppu, int line,
                             sdl_render_data* data) {
+
+    SDL_Rect dst_rect = {
+        .x = 0,
+        .y = line,
+        .w = nes_ppu_render_w,
+        .h = 1,
+    };
+
+    if (line >= 0) {
+        // Emulate the happy part of the backdrop override quirk
+        int pal_idx = (ppu->addr >= nes_ppu_mem_pal_start) ?
+                      (ppu->addr & (nes_ppu_mem_pal_size - 1)) : 0;
+        SDL_Color ext = nes_palette[ppu->palette[pal_idx]];
+        SDL_FillRect(data->target, &dst_rect, ((int)ext.r << 16) |
+                                              ((int)ext.g << 8) |
+                                              ext.b);
+    }
+
     if (!(ppu->mask & (ppu_Mask_Sprite | ppu_Mask_Back))) {
         // Do nothing if BOTH are disabled.
         return;
@@ -725,40 +766,58 @@ static void render_scanline(nes_ppu* ppu, int line,
         .h = 1,
     };
 
-    SDL_Rect dst_rect = {
-        .x = 0,
-        .y = line,
-        .w = nes_ppu_render_w,
-        .h = 1,
-    };
-
     uint8_t* foreground = data->foreground->pixels;
     uint8_t* background = data->background->pixels;
 
-    if (ppu->mask & ppu_Mask_Sprite) {
-        memset(foreground, 0xFFU, nes_ppu_render_w);
-        render_line_sprites(ppu, foreground, line,
-                            oam_Attr_Background);
-        SDL_BlitSurface(data->foreground, &src_rect,
-                        data->target, &dst_rect);
-    }
 
     // We check for hits if EITHER layer is enabled.
-    render_bg_scanline(ppu, line, background);
+    render_bg_scanline(ppu, background);
     if (ppu->hit_line <= 0) {
         update_scanline_hit(ppu, background, line);
     }
 
-    if (ppu->mask & ppu_Mask_Back) {
-        SDL_BlitSurface(data->background, &src_rect,
-                        data->target, &dst_rect);
+
+    if (line >= 0) {
+        oam_sprite line_sprites[8] = {0};
+        int n_sprites = select_line_sprites(ppu, line,
+                                            line_sprites, 8);
+
+        if (ppu->mask & ppu_Mask_Sprite) {
+            memset(foreground, 0xFFU, nes_ppu_render_w);
+            render_line_sprites(ppu, foreground, line,
+                                oam_Attr_Background,
+                                line_sprites, n_sprites);
+            SDL_BlitSurface(data->foreground, &src_rect,
+                            data->target, &dst_rect);
+        }
+
+        if (ppu->mask & ppu_Mask_Back) {
+            SDL_BlitSurface(data->background, &src_rect,
+                            data->target, &dst_rect);
+        }
+
+        if (ppu->mask & ppu_Mask_Sprite) {
+            memset(foreground, 0xFFU, nes_ppu_render_w);
+            render_line_sprites(ppu, foreground, line, 0,
+                                line_sprites, n_sprites);
+            SDL_BlitSurface(data->foreground, &src_rect,
+                            data->target, &dst_rect);
+        }
     }
 
-    if (ppu->mask & ppu_Mask_Sprite) {
-        memset(foreground, 0xFFU, nes_ppu_render_w);
-        render_line_sprites(ppu, foreground, line, 0);
-        SDL_BlitSurface(data->foreground, &src_rect,
-                        data->target, &dst_rect);
+    /*if (line + 1 < nes_ppu_height)*/ {
+        ppu->scroll_y++;
+        if (ppu->scroll_y >= nes_ppu_render_h) {
+            ppu->scroll_y -= nes_ppu_render_h;
+            ppu->control ^= 0b10;
+        }
+/*
+        // We check for hits if EITHER layer is enabled.
+        render_bg_scanline(ppu, background);
+        if (ppu->hit_line <= 0) {
+            update_scanline_hit(ppu, background, line + 1);
+        }
+*/
     }
 }
 
@@ -768,14 +827,6 @@ static int sdl_render(nes_Renderer* rend, nes_ppu* ppu) {
 
     if (ppu->scanline < nes_ppu_prerender) {
         REND_LOG("Scanline %3d -> Prerender\n", ppu->scanline);
-
-        // Emulate the happy part of the backdrop override quirk
-        int pal_idx = (ppu->addr >= nes_ppu_mem_pal_start) ?
-                      (ppu->addr & (nes_ppu_mem_pal_size - 1)) : 0;
-        SDL_Color ext = nes_palette[ppu->palette[pal_idx]];
-        SDL_FillRect(data->target, NULL, ((int)ext.r << 16) |
-                                         ((int)ext.g << 8) |
-                                         ext.b);
 /*
         memset(data->foreground->pixels, 0xFFU,
                data->foreground->pitch * data->foreground->h);
@@ -787,6 +838,10 @@ static int sdl_render(nes_Renderer* rend, nes_ppu* ppu) {
             render_sprites(ppu, data->sprite, data->foreground, 0);
         }
 */
+/*
+        int line = ppu->scanline - (int)nes_ppu_prerender;
+        render_scanline(ppu, line, data);
+*/
 
     } else if (ppu->scanline < nes_ppu_prerender +
                                nes_ppu_height) {
@@ -800,7 +855,10 @@ static int sdl_render(nes_Renderer* rend, nes_ppu* ppu) {
             render_block_line(ppu, line, data);
         }
 */
-        REND_LOG("Scanline %3d : X %d Y %d\n", ppu->scanline, ppu->scroll_x, ppu->scroll_y);
+        REND_LOG("Scanline %3d : N %d X %d Y %d\n",
+                 ppu->scanline,
+                 ppu->control & ppu_Control_Nametable_Mask,
+                 ppu->scroll_x, ppu->scroll_y);
         int line = ppu->scanline - (int)nes_ppu_prerender;
         render_scanline(ppu, line, data);