SCI32: Clean up GfxCursor32

* Rewrap doxygen comments to 80 columns
* Renamings for clarity
* Deduplicate copy/paint code

2 files changed, 139 insertions, 183 deletions

diff --git a/engines/sci/graphics/cursor32.cpp b/engines/sci/graphics/cursor32.cpp
index 74fbafa2f0..ae8c01495c 100644
--- a/engines/sci/graphics/cursor32.cpp
+++ b/engines/sci/graphics/cursor32.cpp
@@ -34,22 +34,22 @@ namespace Sci {
 GfxCursor32::GfxCursor32() :
 	_hideCount(0),
 	_position(0, 0),
-	_writeToVMAP(false) {
+	_needsPaint(false) {
 }
 
-void GfxCursor32::init(const Buffer &vmap) {
-	_vmap = vmap;
-	_vmapRegion.rect = Common::Rect(_vmap.screenWidth, _vmap.screenHeight);
-	_vmapRegion.data = (byte *)_vmap.getPixels();
-	_restrictedArea = _vmapRegion.rect;
+void GfxCursor32::init(const Buffer &outputBuffer) {
+	_screen = outputBuffer;
+	_screenRegion.rect = Common::Rect(_screen.screenWidth, _screen.screenHeight);
+	_screenRegion.data = static_cast<byte *>(_screen.getPixels());
+	_restrictedArea = _screenRegion.rect;
 }
 
 GfxCursor32::~GfxCursor32() {
 	free(_cursor.data);
 	free(_cursorBack.data);
-	free(_drawBuff1.data);
-	free(_drawBuff2.data);
-	free(_savedVmapRegion.data);
+	free(_scratch1.data);
+	free(_scratch2.data);
+	free(_savedScreenRegion.data);
 }
 
 void GfxCursor32::hide() {
@@ -58,25 +58,26 @@ void GfxCursor32::hide() {
 	}
 
 	if (!_cursorBack.rect.isEmpty()) {
-		drawToHardware(_cursorBack);
+		drawToScreen(_cursorBack);
 	}
 }
 
 void GfxCursor32::revealCursor() {
 	_cursorBack.rect = _cursor.rect;
-	_cursorBack.rect.clip(_vmapRegion.rect);
+	_cursorBack.rect.clip(_screenRegion.rect);
 	if (_cursorBack.rect.isEmpty()) {
 		return;
 	}
 
-	readVideo(_cursorBack);
-	_drawBuff1.rect = _cursor.rect;
-	copy(_drawBuff1, _cursorBack);
-	paint(_drawBuff1, _cursor);
-	drawToHardware(_drawBuff1);
+	copyFromScreen(_cursorBack);
+	_scratch1.rect = _cursor.rect;
+	copy<false>(_scratch1, _cursorBack);
+	copy<true>(_scratch1, _cursor);
+	drawToScreen(_scratch1);
 }
 
-void GfxCursor32::paint(DrawRegion &target, const DrawRegion &source) {
+template <bool SKIP>
+void GfxCursor32::copy(DrawRegion &target, const DrawRegion &source) {
 	if (source.rect.isEmpty()) {
 		return;
 	}
@@ -96,25 +97,33 @@ void GfxCursor32::paint(DrawRegion &target, const DrawRegion &source) {
 	const byte *sourcePixel = source.data + (sourceYOffset * source.rect.width()) + sourceXOffset;
 	const uint8 skipColor = source.skipColor;
 
-	const int16 sourceStride = source.rect.width() - drawRectWidth;
-	const int16 targetStride = target.rect.width() - drawRectWidth;
+	int16 sourceStride = source.rect.width();
+	int16 targetStride = target.rect.width();
+	if (SKIP) {
+		sourceStride -= drawRectWidth;
+		targetStride -= drawRectWidth;
+	}
 
 	for (int16 y = 0; y < drawRectHeight; ++y) {
-		for (int16 x = 0; x < drawRectWidth; ++x) {
-			if (*sourcePixel != skipColor) {
-				*targetPixel = *sourcePixel;
+		if (SKIP) {
+			for (int16 x = 0; x < drawRectWidth; ++x) {
+				if (*sourcePixel != skipColor) {
+					*targetPixel = *sourcePixel;
+				}
+				++targetPixel;
+				++sourcePixel;
 			}
-			++targetPixel;
-			++sourcePixel;
+		} else {
+			memcpy(targetPixel, sourcePixel, drawRectWidth);
 		}
-		sourcePixel += sourceStride;
 		targetPixel += targetStride;
+		sourcePixel += sourceStride;
 	}
 }
 
-void GfxCursor32::drawToHardware(const DrawRegion &source) {
+void GfxCursor32::drawToScreen(const DrawRegion &source) {
 	Common::Rect drawRect(source.rect);
-	drawRect.clip(_vmapRegion.rect);
+	drawRect.clip(_screenRegion.rect);
 	const int16 sourceXOffset = drawRect.left - source.rect.left;
 	const int16 sourceYOffset = drawRect.top - source.rect.top;
 	byte *sourcePixel = source.data + (sourceYOffset * source.rect.width()) + sourceXOffset;
@@ -174,7 +183,7 @@ void GfxCursor32::setRestrictedArea(const Common::Rect &rect) {
 }
 
 void GfxCursor32::clearRestrictedArea() {
-	_restrictedArea = _vmapRegion.rect;
+	_restrictedArea = _screenRegion.rect;
 }
 
 void GfxCursor32::setView(const GuiResourceId viewId, const int16 loopNo, const int16 celNo) {
@@ -279,58 +288,29 @@ void GfxCursor32::setView(const GuiResourceId viewId, const int16 loopNo, const
 		_cursor.rect = Common::Rect(_width, _height);
 		*_cursor.data = _cursor.skipColor;
 		_cursorBack.rect = _cursor.rect;
-		_cursorBack.rect.clip(_vmapRegion.rect);
+		_cursorBack.rect.clip(_screenRegion.rect);
 		if (!_cursorBack.rect.isEmpty()) {
-			readVideo(_cursorBack);
+			copyFromScreen(_cursorBack);
 		}
 	}
 
 	_cursorBack.data = (byte *)realloc(_cursorBack.data, _width * _height);
 	memset(_cursorBack.data, 0, _width * _height);
-	_drawBuff1.data = (byte *)realloc(_drawBuff1.data, _width * _height);
-	_drawBuff2.data = (byte *)realloc(_drawBuff2.data, _width * _height * 4);
-	_savedVmapRegion.data = (byte *)realloc(_savedVmapRegion.data, _width * _height);
+	_scratch1.data = (byte *)realloc(_scratch1.data, _width * _height);
+	_scratch2.data = (byte *)realloc(_scratch2.data, _width * _height * 4);
+	_savedScreenRegion.data = (byte *)realloc(_savedScreenRegion.data, _width * _height);
 
 	unhide();
 }
 
-void GfxCursor32::readVideo(DrawRegion &target) {
-	// NOTE: In SSCI, mouse events were received via hardware interrupt, so
-	// there was a separate branch here that would read from VRAM instead of
-	// from the game's back buffer when a mouse event was received while the
-	// back buffer was being updated. In ScummVM, mouse events are polled, which
-	// means it is not possible to receive a mouse event during a back buffer
-	// update, so the code responsible for handling that is removed.
-	copy(target, _vmapRegion);
-}
-
-void GfxCursor32::copy(DrawRegion &target, const DrawRegion &source) {
-	if (source.rect.isEmpty()) {
-		return;
-	}
-
-	Common::Rect drawRect(source.rect);
-	drawRect.clip(target.rect);
-	if (drawRect.isEmpty()) {
-		return;
-	}
-
-	const int16 sourceXOffset = drawRect.left - source.rect.left;
-	const int16 sourceYOffset = drawRect.top - source.rect.top;
-	const int16 drawWidth = drawRect.width();
-	const int16 drawHeight = drawRect.height();
-
-	byte *targetPixel = target.data + ((drawRect.top - target.rect.top) * target.rect.width()) + (drawRect.left - target.rect.left);
-	const byte *sourcePixel = source.data + (sourceYOffset * source.rect.width()) + sourceXOffset;
-
-	const int16 sourceStride = source.rect.width();
-	const int16 targetStride = target.rect.width();
-
-	for (int y = 0; y < drawHeight; ++y) {
-		memcpy(targetPixel, sourcePixel, drawWidth);
-		targetPixel += targetStride;
-		sourcePixel += sourceStride;
-	}
+void GfxCursor32::copyFromScreen(DrawRegion &target) {
+	// In SSCI, mouse events were received via hardware interrupt, so there was
+	// a separate branch here that would read from VRAM instead of from the
+	// game's back buffer when a mouse event was received while the back buffer
+	// was being updated. In ScummVM, mouse events are polled, which means it is
+	// not possible to receive a mouse event during a back buffer update, so the
+	// code responsible for handling that is removed.
+	copy<false>(target, _screenRegion);
 }
 
 void GfxCursor32::setPosition(const Common::Point &position) {
@@ -349,32 +329,32 @@ void GfxCursor32::setPosition(const Common::Point &position) {
 }
 
 void GfxCursor32::gonnaPaint(Common::Rect paintRect) {
-	if (!_hideCount && !_writeToVMAP && !_cursorBack.rect.isEmpty()) {
+	if (!_hideCount && !_needsPaint && !_cursorBack.rect.isEmpty()) {
 		paintRect.left &= ~3;
 		paintRect.right |= 3;
 		if (_cursorBack.rect.intersects(paintRect)) {
-			_writeToVMAP = true;
+			_needsPaint = true;
 		}
 	}
 }
 
 void GfxCursor32::paintStarting() {
-	if (_writeToVMAP) {
-		_savedVmapRegion.rect = _cursor.rect;
-		copy(_savedVmapRegion, _vmapRegion);
-		paint(_vmapRegion, _cursor);
+	if (_needsPaint) {
+		_savedScreenRegion.rect = _cursor.rect;
+		copy<false>(_savedScreenRegion, _screenRegion);
+		copy<true>(_screenRegion, _cursor);
 	}
 }
 
 void GfxCursor32::donePainting() {
-	if (_writeToVMAP) {
-		copy(_vmapRegion, _savedVmapRegion);
-		_savedVmapRegion.rect = Common::Rect();
-		_writeToVMAP = false;
+	if (_needsPaint) {
+		copy<false>(_screenRegion, _savedScreenRegion);
+		_savedScreenRegion.rect = Common::Rect();
+		_needsPaint = false;
 	}
 
 	if (!_hideCount && !_cursorBack.rect.isEmpty()) {
-		copy(_cursorBack, _vmapRegion);
+		copy<false>(_cursorBack, _screenRegion);
 	}
 }
 
@@ -423,45 +403,45 @@ void GfxCursor32::move() {
 	}
 
 	// Cursor moved offscreen
-	if (!_cursor.rect.intersects(_vmapRegion.rect)) {
-		drawToHardware(_cursorBack);
+	if (!_cursor.rect.intersects(_screenRegion.rect)) {
+		drawToScreen(_cursorBack);
 		return;
 	}
 
 	if (!_cursor.rect.intersects(_cursorBack.rect)) {
 		// Cursor moved to a completely different part of the screen
-		_drawBuff1.rect = _cursor.rect;
-		_drawBuff1.rect.clip(_vmapRegion.rect);
-		readVideo(_drawBuff1);
+		_scratch1.rect = _cursor.rect;
+		_scratch1.rect.clip(_screenRegion.rect);
+		copyFromScreen(_scratch1);
 
-		_drawBuff2.rect = _drawBuff1.rect;
-		copy(_drawBuff2, _drawBuff1);
+		_scratch2.rect = _scratch1.rect;
+		copy<false>(_scratch2, _scratch1);
 
-		paint(_drawBuff1, _cursor);
-		drawToHardware(_drawBuff1);
+		copy<true>(_scratch1, _cursor);
+		drawToScreen(_scratch1);
 
-		drawToHardware(_cursorBack);
+		drawToScreen(_cursorBack);
 
 		_cursorBack.rect = _cursor.rect;
-		_cursorBack.rect.clip(_vmapRegion.rect);
-		copy(_cursorBack, _drawBuff2);
+		_cursorBack.rect.clip(_screenRegion.rect);
+		copy<false>(_cursorBack, _scratch2);
 	} else {
 		// Cursor moved, but still overlaps the previous cursor location
 		Common::Rect mergedRect(_cursorBack.rect);
 		mergedRect.extend(_cursor.rect);
-		mergedRect.clip(_vmapRegion.rect);
+		mergedRect.clip(_screenRegion.rect);
 
-		_drawBuff2.rect = mergedRect;
-		readVideo(_drawBuff2);
+		_scratch2.rect = mergedRect;
+		copyFromScreen(_scratch2);
 
-		copy(_drawBuff2, _cursorBack);
+		copy<false>(_scratch2, _cursorBack);
 
 		_cursorBack.rect = _cursor.rect;
-		_cursorBack.rect.clip(_vmapRegion.rect);
-		copy(_cursorBack, _drawBuff2);
+		_cursorBack.rect.clip(_screenRegion.rect);
+		copy<false>(_cursorBack, _scratch2);
 
-		paint(_drawBuff2, _cursor);
-		drawToHardware(_drawBuff2);
+		copy<true>(_scratch2, _cursor);
+		drawToScreen(_scratch2);
 	}
 }
 
diff --git a/engines/sci/graphics/cursor32.h b/engines/sci/graphics/cursor32.h
index 562a2c50de..fb07c49dbb 100644
--- a/engines/sci/graphics/cursor32.h
+++ b/engines/sci/graphics/cursor32.h
@@ -23,6 +23,7 @@
 #ifndef SCI_GRAPHICS_CURSOR32_H
 #define SCI_GRAPHICS_CURSOR32_H
 
+#include "common/array.h"           // for Array
 #include "common/rect.h"            // for Point, Rect
 #include "common/scummsys.h"        // for int16, byte, uint8
 #include "common/serializer.h"      // for Serializable, Serializer (ptr only)
@@ -37,11 +38,10 @@ public:
 	~GfxCursor32();
 
 	/**
-	 * Initialises the cursor system with the given
-	 * buffer to use as the output buffer for
-	 * rendering the cursor.
+	 * Initialises the cursor system with the given buffer to use as the output
+	 * buffer for rendering the cursor.
 	 */
-	void init(const Buffer &vmap);
+	void init(const Buffer &outputBuffer);
 
 	/**
 	 * Called when the hardware mouse moves.
@@ -49,41 +49,35 @@ public:
 	bool deviceMoved(Common::Point &position);
 
 	/**
-	 * Called by GfxFrameout once for each show
-	 * rectangle that is going to be drawn to
-	 * hardware.
+	 * Called by GfxFrameout once for each show rectangle that is going to be
+	 * drawn to hardware.
 	 */
 	void gonnaPaint(Common::Rect paintRect);
 
 	/**
-	 * Called by GfxFrameout when the rendering to
-	 * hardware begins.
+	 * Called by GfxFrameout when the rendering to hardware begins.
 	 */
 	void paintStarting();
 
 	/**
-	 * Called by GfxFrameout when the output buffer
-	 * has finished rendering to hardware.
+	 * Called by GfxFrameout when the output buffer has finished rendering to
+	 * hardware.
 	 */
 	void donePainting();
 
 	/**
-	 * Hides the cursor. Each call to `hide` will
-	 * increment a hide counter, which must be
-	 * returned to 0 before the cursor will be
-	 * shown again.
+	 * Hides the cursor. Each call to `hide` will increment a hide counter,
+	 * which must be returned to 0 before the cursor will be shown again.
 	 */
 	void hide();
 
 	/**
-	 * Shows the cursor, if the hide counter is
-	 * returned to 0.
+	 * Shows the cursor, if the hide counter is returned to 0.
 	 */
 	void unhide();
 
 	/**
-	 * Shows the cursor regardless of the state of
-	 * the hide counter.
+	 * Shows the cursor regardless of the state of the hide counter.
 	 */
 	void show();
 
@@ -93,14 +87,12 @@ public:
 	void setView(const GuiResourceId viewId, const int16 loopNo, const int16 celNo);
 
 	/**
-	 * Explicitly sets the position of the cursor,
-	 * in game script coordinates.
+	 * Explicitly sets the position of the cursor, in game script coordinates.
 	 */
 	void setPosition(const Common::Point &position);
 
 	/**
-	 * Sets the region that the mouse is allowed
-	 * to move within.
+	 * Sets the region that the mouse is allowed to move within.
 	 */
 	void setRestrictedArea(const Common::Rect &rect);
 
@@ -109,10 +101,6 @@ public:
 	 */
 	void clearRestrictedArea();
 
-#ifdef ENABLE_SCI32_MAC
-	void setMacCursorRemapList(int cursorCount, reg_t *cursors);
-#endif
-
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
 private:
@@ -121,42 +109,41 @@ private:
 		byte *data;
 		uint8 skipColor;
 
-		DrawRegion() : rect(), data(nullptr) {}
+		DrawRegion() : data(nullptr) {}
 	};
 
 	/**
-	 * Information about the current cursor.
-	 * Used to restore cursor when loading a
-	 * savegame.
+	 * Information about the current cursor. Used to restore cursor when loading
+	 * a savegame.
 	 */
 	CelInfo32 _cursorInfo;
 
 	/**
-	 * Content behind the cursor? TODO
+	 * The content of the frame buffer which was behind the cursor prior to its
+	 * being drawn.
 	 */
 	DrawRegion _cursorBack;
 
 	/**
 	 * Scratch buffer.
 	 */
-	DrawRegion _drawBuff1;
+	DrawRegion _scratch1;
 
 	/**
 	 * Scratch buffer 2.
 	 */
-	DrawRegion _drawBuff2;
+	DrawRegion _scratch2;
 
 	/**
-	 * A draw region representing the current
-	 * output buffer.
+	 * A draw region representing the entire output buffer.
 	 */
-	DrawRegion _vmapRegion;
+	DrawRegion _screenRegion;
 
 	/**
-	 * The content behind the cursor in the
+	 * The region behind the cursor immediately before it is painted to the
 	 * output buffer.
 	 */
-	DrawRegion _savedVmapRegion;
+	DrawRegion _savedScreenRegion;
 
 	/**
 	 * The cursor bitmap.
@@ -164,93 +151,82 @@ private:
 	DrawRegion _cursor;
 
 	/**
-	 * The width and height of the cursor,
-	 * in screen coordinates.
+	 * The width and height of the cursor, in screen coordinates.
 	 */
 	int16 _width, _height;
 
 	/**
-	 * The output buffer where the cursor is
-	 * rendered.
+	 * The output buffer where the cursor is rendered.
 	 */
-	Buffer _vmap;
+	Buffer _screen;
 
 	/**
-	 * The number of times the cursor has been
-	 * hidden.
+	 * The number of times the cursor has been hidden.
 	 */
 	int _hideCount;
 
 	/**
-	 * The rendered position of the cursor, in
-	 * screen coordinates.
+	 * The rendered position of the cursor, in screen coordinates.
 	 */
 	Common::Point _position;
 
 	/**
-	 * The position of the cursor hot spot, relative
-	 * to the cursor origin, in screen pixels.
+	 * The position of the cursor hot spot, relative to the cursor origin, in
+	 * screen pixels.
 	 */
 	Common::Point _hotSpot;
 
 	/**
-	 * The area within which the cursor is allowed
-	 * to move, in screen pixels.
+	 * The area within which the cursor is allowed to move, in screen pixels.
 	 */
 	Common::Rect _restrictedArea;
 
 	/**
-	 * Indicates whether or not the cursor needs to
-	 * be repainted on the output buffer due to a
-	 * change of graphics in the area underneath the
-	 * cursor.
-	 */
-	bool _writeToVMAP;
-
-	// Mac versions of games use a remap list to remap their cursors
-	Common::Array<uint16> _macCursorRemap;
-
-	/**
-	 * Reads data from the output buffer or hardware
-	 * to the given draw region.
+	 * Indicates whether or not the cursor needs to be repainted on the output
+	 * buffer due to a change of graphics in the area underneath the cursor.
 	 */
-	void readVideo(DrawRegion &target);
+	bool _needsPaint;
 
 	/**
-	 * Reads data from the output buffer to the
-	 * given draw region.
+	 * Reads data from the output buffer to the given draw region.
 	 */
-	void readVideoFromVmap(DrawRegion &target);
+	void copyFromScreen(DrawRegion &target);
 
 	/**
-	 * Copies pixel data from the given source to
-	 * the given target.
+	 * Copies pixel data from the given source to the given target. If SKIP is
+	 * true, pixels that match the `skipColor` property of the source will be
+	 * skipped.
+	 *
+	 * @note In SSCI, the function that did not handle skip color was called
+	 * `copy` and the one that did was called `paint`.
 	 */
+	template <bool SKIP>
 	void copy(DrawRegion &target, const DrawRegion &source);
 
 	/**
-	 * Draws from the given source onto the given
-	 * target, skipping pixels in the source that
-	 * match the `skipColor` property.
-	 */
-	void paint(DrawRegion &target, const DrawRegion &source);
-
-	/**
-	 * Draws the cursor to the position it was
-	 * drawn to prior to moving offscreen or being
-	 * hidden by a call to `hide`.
+	 * Draws the cursor to the position it was drawn to prior to moving
+	 * offscreen or being hidden by a call to `hide`.
 	 */
 	void revealCursor();
 
 	/**
 	 * Draws the given source to the output buffer.
 	 */
-	void drawToHardware(const DrawRegion &source);
+	void drawToScreen(const DrawRegion &source);
 
 	/**
 	 * Renders the cursor at its new location.
 	 */
 	void move();
+
+#ifdef ENABLE_SCI32_MAC
+public:
+	void setMacCursorRemapList(int cursorCount, reg_t *cursors);
+
+private:
+	// Mac versions of games use a remap list to remap their cursors
+	Common::Array<uint16> _macCursorRemap;
+#endif
 };
 
 } // End of namespace Sci