I shouldn't be mixing cleanups and bug fixes, but I'm doing so anyway, just

this once. :-)

The parameters to drawLine() aren't clipped to the screen size, which meant
that it was accessing memory out of bounds when marking the screen as
dirty. The function now uses plotPoint(), which does the bounds checking
and screen dirtying for us. Apart from being a little easier to read, it
dirties only the parts of the screen the line actually passes through,
instead of a rectangle defined by the line's end points.

Since drawLine() is only used for debugging, I wouldn't consider this a
particularly serious bug.

Next change is that only the pixels inside the original parallax layer are
considered when creating the block surfaces. This may make the drawing
slightly more efficient, since fewer surfaces will be labelled as
transparent.

Plus some other minor cleanups.

svn-id: r14340

3 files changed, 74 insertions, 98 deletions

diff --git a/sword2/driver/d_draw.h b/sword2/driver/d_draw.h
index 5e30e4d79c..df085c127a 100644
--- a/sword2/driver/d_draw.h
+++ b/sword2/driver/d_draw.h
@@ -221,7 +221,7 @@ public:
 	int32 initialiseBackgroundLayer(Parallax *p);
 	void closeBackgroundLayer(void);
 
-	void plotPoint(uint16 x, uint16 y, uint8 colour);
+	void plotPoint(int16 x, int16 y, uint8 colour);
 	void drawLine(int16 x1, int16 y1, int16 x2, int16 y2, uint8 colour);
 #ifdef BACKEND_8BIT
 	void plotYUV(byte *lut, int width, int height, byte *const *dat);
diff --git a/sword2/driver/render.cpp b/sword2/driver/render.cpp
index edf91cd22c..b3d73df142 100644
--- a/sword2/driver/render.cpp
+++ b/sword2/driver/render.cpp
@@ -28,9 +28,6 @@ namespace Sword2 {
 
 #define MILLISECSPERCYCLE	83
 
-#define BLOCKWBITS		6
-#define BLOCKHBITS		6
-
 void Graphics::updateRect(Common::Rect *r) {
 	_vm->_system->copyRectToScreen(_buffer + r->top * _screenWide + r->left,
 		_screenWide, r->left, r->top, r->right - r->left,
@@ -57,11 +54,11 @@ void Graphics::blitBlockSurface(BlockSurface *s, Common::Rect *r, Common::Rect *
 		r->right = clipRect->right;
 
 	byte *dst = _buffer + r->top * _screenWide + r->left;
-	int i, j;
+	int i;
 
 	if (s->transparent) {
 		for (i = 0; i < r->bottom - r->top; i++) {
-			for (j = 0; j < r->right - r->left; j++) {
+			for (int j = 0; j < r->right - r->left; j++) {
 				if (src[j])
 					dst[j] = src[j];
 			}
@@ -195,16 +192,15 @@ void Graphics::scaleImageGood(byte *dst, uint16 dstPitch, uint16 dstWidth, uint1
  * @param colour colour of the point
  */
 
-void Graphics::plotPoint(uint16 x, uint16 y, uint8 colour) {
-	byte *buf = _buffer + 40 * RENDERWIDE;
-	int16 newx, newy;
-	
-	newx = x - _scrollX;
-	newy = y - _scrollY;
+void Graphics::plotPoint(int16 x, int16 y, uint8 colour) {
+	byte *buf = _buffer + MENUDEEP * RENDERWIDE;
+
+	x -= _scrollX;
+	y -= _scrollY;
 
-	if (newx >= 0 && newx < RENDERWIDE && newy >= 0 && newy < RENDERDEEP) {
-		buf[newy * RENDERWIDE + newx] = colour;
-		markAsDirty(newx, newy + 40, newx, newy + 40);
+	if (x >= 0 && x < RENDERWIDE && y >= 0 && y < RENDERDEEP) {
+		buf[y * RENDERWIDE + x] = colour;
+		markAsDirty(x, y + MENUDEEP, x, y + MENUDEEP);
 	}
 }
 
@@ -218,22 +214,14 @@ void Graphics::plotPoint(uint16 x, uint16 y, uint8 colour) {
  */
 
 // Uses Bressnham's incremental algorithm!
+
 void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
-	byte *buf = _buffer + 40 * RENDERWIDE;
-	int dx, dy;
 	int dxmod, dymod;
 	int ince, incne;
 	int d;
 	int x, y;
 	int addTo;
 
-	x1 -= _scrollX;
-	y1 -= _scrollY;
-	x0 -= _scrollX;
-	y0 -= _scrollY;
-
-	markAsDirty(MIN(x0, x1), MIN(y0, y1) + 40, MAX(x0, x1), MAX(y0, y1) + 40);
-
 	// Make sure we're going from left to right
 
 	if (x1 < x0) {
@@ -241,8 +229,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 		SWAP(y0, y1);
 	}
 
-	dx = x1 - x0;
-	dy = y1 - y0;
+	int dx = x1 - x0;
+	int dy = y1 - y0;
 	
 	if (dx < 0)
 		dxmod = -dx;
@@ -261,8 +249,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 			incne = 2 * (dy - dx);
 			x = x0;
 			y = y0;
-			if (x >= 0 && x < RENDERWIDE && y >= 0 && y < RENDERDEEP)
-				buf[y * RENDERWIDE + x] = colour;
+
+			plotPoint(x, y, colour);
 
 			while (x < x1) {
 				if (d <= 0) {
@@ -273,8 +261,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 					x++;
 					y++;
 				}
-				if (x >= 0 && x < RENDERWIDE && y >= 0 && y < RENDERDEEP)
-					buf[y * RENDERWIDE + x] = colour;
+
+				plotPoint(x, y, colour);
 			}
 		} else {
 			addTo = y0;
@@ -288,8 +276,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 			incne = 2 * (dy - dx);
 			x = x0;
 			y = y0;
-			if (x >= 0 && x < RENDERWIDE && addTo - y >= 0 && addTo - y < RENDERDEEP)
-				buf[(addTo - y) * RENDERWIDE + x] = colour;
+
+			plotPoint(x, addTo - y, colour);
 
 			while (x < x1) {
 				if (d <= 0) {
@@ -300,8 +288,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 					x++;
 					y++;
 				}
-				if (x >= 0 && x < RENDERWIDE && addTo - y >= 0 && addTo - y < RENDERDEEP)
-					buf[(addTo - y) * RENDERWIDE + x] = colour;
+
+				plotPoint(x, addTo - y, colour);
 			}
 		}
 	} else {
@@ -320,8 +308,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 			incne = 2 * (dx - dy);
 			x = x0;
 			y = y0;
-			if (x >= 0 && x < RENDERWIDE && y >= 0 && y < RENDERDEEP)
-				buf[y * RENDERWIDE + x] = colour;
+
+			plotPoint(x, y, colour);
 
 			while (y < y1) {
 				if (d <= 0) {
@@ -332,8 +320,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 					x++;
 					y++;
 				}
-				if (x >= 0 && x < RENDERWIDE && y >= 0 && y < RENDERDEEP)
-					buf[y * RENDERWIDE + x] = colour;
+
+				plotPoint(x, y, colour);
 			}
 		} else {
 			addTo = x0;
@@ -347,8 +335,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 			incne = 2 * (dx - dy);
 			x = x0;
 			y = y0;
-			if (addTo - x >= 0 && addTo - x < RENDERWIDE && y >= 0 && y < RENDERDEEP)
-				buf[y * RENDERWIDE + addTo - x] = colour;
+
+			plotPoint(addTo - x, y, colour);
 
 			while (y < y1) {
 				if (d <= 0) {
@@ -359,8 +347,8 @@ void Graphics::drawLine(int16 x0, int16 y0, int16 x1, int16 y1, uint8 colour) {
 					x++;
 					y++;
 				}
-				if (addTo - x >= 0 && addTo - x < RENDERWIDE && y >= 0 && y < RENDERDEEP)
-					buf[y * RENDERWIDE + addTo - x] = colour;
+
+				plotPoint(addTo - x, y, colour);
 			}
 		}
 	}
@@ -412,7 +400,7 @@ void Graphics::renderParallax(Parallax *p, int16 l) {
 			if (_blockSurfaces[l][i + j * _xBlocks[l]]) {
 				r.left = i * BLOCKWIDTH - x;
 				r.right = r.left + BLOCKWIDTH;
-				r.top = j * BLOCKHEIGHT - y + 40;
+				r.top = j * BLOCKHEIGHT - y + MENUDEEP;
 				r.bottom = r.top + BLOCKHEIGHT;
 				blitBlockSurface(_blockSurfaces[l][i + j * _xBlocks[l]], &r, &clipRect);
 			}
@@ -558,31 +546,21 @@ void Graphics::setScrollTarget(int16 sx, int16 sy) {
  */
 
 int32 Graphics::initialiseBackgroundLayer(Parallax *p) {
-	byte *memchunk;
-	uint8 zeros;
-	uint16 count;
 	uint16 i, j, k;
-	uint16 x;
 	byte *data;
 	byte *dst;
-	ParallaxLine line;
-	byte *pLine;
 
 	debug(2, "initialiseBackgroundLayer");
 
-	// This function is called to re-initialise the layers if they have
-	// been lost. We know this if the layers have already been assigned.
-
-	if (_layer == MAXLAYERS)
-		closeBackgroundLayer();
+	assert(_layer < MAXLAYERS);
 
 	if (!p) {
 		_layer++;
 		return RD_OK;
 	}
 
-	_xBlocks[_layer] = (p->w + BLOCKWIDTH - 1) >> BLOCKWBITS;
-	_yBlocks[_layer] = (p->h + BLOCKHEIGHT - 1) >> BLOCKHBITS;
+	_xBlocks[_layer] = (p->w + BLOCKWIDTH - 1) / BLOCKWIDTH;
+	_yBlocks[_layer] = (p->h + BLOCKHEIGHT - 1) / BLOCKHEIGHT;
 
 	_blockSurfaces[_layer] = (BlockSurface **) calloc(_xBlocks[_layer] * _yBlocks[_layer], sizeof(BlockSurface *));
 	if (!_blockSurfaces[_layer])
@@ -590,66 +568,70 @@ int32 Graphics::initialiseBackgroundLayer(Parallax *p) {
 
 	// Decode the parallax layer into a large chunk of memory
 
-	memchunk = (byte *) calloc(_xBlocks[_layer] * _yBlocks[_layer], BLOCKWIDTH * BLOCKHEIGHT);
+	byte *memchunk = (byte *) calloc(_xBlocks[_layer] * _yBlocks[_layer], BLOCKWIDTH * BLOCKHEIGHT);
 	if (!memchunk)
 		return RDERR_OUTOFMEMORY;
 
 	for (i = 0; i < p->h; i++) {
-		if (p->offset[i] == 0)
+		if (!p->offset[i])
 			continue;
 
-		pLine = (byte *) p + FROM_LE_32(p->offset[i]);
-		line.packets = READ_LE_UINT16(pLine);
-		line.offset = READ_LE_UINT16(pLine + 2);
-		data = pLine + sizeof(ParallaxLine);
-		x = line.offset;
+		byte *pLine = (byte *) p + FROM_LE_32(p->offset[i]);
+		uint16 packets = READ_LE_UINT16(pLine);
+		uint16 offset = READ_LE_UINT16(pLine + 2);
 
-		dst = memchunk + i * p->w + x;
+		data = pLine + 4;
+		dst = memchunk + i * p->w + offset;
 
-		zeros = 0;
-		if (line.packets == 0)	{
+		if (!packets) {
 			memcpy(dst, data, p->w);
 			continue;
 		}
 
-		for (j = 0; j < line.packets; j++) {
+		bool zeros = false;
+
+		for (j = 0; j < packets; j++) {
 			if (zeros) {
 				dst += *data;
-				x += *data;
+				offset += *data;
 				data++;
-				zeros = 0;
-			} else if (*data == 0) {
+				zeros = false;
+			} else if (!*data) {
 				data++;
-				zeros = 1;
+				zeros = true;
 			} else {
-				count = *data++;
+				uint16 count = *data++;
 				memcpy(dst, data, count);
 				data += count;
 				dst += count;
-				x += count;
-				zeros = 1;
+				offset += count;
+				zeros = true;
 			}
 		}
 	}
 
-	// Now create the surfaces!
+	// The large memory chunk is now divided into a number of smaller
+	// surfaces. For most parallax layers, we'll end up using less memory
+	// this way, and it will be faster to draw since completely transparent
+	// surfaces are discarded.
 
 	for (i = 0; i < _xBlocks[_layer] * _yBlocks[_layer]; i++) {
 		bool block_has_data = false;
 		bool block_is_transparent = false;
 
-		data = memchunk + (p->w * BLOCKHEIGHT * (i / _xBlocks[_layer])) + BLOCKWIDTH * (i % _xBlocks[_layer]);
+		int x = BLOCKWIDTH * (i % _xBlocks[_layer]);
+		int y = BLOCKHEIGHT * (i / _xBlocks[_layer]);
 
-		// FIXME: The 'block_is_transparent' flag should only consider
-		// data that is inside the parallax layer. Still, it won't do
-		// any harm to leave it this way...
+		data = memchunk + p->w * y + x;
 
 		for (j = 0; j < BLOCKHEIGHT; j++) {
 			for (k = 0; k < BLOCKWIDTH; k++) {
-				if (data[j * p->w + k])
-					block_has_data = true;
-				else
-					block_is_transparent = true;
+				if (x + k < p->w && y + j < p->h) {
+					if (data[j * p->w + k])
+						block_has_data = true;
+					else
+						block_is_transparent = true;
+				}
 			}
 		}
 
@@ -685,13 +667,13 @@ int32 Graphics::initialiseBackgroundLayer(Parallax *p) {
 void Graphics::closeBackgroundLayer(void) {
 	debug(2, "CloseBackgroundLayer");
 
-	for (int j = 0; j < MAXLAYERS; j++) {
-		if (_blockSurfaces[j]) {
-			for (int i = 0; i < _xBlocks[j] * _yBlocks[j]; i++)
-				if (_blockSurfaces[j][i])
-					free(_blockSurfaces[j][i]);
-			free(_blockSurfaces[j]);
-			_blockSurfaces[j] = NULL;
+	for (int i = 0; i < MAXLAYERS; i++) {
+		if (_blockSurfaces[i]) {
+			for (int j = 0; j < _xBlocks[i] * _yBlocks[i]; j++)
+				if (_blockSurfaces[i][j])
+					free(_blockSurfaces[i][j]);
+			free(_blockSurfaces[i]);
+			_blockSurfaces[i] = NULL;
 		}
 	}
 
@@ -710,7 +692,7 @@ void Graphics::plotYUV(byte *lut, int width, int height, byte *const *dat) {
 
 	for (y = 0; y < height; y += 2) {
 		for (x = 0; x < width; x += 2) {
-			int i = ((((dat[2][cpos] + ROUNDADD) >> SHIFT) * (BITDEPTH+1)) + ((dat[1][cpos] + ROUNDADD)>>SHIFT)) * (BITDEPTH+1);
+			int i = ((((dat[2][cpos] + ROUNDADD) >> SHIFT) * (BITDEPTH + 1)) + ((dat[1][cpos] + ROUNDADD) >> SHIFT)) * (BITDEPTH + 1);
 			cpos++;
 
 			buf[linepos               ] = lut[i + ((dat[0][        ypos  ] + ROUNDADD) >> SHIFT)];
diff --git a/sword2/driver/render.h b/sword2/driver/render.h
index 619acea867..2bf0ff2669 100644
--- a/sword2/driver/render.h
+++ b/sword2/driver/render.h
@@ -25,13 +25,7 @@
 namespace Sword2 {
 
 #define RENDERWIDE		640
-#define ALIGNRENDERDEEP		480
-#define RENDERDEEP		(ALIGNRENDERDEEP - (MENUDEEP * 2))
-
-struct ParallaxLine {
-	uint16 packets;
-	uint16 offset;
-};
+#define RENDERDEEP		(480 - (MENUDEEP * 2))
 
 } // End of namespace Sword2