From 3941cf8ff6275bb9bbd36f385168f23af7a7d41b Mon Sep 17 00:00:00 2001
From: Jeff Parsons <Jeff@pcjs.org>
Date: Mon, 2 May 2016 16:19:50 -0700
Subject: [PATCH] Fixed Space Invaders buffer update (the left-most pixel of
 every byte is bit 0, not bit 7 as in PC video adapters)

---
 devices/pc8080/machine/invaders/machine.xml |  2 +-
 modules/pc8080/lib/video.js                 | 50 ++++++++++++++++-----
 modules/shared/templates/components.xsl     |  8 +++-
 3 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/devices/pc8080/machine/invaders/machine.xml b/devices/pc8080/machine/invaders/machine.xml
index 9ffe20951..469e1950f 100644
--- a/devices/pc8080/machine/invaders/machine.xml
+++ b/devices/pc8080/machine/invaders/machine.xml
@@ -9,7 +9,7 @@
 	<rom id="romF" addr="0x1000" size="0x0800" file="/devices/pc8080/rom/invaders/INVADERS-F.json"/>
 	<rom id="romE" addr="0x1800" size="0x0800" file="/devices/pc8080/rom/invaders/INVADERS-E.json"/>
 	<ram id="ram"  addr="0x2000" size="0x0400"/>
-	<video id="video" screenWidth="896" screenHeight="1024" screenRotation="90" bufferAddr="0x2400" bufferCols="256" bufferRows="224" bufferBits="1" interruptRate="120" width="40%" pos="left" padding="8px">
+	<video id="video" screenWidth="896" screenHeight="1024" screenRotation="90" bufferAddr="0x2400" bufferCols="256" bufferRows="224" bufferBits="1" bufferLeft="0" interruptRate="120" width="40%" pos="left" padding="8px">
 		<menu>
 			<title>224x256 Screen (Rotated)</title>
 		</menu>
diff --git a/modules/pc8080/lib/video.js b/modules/pc8080/lib/video.js
index 1290702df..13252b5b0 100644
--- a/modules/pc8080/lib/video.js
+++ b/modules/pc8080/lib/video.js
@@ -55,20 +55,21 @@ if (NODE) {
  *      bufferAddr: the starting address of the frame buffer (eg, 0x2400)
  *      bufferCols: the width of a single frame buffer row, in pixels (eg, 256)
  *      bufferRows: the number of frame buffer rows (eg, 224)
- *      bufferBits: the number of bits per pixel (default is 1 if omitted)
+ *      bufferBits: the number of bits per column (default is 1 if omitted)
+ *      bufferLeft: the bit position of the left-most pixel in a byte (eg, 0)
  *      interruptRate: normally the same as (or some multiple of) refreshRate (eg, 120)
  *      refreshRate: how many times updateScreen() should be performed per second (eg, 60)
  *
  * We record all the above values now, but we defer creation of the frame buffer until our initBus()
  * handler is called.  At that point, we will also compute the extent of the frame buffer, determine the
  * appropriate "cell" size (ie, the number of pixels that updateScreen() will fetch and process at once),
- * and allocate our cell cache.
+ * and then allocate our cell cache.
  *
  * Why interruptRate in addition to refreshRate?  A higher interrupt rate is required for Space Invaders,
  * because even though the CRT refreshes at 60Hz, the CRT controller interrupts the CPU *twice* per
  * refresh (once after the top half of the screen has been redrawn, and again after the bottom half has
  * been redrawn), so we need an interrupt rate of 120Hz.  We pass the higher rate on to the CPU, so that
- * it will call updateScreen() more frequently, but we limit our screen updates to every *other* call.
+ * it will call updateScreen() more frequently, but we still limit our screen updates to every *other* call.
  *
  * TODO: Consider alternatives to screenRotation; it's expedient, but I'm not sure how efficient it is,
  * and it might be nice (especially for debugging) if we created our own rotated image buffer which we could
@@ -93,6 +94,7 @@ function Video(parmsVideo, canvas, context, textarea, container)
     this.cxBuffer = parmsVideo['bufferCols'];
     this.cyBuffer = parmsVideo['bufferRows'];
     this.nBitsPerPixel = parmsVideo['bufferBits'] || 1;
+    this.iBitFirstPixel = parmsVideo['bufferLeft'] || 0;
 
     this.interruptRate = parmsVideo['interruptRate'];
     this.refreshRate = parmsVideo['refreshRate'] || 60;
@@ -233,6 +235,28 @@ Video.prototype.setPixel = function(imageData, x, y, rgb)
  * and then update the cell cache to match.  Since initCache() sets every cell in the cell cache to an
  * invalid value, we're assured that the next call to updateScreen() will redraw the entire (visible) video buffer.
  *
+ * Example
+ * -------
+ *
+ * The initial Space Invaders screen displays an "S" (part of the word "SCORE") at the end of these rows:
+ *
+ *      2520  00 42 01 00 00 00 00 00-00 00 00 00 00 00 00 00  .B..............
+ *      2530  00 00 00 00 00 00 00 00-00 00 00 00 00 00 32 00  ..............2.
+ *      2540  00 41 01 00 00 00 00 00-00 00 00 00 00 00 00 00  .A..............
+ *      2550  00 00 00 00 00 00 00 00-00 00 00 00 00 00 49 00  ..............I.
+ *      2560  00 49 01 00 00 00 00 00-00 00 00 00 00 00 00 00  .I..............
+ *      2570  00 00 00 00 00 00 00 00-00 00 00 00 00 00 49 00  ..............I.
+ *      2580  00 59 01 00 00 00 00 00-00 00 00 00 00 00 00 00  .Y..............
+ *      2590  00 00 00 00 00 00 00 00-00 00 00 00 00 00 49 00  ..............I.
+ *      25A0  00 66 01 00 00 00 00 00-00 00 00 00 00 00 00 00  .f..............
+ *      25B0  00 00 00 00 00 00 00 00-00 00 00 00 00 00 26 00  ..............&.
+ *
+ *  32      ..11..1.        .1..11..
+ *  49      .1..1..1
+ *  49      .1..1..1
+ *  49      .1..1..1
+ *  26      ..1..11.
+ *
  * @this {Video}
  * @param {number} n (where 0 <= n < getRefreshRate() for a normal update, or -1 for a forced update)
  */
@@ -266,7 +290,6 @@ Video.prototype.updateScreen = function(n)
     var addrLimit = addr + this.sizeBuffer;
 
     var iCell = 0;
-    var wPixelMask = 0x10000;
     var nPixelShift = 1;
     var aPixelColors = this.getColors();
 
@@ -280,22 +303,27 @@ Video.prototype.updateScreen = function(n)
             xBuffer += this.nPixelsPerCell;
         } else {
             this.aCellCache[iCell] = data;
-            var wPixels = (data >> 8) | ((data & 0xff) << 8);
-            var wMask = wPixelMask, nShift = 16;
+            var nShift = 0;
+            var nShiftPixel = this.nBitsPerPixel;
+            var nMask = (1 << nShiftPixel) - 1;
+            if (this.iBitFirstPixel) {
+                nShift = 16 - nShiftPixel;
+                nShiftPixel = -nShiftPixel;
+                data = ((data >> 8) | ((data & 0xff) << 8));
+            }
             if (xBuffer < xDirty) xDirty = xBuffer;
             for (var iPixel = 0; iPixel < this.nPixelsPerCell; iPixel++) {
-                var bPixel = (wPixels & (wMask >>= nPixelShift)) >> (nShift -= nPixelShift);
+                var bPixel = (data >> nShift) & nMask;
                 this.setPixel(this.imageBuffer, xBuffer++, yBuffer, aPixelColors[bPixel]);
+                nShift += nShiftPixel;
             }
             if (xBuffer > xMaxDirty) xMaxDirty = xBuffer;
             if (yBuffer < yDirty) yDirty = yBuffer;
             if (yBuffer >= yMaxDirty) yMaxDirty = yBuffer + 1;
         }
-        addr += 2;
-        iCell++;
+        addr += 2; iCell++;
         if (xBuffer >= this.cxBuffer) {
-            xBuffer = 0;
-            yBuffer++;
+            xBuffer = 0; yBuffer++;
             if (yBuffer > this.cyBuffer) break;
         }
     }
diff --git a/modules/shared/templates/components.xsl b/modules/shared/templates/components.xsl
index e9809f02c..87803066b 100644
--- a/modules/shared/templates/components.xsl
+++ b/modules/shared/templates/components.xsl
@@ -1018,6 +1018,12 @@
 		<xsl:variable name="bufferBits">
 			<xsl:choose>
 				<xsl:when test="@bufferBits"><xsl:value-of select="@bufferBits"/></xsl:when>
+				<xsl:otherwise>1</xsl:otherwise>
+			</xsl:choose>
+		</xsl:variable>
+		<xsl:variable name="bufferLeft">
+			<xsl:choose>
+				<xsl:when test="@bufferLeft"><xsl:value-of select="@bufferLeft"/></xsl:when>
 				<xsl:otherwise>0</xsl:otherwise>
 			</xsl:choose>
 		</xsl:variable>
@@ -1102,7 +1108,7 @@
 		<xsl:call-template name="component">
 			<xsl:with-param name="machine" select="$machine"/>
 			<xsl:with-param name="class">video</xsl:with-param>
-			<xsl:with-param name="parms">,model:'<xsl:value-of select="$model"/>',mode:<xsl:value-of select="$mode"/>,screenWidth:<xsl:value-of select="$screenWidth"/>,screenHeight:<xsl:value-of select="$screenHeight"/>,screenColor:'<xsl:value-of select="$screenColor"/>',screenRotation:<xsl:value-of select="$screenRotation"/>,bufferAddr:<xsl:value-of select="$bufferAddr"/>,bufferCols:<xsl:value-of select="$bufferCols"/>,bufferRows:<xsl:value-of select="$bufferRows"/>,bufferBits:<xsl:value-of select="$bufferBits"/>,memory:<xsl:value-of select="$memory"/>,switches:'<xsl:value-of select="$switches"/>',scale:<xsl:value-of select="$scale"/>,charCols:<xsl:value-of select="$charCols"/>,charRows:<xsl:value-of select="$charRows"/>,fontROM:'<xsl:value-of select="$fontROM"/>',touchScreen:'<xsl:value-of select="$touchScreen"/>',autoLock:<xsl:value-of select="$autoLock"/>,aspectRatio:<xsl:value-of select="$aspectRatio"/>,interruptRate:<xsl:value-of select="$interruptRate"/>,refreshRate:<xsl:value-of select="$refreshRate"/></xsl:with-param>
+			<xsl:with-param name="parms">,model:'<xsl:value-of select="$model"/>',mode:<xsl:value-of select="$mode"/>,screenWidth:<xsl:value-of select="$screenWidth"/>,screenHeight:<xsl:value-of select="$screenHeight"/>,screenColor:'<xsl:value-of select="$screenColor"/>',screenRotation:<xsl:value-of select="$screenRotation"/>,bufferAddr:<xsl:value-of select="$bufferAddr"/>,bufferCols:<xsl:value-of select="$bufferCols"/>,bufferRows:<xsl:value-of select="$bufferRows"/>,bufferBits:<xsl:value-of select="$bufferBits"/>,bufferLeft:<xsl:value-of select="$bufferLeft"/>,memory:<xsl:value-of select="$memory"/>,switches:'<xsl:value-of select="$switches"/>',scale:<xsl:value-of select="$scale"/>,charCols:<xsl:value-of select="$charCols"/>,charRows:<xsl:value-of select="$charRows"/>,fontROM:'<xsl:value-of select="$fontROM"/>',touchScreen:'<xsl:value-of select="$touchScreen"/>',autoLock:<xsl:value-of select="$autoLock"/>,aspectRatio:<xsl:value-of select="$aspectRatio"/>,interruptRate:<xsl:value-of select="$interruptRate"/>,refreshRate:<xsl:value-of select="$refreshRate"/></xsl:with-param>
 		</xsl:call-template>
 	</xsl:template>