SCI32: Fix slow SCI2.1mid transitions

SSCI transitions code sends a large number of small show rects
to the graphics manager, one at a time, for each division of a
transition. Each time a rect is submitted, a call to showBits
is made. This design was used when transitions for SCI32 were
first implemented in ScummVM, and it worked OK because the
hardware surface was updated by EventManager::getSciEvent,
not showBits, so the large number of calls to showBits from the
transitions code did not adversely affect engine performance.

Later in SCI32 engine development, hardware surface updates
were changed to occur in showBits so that the hardware surface
would be updated at frame-out time, instead of at input-in time.
This change meant that now the large number of calls to showBits
from transitions became very expensive, and the engine would
stall constantly refreshing the entire hardware surface.

To fix this problem, the transitions code now (1) maximises the
size of rects coming from transitions, when possible, and (2) only
calls showBits when all the rects from one frame of a transition
have been calculated and added to the show rects list.

Additionally, there were some arithmetic errors in the
implementation of pixel dissolve that have been corrected in this
changeset.

Fixes Trac#9614.

4 files changed, 59 insertions, 46 deletions

diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 843fe5ed86..bf43c8b63b 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -1272,14 +1272,6 @@ void GfxFrameout::throttle() {
 	}
 }
 
-void GfxFrameout::showRect(const Common::Rect &rect) {
-	if (!rect.isEmpty()) {
-		_showList.clear();
-		_showList.add(rect);
-		showBits();
-	}
-}
-
 void GfxFrameout::shakeScreen(int16 numShakes, const ShakeDirection direction) {
 	if (direction & kShakeHorizontal) {
 		// Used by QFG4 room 750
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 9481b0ef5a..9738203b74 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -39,6 +39,7 @@ struct PlaneShowStyle;
  * Roughly equivalent to GraphicsMgr in the actual SCI engine.
  */
 class GfxFrameout {
+	friend class GfxTransitions32;
 private:
 	GfxCursor32 *_cursor;
 	GfxPalette32 *_palette;
diff --git a/engines/sci/graphics/transitions32.cpp b/engines/sci/graphics/transitions32.cpp
index 330b9bedbf..ddcb50b140 100644
--- a/engines/sci/graphics/transitions32.cpp
+++ b/engines/sci/graphics/transitions32.cpp
@@ -81,6 +81,23 @@ void GfxTransitions32::throttle() {
 	g_sci->getEngineState()->_throttleTrigger = true;
 }
 
+void GfxTransitions32::clearShowRects() {
+	g_sci->_gfxFrameout->_showList.clear();
+}
+
+void GfxTransitions32::addShowRect(const Common::Rect &rect) {
+	if (!rect.isEmpty()) {
+		g_sci->_gfxFrameout->_showList.add(rect);
+	}
+}
+
+void GfxTransitions32::sendShowRects() {
+	g_sci->_gfxFrameout->showBits();
+	g_sci->getSciDebugger()->onFrame();
+	clearShowRects();
+	throttle();
+}
+
 #pragma mark -
 #pragma mark Show styles
 
@@ -546,7 +563,7 @@ void GfxTransitions32::processHShutterOut(PlaneShowStyle &showStyle) {
 	error("HShutterOut is not known to be used by any game. Please submit a bug report with details about the game you were playing and what you were doing that triggered this error. Thanks!");
 }
 
-void GfxTransitions32::processHShutterIn(PlaneShowStyle &showStyle) {
+void GfxTransitions32::processHShutterIn(const PlaneShowStyle &showStyle) {
 	if (getSciVersion() <= SCI_VERSION_2_1_EARLY) {
 		error("HShutterIn is not known to be used by any SCI2.1early- game. Please submit a bug report with details about the game you were playing and what you were doing that triggered this error. Thanks!");
 	}
@@ -559,36 +576,37 @@ void GfxTransitions32::processHShutterIn(PlaneShowStyle &showStyle) {
 	const int width = screenRect.width();
 	const int divisionWidth = width / divisions - 1;
 
+	clearShowRects();
+
 	if (width % divisions) {
 		rect.left = (divisionWidth + 1) * divisions + screenRect.left;
 		rect.top = screenRect.top;
 		rect.right = (divisionWidth + 1) * divisions + (width % divisions) + screenRect.left;
 		rect.bottom = screenRect.bottom;
-		g_sci->_gfxFrameout->showRect(rect);
+		addShowRect(rect);
+		sendShowRects();
 	}
 
-	throttle();
-
 	for (int i = 0; i < width / (2 * divisions); ++i) {
 		// Left side
 		rect.left = i * divisions + screenRect.left;
 		rect.top = screenRect.top;
 		rect.right = i * divisions + divisions + screenRect.left;
 		rect.bottom = screenRect.bottom;
-		g_sci->_gfxFrameout->showRect(rect);
+		addShowRect(rect);
 
 		// Right side
 		rect.left = (divisionWidth - i) * divisions + screenRect.left;
 		rect.top = screenRect.top;
 		rect.right = (divisionWidth - i) * divisions + divisions + screenRect.left;
 		rect.bottom = screenRect.bottom;
-		g_sci->_gfxFrameout->showRect(rect);
+		addShowRect(rect);
 
-		throttle();
+		sendShowRects();
 	}
 
-	g_sci->_gfxFrameout->showRect(screenRect);
-	throttle();
+	addShowRect(screenRect);
+	sendShowRects();
 }
 
 void GfxTransitions32::processVShutterOut(PlaneShowStyle &showStyle) {
@@ -743,7 +761,7 @@ bool GfxTransitions32::processPixelDissolve21Early(PlaneShowStyle &showStyle) {
 	return false;
 }
 
-bool GfxTransitions32::processPixelDissolve21Mid(PlaneShowStyle &showStyle) {
+bool GfxTransitions32::processPixelDissolve21Mid(const PlaneShowStyle &showStyle) {
 	// SQ6 room 530
 
 	Plane* plane = g_sci->_gfxFrameout->getVisiblePlanes().findByObject(showStyle.plane);
@@ -760,44 +778,42 @@ bool GfxTransitions32::processPixelDissolve21Mid(PlaneShowStyle &showStyle) {
 	int seq = 1;
 
 	uint iteration = 0;
-	const uint numIterationsPerTick = (width * height + divisions) / divisions;
+	const uint numIterationsPerTick = ARRAYSIZE(g_sci->_gfxFrameout->_showList);
+
+	clearShowRects();
 
 	do {
 		int row = seq / width;
 		int col = seq % width;
 
 		if (row < height) {
-			if (row == height && (planeHeight % divisions)) {
-				if (col == width && (planeWidth % divisions)) {
+			if (row == height - 1 && (planeHeight % divisions)) {
+				if (col == width - 1 && (planeWidth % divisions)) {
 					rect.left = col * divisions;
 					rect.top = row * divisions;
 					rect.right = col * divisions + (planeWidth % divisions);
 					rect.bottom = row * divisions + (planeHeight % divisions);
-					rect.clip(screenRect);
-					g_sci->_gfxFrameout->showRect(rect);
+					addShowRect(rect);
 				} else {
 					rect.left = col * divisions;
 					rect.top = row * divisions;
-					rect.right = col * divisions * 2;
+					rect.right = col * divisions + divisions;
 					rect.bottom = row * divisions + (planeHeight % divisions);
-					rect.clip(screenRect);
-					g_sci->_gfxFrameout->showRect(rect);
+					addShowRect(rect);
 				}
 			} else {
-				if (col == width && (planeWidth % divisions)) {
+				if (col == width - 1 && (planeWidth % divisions)) {
 					rect.left = col * divisions;
 					rect.top = row * divisions;
-					rect.right = col * divisions + (planeWidth % divisions) + 1;
-					rect.bottom = row * divisions * 2 + 1;
-					rect.clip(screenRect);
-					g_sci->_gfxFrameout->showRect(rect);
+					rect.right = col * divisions + (planeWidth % divisions);
+					rect.bottom = row * divisions + divisions;
+					addShowRect(rect);
 				} else {
 					rect.left = col * divisions;
 					rect.top = row * divisions;
-					rect.right = col * divisions * 2 + 1;
-					rect.bottom = row * divisions * 2 + 1;
-					rect.clip(screenRect);
-					g_sci->_gfxFrameout->showRect(rect);
+					rect.right = col * divisions + divisions;
+					rect.bottom = row * divisions + divisions;
+					addShowRect(rect);
 				}
 			}
 		}
@@ -809,20 +825,21 @@ bool GfxTransitions32::processPixelDissolve21Mid(PlaneShowStyle &showStyle) {
 		}
 
 		if (++iteration == numIterationsPerTick) {
-			throttle();
+			sendShowRects();
 			iteration = 0;
 		}
-	} while(seq != 1 && !g_engine->shouldQuit());
+	} while (seq != 1 && !g_engine->shouldQuit());
 
 	rect.left = screenRect.left;
 	rect.top = screenRect.top;
 	rect.right = divisions + screenRect.left;
 	rect.bottom = divisions + screenRect.bottom;
-	rect.clip(screenRect);
-	g_sci->_gfxFrameout->showRect(rect);
-	throttle();
+	addShowRect(rect);
+	sendShowRects();
+
+	addShowRect(screenRect);
+	sendShowRects();
 
-	g_sci->_gfxFrameout->showRect(screenRect);
 	return true;
 }
 
diff --git a/engines/sci/graphics/transitions32.h b/engines/sci/graphics/transitions32.h
index 3968378a3c..12e0d64ec7 100644
--- a/engines/sci/graphics/transitions32.h
+++ b/engines/sci/graphics/transitions32.h
@@ -239,13 +239,16 @@ private:
 	SegManager *_segMan;
 
 	/**
-	 * Throttles transition playback to prevent
-	 * transitions from being instant on fast
-	 * computers.
+	 * Throttles transition playback to prevent transitions from being instant
+	 * on fast computers.
 	 */
 	void throttle();
 	int8 _throttleState;
 
+	void clearShowRects();
+	void addShowRect(const Common::Rect &rect);
+	void sendShowRects();
+
 #pragma mark -
 #pragma mark Show styles
 public:
@@ -352,7 +355,7 @@ private:
 	 * Performs a transition that renders to black
 	 * with a horizontal shutter effect.
 	 */
-	void processHShutterIn(PlaneShowStyle &showStyle);
+	void processHShutterIn(const PlaneShowStyle &showStyle);
 
 	/**
 	 * Performs a transition that renders into a room
@@ -424,7 +427,7 @@ private:
 	 * SCI2.1mid and later implementation of
 	 * pixel dissolve.
 	 */
-	bool processPixelDissolve21Mid(PlaneShowStyle &showStyle);
+	bool processPixelDissolve21Mid(const PlaneShowStyle &showStyle);
 
 	/**
 	 * Performs a transition that fades to black