SCI32: Fix bad rendering of subtitle backgrounds in Torin

The way dimensions of scaled screen items are calculated changed
over the lifetime of SSCI. In early low-resolution and
mixed-resolution games, scaled drawing needed to use at a global
cadence across the entire screen to ensure proper alignment, but
in later games (like Torin), local scaling of individual screen
items seems to be the way scaling is performed.

2 files changed, 97 insertions, 33 deletions

diff --git a/engines/sci/graphics/celobj32.cpp b/engines/sci/graphics/celobj32.cpp
index befa5cda18..f8cd5fd171 100644
--- a/engines/sci/graphics/celobj32.cpp
+++ b/engines/sci/graphics/celobj32.cpp
@@ -176,34 +176,61 @@ struct SCALER_Scale {
 	// line of source data when scaling
 	_reader(celObj, celObj._width) {
 		// In order for scaling ratios to apply equally across objects that
-		// start at different positions on the screen, the pixels that are
-		// read from the source bitmap must all use the same pattern of
-		// division. In other words, cels must follow the same scaling pattern
-		// as if they were drawn starting at an even multiple of the scaling
-		// ratio, even if they were not.
+		// start at different positions on the screen (like the cels of a
+		// picture), the pixels that are read from the source bitmap must all
+		// use the same pattern of division. In other words, cels must follow
+		// a global scaling pattern as if they were always drawn starting at an
+		// even multiple of the scaling ratio, even if they are not.
 		//
 		// To get the correct source pixel when reading out through the scaler,
 		// the engine creates a lookup table for each axis that translates
 		// directly from target positions to the indexes of source pixels using
 		// the global cadence for the given scaling ratio.
+		//
+		// Note, however, that not all games use the global scaling mode.
+		//
+		// SQ6 definitely uses the global scaling mode (an easy visual
+		// comparison is to leave Implants N' Stuff and then look at Roger);
+		// Torin definitely does not (scaling subtitle backgrounds will cause it
+		// to attempt a read out of bounds and crash). They are both SCI
+		// "2.1mid" games, so currently the common denominator looks to be that
+		// games which use global scaling are the ones that use low-resolution
+		// script coordinates too.
 
 		const CelScalerTable *table = CelObj::_scaler->getScalerTable(scaleX, scaleY);
 
-		const int16 unscaledX = (scaledPosition.x / scaleX).toInt();
-		if (FLIP) {
-			int lastIndex = celObj._width - 1;
-			for (int16 x = targetRect.left; x < targetRect.right; ++x) {
-				_valuesX[x] = lastIndex - (table->valuesX[x] - unscaledX);
+		if (g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth == kLowResX) {
+			const int16 unscaledX = (scaledPosition.x / scaleX).toInt();
+			if (FLIP) {
+				int lastIndex = celObj._width - 1;
+				for (int16 x = targetRect.left; x < targetRect.right; ++x) {
+					_valuesX[x] = lastIndex - (table->valuesX[x] - unscaledX);
+				}
+			} else {
+				for (int16 x = targetRect.left; x < targetRect.right; ++x) {
+					_valuesX[x] = table->valuesX[x] - unscaledX;
+				}
+			}
+
+			const int16 unscaledY = (scaledPosition.y / scaleY).toInt();
+			for (int16 y = targetRect.top; y < targetRect.bottom; ++y) {
+				_valuesY[y] = table->valuesY[y] - unscaledY;
 			}
 		} else {
-			for (int16 x = targetRect.left; x < targetRect.right; ++x) {
-				_valuesX[x] = table->valuesX[x] - unscaledX;
+			if (FLIP) {
+				int lastIndex = celObj._width - 1;
+				for (int16 x = 0; x < targetRect.width(); ++x) {
+					_valuesX[targetRect.left + x] = lastIndex - table->valuesX[x];
+				}
+			} else {
+				for (int16 x = 0; x < targetRect.width(); ++x) {
+					_valuesX[targetRect.left + x] = table->valuesX[x];
+				}
 			}
-		}
 
-		const int16 unscaledY = (scaledPosition.y / scaleY).toInt();
-		for (int16 y = targetRect.top; y < targetRect.bottom; ++y) {
-			_valuesY[y] = table->valuesY[y] - unscaledY;
+			for (int16 y = 0; y < targetRect.height(); ++y) {
+				_valuesY[targetRect.top + y] = table->valuesY[y];
+			}
 		}
 	}
 
diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index c1644a5ea3..ebaf132890 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -296,7 +296,30 @@ void ScreenItem::calcRects(const Plane &plane) {
 			}
 
 			if (!scaleX.isOne() || !scaleY.isOne()) {
-				mulinc(_screenItemRect, scaleX, scaleY);
+				// Different games use a different cel scaling mode, but the
+				// difference isn't consistent across SCI versions; instead,
+				// it seems to be related to an update that happened during
+				// SCI2.1mid where games started using hi-resolution game
+				// scripts
+				if (scriptWidth == kLowResX) {
+					mulinc(_screenItemRect, scaleX, scaleY);
+				} else {
+					_screenItemRect.left = (_screenItemRect.left * scaleX).toInt();
+					_screenItemRect.top = (_screenItemRect.top * scaleY).toInt();
+
+					if (scaleX.getNumerator() > scaleX.getDenominator()) {
+						_screenItemRect.right = (_screenItemRect.right * scaleX).toInt();
+					} else {
+						_screenItemRect.right = ((_screenItemRect.right - 1) * scaleX).toInt() + 1;
+					}
+
+					if (scaleY.getNumerator() > scaleY.getDenominator()) {
+						_screenItemRect.bottom = (_screenItemRect.bottom * scaleY).toInt();
+					} else {
+						_screenItemRect.bottom = ((_screenItemRect.bottom - 1) * scaleY).toInt() + 1;
+					}
+				}
+
 				displaceX = (displaceX * scaleX).toInt();
 				displaceY = (displaceY * scaleY).toInt();
 			}
@@ -538,8 +561,6 @@ void ScreenItem::update() {
 	_celObj = nullptr;
 }
 
-// TODO: This code is quite similar to calcRects, so try to deduplicate
-// if possible
 Common::Rect ScreenItem::getNowSeenRect(const Plane &plane) const {
 	CelObj &celObj = getCelObj();
 
@@ -547,10 +568,7 @@ Common::Rect ScreenItem::getNowSeenRect(const Plane &plane) const {
 	Common::Rect nsRect;
 
 	if (_useInsetRect) {
-		// TODO: This is weird. Checking to see if the inset rect is
-		// fully inside the bounds of the celObjRect, and then
-		// clipping to the celObjRect, is pretty useless.
-		if (_insetRect.right > 0 && _insetRect.bottom > 0 && _insetRect.left < celObj._width && _insetRect.top < celObj._height) {
+		if (_insetRect.intersects(celObjRect)) {
 			nsRect = _insetRect;
 			nsRect.clip(celObjRect);
 		} else {
@@ -594,10 +612,7 @@ Common::Rect ScreenItem::getNowSeenRect(const Plane &plane) const {
 			Ratio scriptToCelY(celObj._scaledHeight, scriptHeight);
 			mulru(nsRect, scriptToCelX, scriptToCelY, 0);
 
-			// TODO: This is weird. Checking to see if the inset rect is
-			// fully inside the bounds of the celObjRect, and then
-			// clipping to the celObjRect, is pretty useless.
-			if (nsRect.right > 0 && nsRect.bottom > 0 && nsRect.left < celObj._width && nsRect.top < celObj._height) {
+			if (nsRect.intersects(celObjRect)) {
 				nsRect.clip(celObjRect);
 			} else {
 				nsRect = Common::Rect();
@@ -605,12 +620,34 @@ Common::Rect ScreenItem::getNowSeenRect(const Plane &plane) const {
 		}
 
 		if (!scaleX.isOne() || !scaleY.isOne()) {
-			mulinc(nsRect, scaleX, scaleY);
-			// TODO: This was in the original code, baked into the
-			// multiplication though it is not immediately clear
-			// why this is the only one that reduces the BR corner
-			nsRect.right -= 1;
-			nsRect.bottom -= 1;
+			// Different games use a different cel scaling mode, but the
+			// difference isn't consistent across SCI versions; instead,
+			// it seems to be related to an update that happened during
+			// SCI2.1mid where games started using hi-resolution game
+			// scripts
+			if (scriptWidth == kLowResX) {
+				mulinc(nsRect, scaleX, scaleY);
+				// TODO: This was in the original code, baked into the
+				// multiplication though it is not immediately clear
+				// why this is the only one that reduces the BR corner
+				nsRect.right -= 1;
+				nsRect.bottom -= 1;
+			} else {
+				nsRect.left = (nsRect.left * scaleX).toInt();
+				nsRect.top = (nsRect.top * scaleY).toInt();
+
+				if (scaleX.getNumerator() > scaleX.getDenominator()) {
+					nsRect.right = (nsRect.right * scaleX).toInt();
+				} else {
+					nsRect.right = ((nsRect.right - 1) * scaleX).toInt() + 1;
+				}
+
+				if (scaleY.getNumerator() > scaleY.getDenominator()) {
+					nsRect.bottom = (nsRect.bottom * scaleY).toInt();
+				} else {
+					nsRect.bottom = ((nsRect.bottom - 1) * scaleY).toInt() + 1;
+				}
+			}
 		}
 
 		Ratio celToScriptX(scriptWidth, celObj._scaledWidth);