Additional refactoring

pom.xml

@@ -38,7 +38,7 @@
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
-            <version>4.10</version>
+            <version>4.11</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -52,7 +52,7 @@
         <plugins>
             <plugin>
                 <artifactId>maven-resources-plugin</artifactId>
-                <version>2.5</version>
+                <version>2.6</version>
                 <executions>
                     <execution>
                         <id>copy-resources</id>
@@ -74,7 +74,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-assembly-plugin</artifactId>
-                <version>2.3</version>
+                <version>2.4</version>
                 <configuration>
                     <descriptorRefs>
                         <descriptorRef>jar-with-dependencies</descriptorRef>
@@ -95,15 +95,14 @@
                     </execution>
                 </executions>
             </plugin>
-            <!-- Set Java version to Java 1.5 -->
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-compiler-plugin</artifactId>
-                <version>2.3.2</version>
+                <version>3.1</version>
-                <!-- best lock down version of the plugin too -->
                 <configuration>
-                    <source>1.5</source>
+                    <compilerArgument>-Xlint:unchecked</compilerArgument>
-                    <target>1.5</target>
+                    <source>1.6</source>
+                    <target>1.6</target>
                 </configuration>
             </plugin>
 
@@ -111,7 +110,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-jar-plugin</artifactId>
-                <version>2.3.1</version>
+                <version>2.4</version>
                 <configuration>
                     <archive>
                         <manifest>

src/main/java/com/loomcom/symon/devices/Crtc.java

@@ -20,14 +20,8 @@ public class Crtc extends Device {
     public static final int REGISTER_WRITE           = 1;
 
     // Registers
-    public static final int HORIZONTAL_TOTAL         = 0;
     public static final int HORIZONTAL_DISPLAYED     = 1;
-    public static final int HORIZONTAL_SYNC_POSITION = 2;
-    public static final int H_V_SYNC_WIDTHS          = 3;
-    public static final int VERTICAL_TOTAL           = 4;
-    public static final int VERTICAL_TOTAL_ADJUST    = 5;
     public static final int VERTICAL_DISPLAYED       = 6;
-    public static final int VERTICAL_SYNC_POSITION   = 7;
     public static final int MODE_CONTROL             = 8;
     public static final int SCAN_LINE                = 9;
     public static final int CURSOR_START             = 10;
@@ -36,8 +30,6 @@ public class Crtc extends Device {
     public static final int DISPLAY_START_LOW        = 13;
     public static final int CURSOR_POSITION_HIGH     = 14;
     public static final int CURSOR_POSITION_LOW      = 15;
-    public static final int LPEN_HIGH                = 16;
-    public static final int LPEN_LOW                 = 17;
 
 
     /*
@@ -84,9 +76,9 @@ public class Crtc extends Device {
         this.verticalDisplayed = 25;
         this.scanLinesPerRow = 9;
         this.cursorStartLine = 0;
-        this.cursorStopLine = 8;
+        this.cursorStopLine = 7;
         this.startAddress = 0x7000;
-        this.cursorPosition = 0;
+        this.cursorPosition = startAddress;
         this.pageSize = horizontalDisplayed * verticalDisplayed;
         this.cursorEnabled = true;
         this.cursorBlinkRate = 500;
@@ -94,7 +86,6 @@ public class Crtc extends Device {
 
     @Override
     public void write(int address, int data) throws MemoryAccessException {
-        logger.info("[write] Writing CRTC address=" + address + " data=" + data);
         switch (address) {
             case REGISTER_SELECT:
                 setCurrentRegister(data);
@@ -179,7 +170,6 @@ public class Crtc extends Device {
     }
 
     private void writeRegisterValue(int data) {
-        logger.info("Writing CRTC Register #" + currentRegister + " with value " + String.format("$%02X", data));
         switch (currentRegister) {
             case HORIZONTAL_DISPLAYED:
                 horizontalDisplayed = data;
@@ -190,13 +180,54 @@ public class Crtc extends Device {
                 pageSize = horizontalDisplayed * verticalDisplayed;
                 break;
             case MODE_CONTROL:
-                // TODO: Implement multiple addressing modes.
+                // TODO: Implement multiple addressing modes and cursor skew.
                 break;
             case SCAN_LINE:
                 scanLinesPerRow = data;
                 break;
+            case CURSOR_START:
+                cursorStartLine = data & 0x1f;
+                // Bits 5 and 6 define the cursor mode.
+                int cursorMode = (data & 0x60) >> 5;
+                switch (cursorMode) {
+                    case 0:
+                        cursorEnabled = true;
+                        cursorBlinkRate = 0;
+                        break;
+                    case 1:
+                        cursorEnabled = false;
+                        cursorBlinkRate = 0;
+                        break;
+                    case 2:
+                        cursorEnabled = true;
+                        cursorBlinkRate = 500;
+                        break;
+                    case 3:
+                        cursorEnabled = true;
+                        cursorBlinkRate = 1000;
+                        break;
+                }
+                break;
+            case CURSOR_END:
+                cursorStopLine = data & 0x1f;
+                break;
+            case DISPLAY_START_HIGH:
+                startAddress = ((data & 0xff) << 8) | (startAddress & 0x00ff);
+                // TODO: bounds checking.
+                break;
+            case DISPLAY_START_LOW:
+                startAddress = ((data & 0xff) | (startAddress & 0xff00));
+                // TODO: bounds checking.
+                break;
+            case CURSOR_POSITION_HIGH:
+                cursorPosition = ((data & 0xff) << 8) | (cursorPosition & 0x00ff);
+                // TODO: bounds checking.
+                break;
+            case CURSOR_POSITION_LOW:
+                // TODO: bounds checking.
+                cursorPosition = (data & 0xff) | (cursorPosition & 0xff00);
+                break;
             default:
-                logger.info("Ignoring.");
                 break;
         }
 

src/main/java/com/loomcom/symon/ui/VideoWindow.java

@@ -38,6 +38,21 @@ import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
+/**
+ * VideoWindow represents a graphics framebuffer backed by a 6545 CRTC.
+ * Each time the window's VideoPanel is repainted, the video memory is
+ * scanned and converted to the appropriate bitmap representation.
+ * <p>
+ * The graphical representation of each character is derived from a
+ * character generator ROM image. For this simulation, the Commodore PET
+ * character generator ROM was chosen, but any character generator ROM
+ * could be used in its place.
+ * <p>
+ * It may be convenient to think of this as the View (in the MVC
+ * pattern sense) to the Crtc's Model and Controller. Whenever the CRTC
+ * updates state in a way that may require the view to update, it calls
+ * the <tt>deviceStateChange</tt> callback on this Window.
+ */
 public class VideoWindow extends JFrame implements DeviceChangeListener {
 
     private static final Logger logger = Logger.getLogger(VideoWindow.class.getName());
@@ -56,7 +71,7 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
     private int verticalDisplayed;
     private int scanLinesPerRow;
     private int cursorBlinkRate;
-    private boolean showCursor;
+    private boolean hideCursor;
 
     private Dimension dimensions;
     private Crtc crtc;
@@ -64,6 +79,9 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
     private ScheduledExecutorService scheduler;
     private ScheduledFuture<?> cursorBlinker;
 
+    /**
+     * A panel representing the composite video output, with fast Graphics2D painting.
+     */
     private class VideoPanel extends JPanel {
         @Override
         public void paintComponent(Graphics g) {
@@ -71,7 +89,7 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
                 int address = crtc.getStartAddress() + i;
                 int originX = (i % horizontalDisplayed) * CHAR_WIDTH;
                 int originY = (i / horizontalDisplayed) * scanLinesPerRow;
-                image.getRaster().setPixels(originX, originY, CHAR_WIDTH, scanLinesPerRow, getGlyph(i, videoRam[address]));
+                image.getRaster().setPixels(originX, originY, CHAR_WIDTH, scanLinesPerRow, getGlyph(address));
             }
             Graphics2D g2d = (Graphics2D)g;
             if (shouldScale) {
@@ -92,12 +110,15 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
 
     }
 
+    /**
+     * Runnable task that blinks the cursor.
+     */
     private class CursorBlinker implements Runnable {
         public void run() {
             SwingUtilities.invokeLater(new Runnable() {
                 public void run() {
                     if (cursorBlinkRate > 0) {
-                        showCursor = !showCursor;
+                        hideCursor = !hideCursor;
                         repaint();
                     }
                 }
@@ -110,7 +131,7 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
 
         this.scheduler = Executors.newSingleThreadScheduledExecutor();
         this.crtc = crtc;
-        this.charRom = convertCharRom(loadCharRom("/pet.rom"), CHAR_WIDTH);
+        this.charRom = loadCharRom("/pet.rom");
         this.videoRam = crtc.getDmaAccess();
         this.scaleX = scaleX;
         this.scaleY = scaleY;
@@ -137,57 +158,22 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
 
     }
 
-    private byte[] loadCharRom(String resource) throws IOException {
+    public void refreshDisplay() {
-        BufferedInputStream bis = null;
+        // TODO: Verify whether this is necessary. Does `repaint()' do anything if the window is not visible?
-        try {
+        if (isVisible()) {
-            bis = new BufferedInputStream(this.getClass().getResourceAsStream(resource));
+            repaint();
-            ByteArrayOutputStream bos = new ByteArrayOutputStream();
-            while (bis.available() > 0) {
-                bos.write(bis.read());
-            }
-            bos.flush();
-            bos.close();
-            return bos.toByteArray();
-        } finally {
-            if (bis != null) {
-                bis.close();
-            }
         }
     }
 
-    public void createAndShowUi() {
-        setTitle("Composite Video");
-
-        int borderWidth = 20;
-        int borderHeight = 20;
-
-        JPanel containerPane = new JPanel();
-        containerPane.setBorder(BorderFactory.createEmptyBorder(borderHeight, borderWidth, borderHeight, borderWidth));
-        containerPane.setLayout(new BorderLayout());
-        containerPane.setBackground(Color.black);
-
-        containerPane.add(new VideoPanel(), BorderLayout.CENTER);
-
-        getContentPane().add(containerPane, BorderLayout.CENTER);
-        setResizable(false);
-        pack();
-    }
-
-    private void buildImage() {
-        int rasterWidth = CHAR_WIDTH * horizontalDisplayed;
-        int rasterHeight = scanLinesPerRow * verticalDisplayed;
-        this.image = new BufferedImage(rasterWidth, rasterHeight, BufferedImage.TYPE_BYTE_BINARY);
-        this.dimensions = new Dimension(rasterWidth * scaleX, rasterHeight * scaleY);
-    }
-
     /**
      * Called by the CRTC on state change.
      */
     public void deviceStateChanged() {
 
-        // Certain state
         boolean repackNeeded = false;
 
+        // TODO: I'm not entirely happy with this pattern, and I'd like to make it a bit DRY-er.
+
         if (horizontalDisplayed != crtc.getHorizontalDisplayed()) {
             horizontalDisplayed = crtc.getHorizontalDisplayed();
             repackNeeded = true;
@@ -209,6 +195,7 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
             if (cursorBlinker != null) {
                 cursorBlinker.cancel(true);
                 cursorBlinker = null;
+                hideCursor = false;
             }
 
             if (cursorBlinkRate > 0) {
@@ -226,26 +213,22 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
         }
     }
 
-    /**
+    private void createAndShowUi() {
-     * Convert a raw binary Character ROM image into an array of pixel data usable
+        setTitle("Composite Video");
-     * by the Raster underlying the display's BufferedImage.
-     *
-     * @param rawBytes
-     * @param charWidth
-     * @return
-     */
-    private int[] convertCharRom(byte[] rawBytes, int charWidth) {
-        int[] converted = new int[rawBytes.length * charWidth];
 
-        int romIndex = 0;
+        int borderWidth = 20;
-        for (int i = 0; i < converted.length;) {
+        int borderHeight = 20;
-            byte charRow = rawBytes[romIndex++];
 
-            for (int j = 7; j >= 0; j--) {
+        JPanel containerPane = new JPanel();
-                converted[i++] = ((charRow & (1 << j)) == 0) ? 0 : 0xff;
+        containerPane.setBorder(BorderFactory.createEmptyBorder(borderHeight, borderWidth, borderHeight, borderWidth));
-            }
+        containerPane.setLayout(new BorderLayout());
-        }
+        containerPane.setBackground(Color.black);
-        return converted;
+
+        containerPane.add(new VideoPanel(), BorderLayout.CENTER);
+
+        getContentPane().add(containerPane, BorderLayout.CENTER);
+        setResizable(false);
+        pack();
     }
 
     /**
@@ -253,11 +236,11 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
      * Character ROM plus cursor overlay (if any). The cursor overlay simulates an XOR
      * of the Character Rom output and the 6545 Cursor output.
      *
-     * @param position The position within the character field, from 0 to (horizontalDisplayed * verticalDisplayed)
+     * @param address The address of the character being requested.
-     * @param chr The character value within the ROM to display.
+     * @return An array of integers representing the pixel data.
-     * @return
      */
-    private int[] getGlyph(int position, int chr) {
+    private int[] getGlyph(int address) {
+        int chr = videoRam[address];
         int romOffset = (chr & 0xff) * (CHAR_HEIGHT * CHAR_WIDTH);
         int[] glyph = new int[CHAR_WIDTH * scanLinesPerRow];
 
@@ -267,7 +250,7 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
         }
 
         // Overlay the cursor
-        if (showCursor && crtc.isCursorEnabled() && crtc.getCursorPosition() == position) {
+        if (!hideCursor && crtc.isCursorEnabled() && crtc.getCursorPosition() == address) {
             int cursorStart = Math.min(glyph.length, crtc.getCursorStartLine() * CHAR_WIDTH);
             int cursorStop = Math.min(glyph.length, (crtc.getCursorStopLine() + 1) * CHAR_WIDTH);
 
@@ -279,11 +262,54 @@ public class VideoWindow extends JFrame implements DeviceChangeListener {
         return glyph;
     }
 
-    public void refreshDisplay() {
+    private void buildImage() {
-        // TODO: Verify whether this is necessary. Does `repaint()' do anything if the window is not visible?
+        int rasterWidth = CHAR_WIDTH * horizontalDisplayed;
-        if (isVisible()) {
+        int rasterHeight = scanLinesPerRow * verticalDisplayed;
-            repaint();
+        this.image = new BufferedImage(rasterWidth, rasterHeight, BufferedImage.TYPE_BYTE_BINARY);
-        }
+        this.dimensions = new Dimension(rasterWidth * scaleX, rasterHeight * scaleY);
     }
 
+    /**
+     * Load a Character ROM file and convert it into an array of pixel data usable
+     * by the underlying BufferedImage's Raster.
+     * <p>
+     * Since the BufferedImage is a TYPE_BYTE_BINARY, the data must be converted
+     * into a single byte per pixel, 0 for black and 255 for white.
+
+     * @param resource The ROM file resource to load.
+     * @return An array of glyphs, each ready for insertion.
+     * @throws IOException
+     */
+    private int[] loadCharRom(String resource) throws IOException {
+        BufferedInputStream bis = null;
+        try {
+            bis = new BufferedInputStream(this.getClass().getResourceAsStream(resource));
+            ByteArrayOutputStream bos = new ByteArrayOutputStream();
+            while (bis.available() > 0) {
+                bos.write(bis.read());
+            }
+            bos.flush();
+            bos.close();
+
+            byte[] raw = bos.toByteArray();
+
+            // Now convert the raw ROM image into a format suitable for
+            // insertion directly into the BufferedImage.
+            int[] converted = new int[raw.length * CHAR_WIDTH];
+
+            int romIndex = 0;
+            for (int i = 0; i < converted.length;) {
+                byte charRow = raw[romIndex++];
+
+                for (int j = 7; j >= 0; j--) {
+                    converted[i++] = ((charRow & (1 << j)) == 0) ? 0 : 0xff;
+                }
+            }
+            return converted;
+        } finally {
+            if (bis != null) {
+                bis.close();
+            }
+        }
+    }
 }