From 014a89e4f6bc520d7fa31abe7144e66a4890ac52 Mon Sep 17 00:00:00 2001 From: HarianthK Date: Mon, 14 Sep 2026 14:30:24 -0700 Subject: [PATCH] Fix the quirked jump address, wrap the sprite start when clipping, and make Fx0A wait for release With jumpQuirks on, Bxnn added only the low byte of the address to Vx, so BE00 with vE at 0x9C landed at 0x09C instead of 0xE9C. The whole twelve bit address counts. The existing test for the quirked jump encoded the low byte reading and changes with this. With clipQuirks on, a sprite whose start lay past the edge was not drawn at all, because the position was only wrapped when clipping was off. The start always wraps; clipping only applies to the overhang. Fx0A moved on as soon as a key went down. The original hardware waits for the key to come up. The key seen going down is remembered and the instruction completes once it is released, read from the key state so a press from before the wait cannot satisfy it. Four tests added, one updated. The screen mock now reports a size so the wrap has something to wrap around. --- .../components/CentralProcessingUnit.java | 38 ++++++++++-- .../components/CentralProcessingUnitTest.java | 59 ++++++++++++++++++- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/src/main/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnit.java b/src/main/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnit.java index 763cf78..53d1a37 100644 --- a/src/main/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnit.java +++ b/src/main/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnit.java @@ -116,6 +116,9 @@ public class CentralProcessingUnit extends Thread // Whether the CPU is waiting for a keypress private boolean awaitingKeypress = false; + // The key seen going down while waiting on Fx0A, or -1 while none has + private int keypressPending = -1; + // Whether shift quirks are enabled private boolean shiftQuirks = false; @@ -838,9 +841,12 @@ protected void loadIndexWithValue() { */ protected void jumpToRegisterPlusValue() { if (jumpQuirks) { + // The whole twelve bit address counts; its top nibble both names + // the register and is part of the address, so BE00 with vE at + // 0x9C jumps to 0xE9C. int x = (operand & 0xF00) >> 8; - pc = v[x] + (operand & 0x00FF); - lastOpDesc = "JUMP V" + toHex(x, 1) + " + " + toHex(operand & 0x00FF, 4); + pc = v[x] + (operand & 0x0FFF); + lastOpDesc = "JUMP V" + toHex(x, 1) + " + " + toHex(operand & 0x0FFF, 3); } else { pc = v[0] + (operand & 0x0FFF); lastOpDesc = "JUMP V0 + " + toHex(operand & 0x0FFF, 3); @@ -910,6 +916,10 @@ protected void drawSprite() { * @param activeIndex the effective index to use when loading sprite data */ private void drawExtendedSprite(int xPos, int yPos, int bitplane, int activeIndex) { + // The starting position always wraps around the screen. Clipping only + // decides what happens to the part of the sprite past the edge. + xPos = xPos % screen.getWidth(); + yPos = yPos % screen.getHeight(); for (int yIndex = 0; yIndex < 16; yIndex++) { for (int xByte = 0; xByte < 2; xByte++) { short colorByte = memory.read(activeIndex + (yIndex * 2) + xByte); @@ -948,6 +958,9 @@ private void drawExtendedSprite(int xPos, int yPos, int bitplane, int activeInde * @param activeIndex the effective index to use when loading sprite data */ private void drawNormalSprite(int xPos, int yPos, int numBytes, int bitplane, int activeIndex) { + // As above: the start wraps, clipping applies to the overhang. + xPos = xPos % screen.getWidth(); + yPos = yPos % screen.getHeight(); for (int yIndex = 0; yIndex < numBytes; yIndex++) { short colorByte = memory.read(activeIndex + yIndex); int yCoord = yPos + yIndex; @@ -1065,6 +1078,7 @@ protected void moveDelayTimerIntoRegister() { */ protected void waitForKeypress() { awaitingKeypress = true; + keypressPending = -1; } /** @@ -1081,13 +1095,27 @@ protected boolean isAwaitingKeypress() { * specified register. If no key is waiting, returns without doing anything. */ protected void decodeKeypressAndContinue() { - int currentKey = keyboard.getCurrentKey(); - if (currentKey == -1) { + // The original hardware moves on when the key is released, not when + // it goes down. So a key going down is remembered, and execution + // continues once that key is up again. Reading the key state rather + // than the last key pressed also means a press from before the wait + // began cannot satisfy it. + if (keypressPending == -1) { + for (int key = 0; key < 16; key++) { + if (keyboard.isKeyPressed(key)) { + keypressPending = key; + return; + } + } + return; + } + if (keyboard.isKeyPressed(keypressPending)) { return; } int x = (operand & 0x0F00) >> 8; - v[x] = (short) currentKey; + v[x] = (short) keypressPending; + keypressPending = -1; lastOpDesc = "KEYD V" + toHex(x, 1); awaitingKeypress = false; } diff --git a/src/test/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnitTest.java b/src/test/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnitTest.java index d286418..f6d8fe8 100644 --- a/src/test/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnitTest.java +++ b/src/test/java/ca/craigthomas/chip8java/emulator/components/CentralProcessingUnitTest.java @@ -38,6 +38,8 @@ public class CentralProcessingUnitTest public void setUp() { memory = new Memory(); screenMock = Mockito.mock(Screen.class); + Mockito.when(screenMock.getWidth()).thenReturn(64); + Mockito.when(screenMock.getHeight()).thenReturn(32); keyboardMock = Mockito.mock(Keyboard.class); Mockito.when(keyboardMock.getCurrentKey()).thenReturn(9); cpu = new CentralProcessingUnit(memory, keyboardMock, screenMock); @@ -775,12 +777,50 @@ public void testJumpToRegisterPlusValueJumpQuirks() { cpu.operand = (short) value; cpu.operand |= (register << 8); cpu.jumpToRegisterPlusValue(); - assertEquals(index + value, cpu.pc); + // The whole address counts, including the nibble that + // names the register, which the jump row of Timendus' + // quirks test checks. + assertEquals(index + ((register << 8) | value), cpu.pc); } } } } + @Test + public void testJumpToRegisterPlusValueJumpQuirksKeepsHighNibble() { + cpu.setJumpQuirks(true); + cpu.v[0xE] = 0x9C; + cpu.operand = 0xBE00; + cpu.jumpToRegisterPlusValue(); + assertEquals(0xE9C, cpu.pc); + } + + @Test + public void testDecodeKeypressContinuesOnReleaseNotPress() { + Keyboard keyboard = Mockito.mock(Keyboard.class); + cpu = new CentralProcessingUnit(memory, keyboard, screenMock); + cpu.operand = 1 << 8; + cpu.waitForKeypress(); + Mockito.when(keyboard.isKeyPressed(5)).thenReturn(true); + cpu.decodeKeypressAndContinue(); + assertTrue("a key going down is not enough", cpu.isAwaitingKeypress()); + assertEquals(0, cpu.v[1]); + Mockito.when(keyboard.isKeyPressed(5)).thenReturn(false); + cpu.decodeKeypressAndContinue(); + assertFalse("the key coming up lets the program move on", cpu.isAwaitingKeypress()); + assertEquals(5, cpu.v[1]); + } + + @Test + public void testDecodeKeypressIgnoresKeysNotHeld() { + Keyboard keyboard = Mockito.mock(Keyboard.class); + cpu = new CentralProcessingUnit(memory, keyboard, screenMock); + cpu.operand = 1 << 8; + cpu.waitForKeypress(); + cpu.decodeKeypressAndContinue(); + assertTrue("with nothing held the wait goes on", cpu.isAwaitingKeypress()); + } + @Test public void testAddRegisterToIndex() { for (int register = 0; register < 0xF; register++) { @@ -1894,6 +1934,23 @@ public void testDrawSpriteDrawsCorrectPattern() throws FontFormatException, IOEx tearDownCanvas(); } + @Test + public void testDrawSpriteClipQuirksWrapsStartPosition() throws FontFormatException, IOException { + setUpCanvas(); + cpu = new CentralProcessingUnit(memory, keyboardMock, screen); + cpu.setClipQuirks(true); + cpu.index = 0x200; + memory.write(0x80, 0x200); + cpu.v[0] = 64; + cpu.v[1] = 32; + cpu.operand = 0x11; + cpu.drawSprite(); + // A start of (64, 32) is (0, 0) on the screen even when clipping, + // which is the clipping row of Timendus' quirks test. + assertTrue(screen.getPixel(0, 0, 1)); + tearDownCanvas(); + } + @Test public void testDrawSpriteExtendedDrawsCorrectPattern() throws FontFormatException, IOException { setUpCanvas();