Merge pull request #2 from andrew-hoffman/accurate-mmc3

Accurate mmc3

.gitignore

@@ -26,3 +26,5 @@ nesdebug*.txt
 !retrace.jar
 
 
+/dist/
+/HalfNES.exe

changelog.txt

@@ -1,5 +1,22 @@
 HalfNES Version Notes:
 
+057 (7/30/2014)
+-PPU is now pixel accurate. Meaning that many glitchy games are now completely
+fixed. However it also means that performance is worse.
+-MMC3 and MMC5 scanline counters have been rewritten to be more accurate.
+Fixed games:
+-Marble Madness
+-Laser Invasion
+-Mickey's Adventures in Letterland
+-Rad Racer
+-Slalom
+-Tiny Toon Adventures (the status bar was off by one scan line before)
+-Mega Man 3 (displays same glitchy lines as real hardware)
+-Every Namco 163 game (scanline counter was broken)
+
+Also passes several more of blargg's PPU and MMC3 tests.
+I haven't fixed all of the MMC3 based multicart mappers yet.
+
 056 (7/17/2014)
 -Multithreaded NTSC filter for better performance
 -Rewrote CPU/PPU timing entirely (passes many timing tests that used to fail)

src/com/grapeshot/halfnes/PPU.java

@@ -18,8 +18,8 @@ public class PPU {
     private int loopyV = 0x0;//ppu memory pointer
     private int loopyT = 0x0;//temp pointer
     private int loopyX = 0;//fine x scroll
-    private int scanline = 0;
-    private int cycles = 0;
+    public int scanline = 0;
+    public int cycles = 0;
     private int framecount = 0;
     private int div = 2;
     private final int[] OAM = new int[256], spriteshiftregH = new int[8],
@@ -186,14 +186,12 @@ public final void write(final int regnum, final int data) {
                     loopyT += data;
                     loopyV = loopyT;
                     even = true;
-
                 }
                 break;
             case 7:
                 // PPUDATA
                 mapper.ppuWrite((loopyV & 0x3fff), data);
                 loopyV += vraminc;
-                // increments on write but NOT on read
                 break;
             default:
                 break;
@@ -205,7 +203,7 @@ public final void write(final int regnum, final int data) {
      *
      * @return true
      */
-    private boolean ppuIsOn() {
+    public boolean ppuIsOn() {
         return getbit(ppuregs[1], 3) || getbit(ppuregs[1], 4);
     }
 
@@ -268,12 +266,20 @@ public final void clock() {
                 //clear the oam address from pxls 257-341 continuously
                 ppuregs[3] = 0;
             }
-//          if ((cycles == 340)) {
-//              //read the same nametable byte twice
-//              //this signals the MMC5 to increment the scanline counter
-//              fetchNTByte();
-//              fetchNTByte();
-//          }
+            if ((cycles == 340) && ppuIsOn()) {
+                //read the same nametable byte twice
+                //this signals the MMC5 to increment the scanline counter
+                fetchNTByte();
+                fetchNTByte();
+            }
+            if (cycles == 260 && ppuIsOn()) {
+                //evaluate sprites for NEXT scanline (as long as either background or sprites are enabled)
+                //this does in fact happen on scanine 261 but it doesn't do anything useful
+                //it's cycle 260 because that's when the first important sprite byte is read
+                //actually sprite overflow should be set by sprite eval somewhat before
+                //so this needs to be split into 2 parts, the eval and the data fetches
+                evalSprites();
+            }
             if (scanline == 261) {
                 if (cycles == 0) {// turn off vblank, sprite 0, sprite overflow flags
                     setvblankflag(false);
@@ -287,16 +293,22 @@ public final void clock() {
             //handle vblank on / off
             setvblankflag(true);
         }
+        if (!ppuIsOn() || (scanline > 240 && scanline < 261)) {
+            //HACK ALERT
+            //handle the case of MMC3 mapper watching A12 toggle
+            //even when read or write aren't asserted on the bus
+            //needed to pass Blargg's mmc3 tests
+            mapper.checkA12(loopyV & 0x3fff);
+        }
         if (scanline < 240) {
             if (cycles >= 1 && cycles <= 256) {
                 int bufferoffset = (scanline << 8) + (cycles - 1);
                 //bg drawing
                 if (getbit(ppuregs[1], 3)) { //if background is on, draw a line of that
                     final boolean isBG = drawBGPixel(bufferoffset);
                     //sprite drawing
-                    if (found > 0) {
-                        drawSprites(scanline << 8, cycles - 1, isBG);
-                    }
+                    drawSprites(scanline << 8, cycles - 1, isBG);
+
                 } else {
                     //rendering is off, so draw either the background color OR
                     //if the PPU address points to the palette, draw that color instead.
@@ -311,12 +323,6 @@ public final void clock() {
                 final int emph = (ppuregs[1] & 0xe0) << 1;
                 bitmap[bufferoffset] = bitmap[bufferoffset] & 0x3f | emph;
 
-            } else if (cycles == 257) {
-                //evaluate sprites for NEXT scanline (as long as either background or sprites are enabled)
-                //this does not happen on scanine 261
-                if (ppuIsOn()) {
-                    evalSprites();
-                }
             }
         }
         //handle nmi
@@ -352,10 +358,7 @@ private void bgFetch() {
         //background fetches
         switch ((cycles - 1) & 7) {
             case 1:
-                //fetch nt byte
-                tileAddr = mapper.ppuRead(
-                        ((loopyV & 0xc00) | 0x2000) + (loopyV & 0x3ff)) * 16 
-                        + (bgpattern ? 0x1000 : 0);
+                fetchNTByte();
                 break;
             case 3:
                 //fetch attribute (FIX MATH)
@@ -412,6 +415,13 @@ private void bgFetch() {
         }
     }
 
+    private void fetchNTByte() {
+        //fetch nt byte
+        tileAddr = mapper.ppuRead(
+                ((loopyV & 0xc00) | 0x2000) + (loopyV & 0x3ff)) * 16
+                + (bgpattern ? 0x1000 : 0);
+    }
+
     private boolean drawBGPixel(int bufferoffset) {
         //background drawing
         //loopyX picks bits
@@ -493,30 +503,38 @@ private void evalSprites() {
                 }
                 //get tile address (8x16 sprites can use both pattern tbl pages but only the even tiles)
                 final int tilenum = OAM[spritestart + 1];
-                if (spritesize) {
-                    tilefetched = ((tilenum & 1) * 0x1000)
-                            + (tilenum & 0xfe) * 16;
-                } else {
-                    tilefetched = tilenum * 16
-                            + ((sprpattern) ? 0x1000 : 0);
-                }
-                tilefetched += offset;
-                //now load up the shift registers for said sprite
-                final boolean hflip = getbit(oamextra, 6);
-                if (!hflip) {
-                    spriteshiftregL[found] = reverseByte(mapper.ppuRead(tilefetched));
-                    spriteshiftregH[found] = reverseByte(mapper.ppuRead(tilefetched + 8));
-                } else {
-                    spriteshiftregL[found] = mapper.ppuRead(tilefetched);
-                    spriteshiftregH[found] = mapper.ppuRead(tilefetched + 8);
-                }
+                spriteFetch(spritesize, tilenum, offset, oamextra);
                 ++found;
             }
         }
         for (int i = found; i < 8; ++i) {
             //fill unused sprite registers with zeros
             spriteshiftregL[found] = 0;
             spriteshiftregH[found] = 0;
+            //also, we need to do 8 reads no matter how many sprites we found
+            //dummy reads are to sprite 0xff
+            spriteFetch(spritesize, 0xff, 0, 0);
+        }
+    }
+
+    private void spriteFetch(final boolean spritesize, final int tilenum, int offset, final int oamextra) {
+        int tilefetched;
+        if (spritesize) {
+            tilefetched = ((tilenum & 1) * 0x1000)
+                    + (tilenum & 0xfe) * 16;
+        } else {
+            tilefetched = tilenum * 16
+                    + ((sprpattern) ? 0x1000 : 0);
+        }
+        tilefetched += offset;
+        //now load up the shift registers for said sprite
+        final boolean hflip = getbit(oamextra, 6);
+        if (!hflip) {
+            spriteshiftregL[found] = reverseByte(mapper.ppuRead(tilefetched));
+            spriteshiftregH[found] = reverseByte(mapper.ppuRead(tilefetched + 8));
+        } else {
+            spriteshiftregL[found] = mapper.ppuRead(tilefetched);
+            spriteshiftregH[found] = mapper.ppuRead(tilefetched + 8);
         }
     }
 

src/com/grapeshot/halfnes/mappers/FME7Mapper.java

@@ -131,11 +131,11 @@ public final void cartWrite(final int addr, final int data) {
                     }
                     break;
                 case 0xe:
-                    irqcounter &= 0xff;
+                    irqcounter &= 0xff00;
                     irqcounter |= data;
                     break;
                 case 0xf:
-                    irqcounter &= 0xff00;
+                    irqcounter &= 0xff;
                     irqcounter |= (data << 8);
                     break;
             }

src/com/grapeshot/halfnes/mappers/MMC3Mapper.java

@@ -12,16 +12,16 @@
  */
 public class MMC3Mapper extends Mapper {
 
-    private int whichbank = 0;
-    private boolean prgconfig = false;
-    private boolean chrconfig = false;
-    private int irqctrreload = 0;
-    private int irqctr = 0;
-    private boolean irqenable = false;
-    private boolean irqreload = false;
-    private int bank6 = 0;
-    private int[] chrreg = {0, 0, 0, 0, 0, 0};
-    private boolean interrupted = false;
+    protected int whichbank = 0;
+    protected boolean prgconfig = false;
+    protected boolean chrconfig = false;
+    protected int irqctrreload = 0;
+    protected int irqctr = 0;
+    protected boolean irqenable = false;
+    protected boolean irqreload = false;
+    protected int bank6 = 0;
+    protected int[] chrreg = {0, 0, 0, 0, 0, 0};
+    protected boolean interrupted = false;
 
     @Override
     public void loadrom() throws BadMapperException {
@@ -39,13 +39,14 @@ public void loadrom() throws BadMapperException {
     }
 
     @Override
-    public final void cartWrite(int addr, int data) {
+    public void cartWrite(int addr, int data) {
         if (addr < 0x8000 || addr > 0xffff) {
             super.cartWrite(addr, data);
             return;
         }
         //bankswitches here
         //different register for even/odd writes
+        //System.err.println("mmc3 write " + utils.hex(addr) + " " + utils.hex(data));
         if (utils.getbit(addr, 0)) {
             //odd registers
             if ((addr >= 0x8000) && (addr <= 0x9fff)) {
@@ -69,7 +70,6 @@ public final void cartWrite(int addr, int data) {
             } else if ((addr >= 0xc000) && (addr <= 0xdfff)) {
                 //any value here reloads irq counter
                 irqreload = true;
-
             } else if ((addr >= 0xe000) && (addr <= 0xffff)) {
                 //iany value here enables interrupts
                 irqenable = true;
@@ -95,20 +95,18 @@ public final void cartWrite(int addr, int data) {
             } else if ((addr >= 0xc000) && (addr <= 0xdfff)) {
                 //value written here used to reload irq counter _@ end of scanline_
                 irqctrreload = data;
-                irqreload = true;
             } else if ((addr >= 0xe000) && (addr <= 0xffff)) {
                 //any value here disables IRQ and acknowledges
                 if (interrupted) {
                     --cpu.interrupt;
                 }
                 interrupted = false;
                 irqenable = false;
-                irqctr = irqctrreload;
             }
         }
     }
 
-    private void setupchr() {
+    protected void setupchr() {
         if (chrconfig) {
 
             setppubank(1, 0, chrreg[2]);
@@ -130,7 +128,7 @@ private void setupchr() {
         }
     }
 
-    private void setbank6() {
+    protected void setbank6() {
         if (!prgconfig) {
             //map c000-dfff to last bank, 8000-9fff to selected bank
             for (int i = 0; i < 8; ++i) {
@@ -146,38 +144,63 @@ private void setbank6() {
         }
     }
 
+    private boolean lastA12 = false;
+
     @Override
-    public void notifyscanline(int scanline) {
-        //Scanline counter
-        if (scanline > 239 && scanline != 261) {
-            //clocked on LAST line of vblank and all lines of frame. Not on 240.
-            return;
-        }
-        if (!ppu.mmc3CounterClocking()) {
-            return;
+    public int ppuRead(int addr) {
+        //note: to pass blargg's mmc3 tests the vram address is read
+        //in a loop while the PPU is not rendering
+        //actually the read signal is not asserted then
+        //but I have no other way to call into the mapper code when
+        //the address changes.
+        checkA12(addr);
+        return super.ppuRead(addr);
+    }
+
+    @Override
+    public void ppuWrite(int addr, int data) {
+        checkA12(addr);
+        super.ppuWrite(addr, data);
+    }
+
+    int a12timer = 0;
+    int prevcpuclocks = 0;
+
+    @Override
+    public void checkA12(int addr) {
+        boolean a12 = utils.getbit(addr, 12);
+        if (a12 && (!lastA12)) {
+            //rising edge
+            if ((a12timer <= 0)) {
+                clockScanCounter();
+            }
+        } else if (!a12 && lastA12) {
+            //falling edge
+            a12timer = 8;
         }
+        --a12timer;
+        lastA12 = a12;
+    }
 
-        if (irqreload) {
-            irqreload = false;
+    private void clockScanCounter() {
+        if (irqreload || (irqctr == 0)) {
+            //System.err.println(ppu.scanline + "reloading" + irqctrreload);
             irqctr = irqctrreload;
+            irqreload = false;
+        } else {
+            --irqctr;
         }
-
-        if (irqctr-- <= 0) {
-            if (irqctrreload == 0) {
-                return;
-                //irqs stop being generated if reload set to zero
-            }
-            if (irqenable) {
-                if (!interrupted) {
-                    ++cpu.interrupt;
-                    interrupted = true;
-                }
+        if ((irqctr == 0) && irqenable) {
+            if (!interrupted) {
+                ++cpu.interrupt;
+                interrupted = true;
+                //System.err.println("interrupt line " + ppu.scanline + " reload " + irqctrreload);
             }
-            irqctr = irqctrreload;
         }
+
     }
 
-    private void setppubank(int banksize, int bankpos, int banknum) {
+    protected void setppubank(int banksize, int bankpos, int banknum) {
 //        System.err.println(banksize + ", " + bankpos + ", "+ banknum);
         for (int i = 0; i < banksize; ++i) {
             chr_map[i + bankpos] = (1024 * ((banknum) + i)) % chrsize;