[Scummvm-cvs-logs] scummvm master -> 948e4487389f6ae99d4a61e0cf6d995d16e62c82

csnover csnover at users.noreply.github.com
Fri Jul 1 23:42:14 CEST 2016


This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
6c8661d144 SCI32: Fix bad rendering of subtitle backgrounds in Torin
948e448738 SCI32: Fix signed comparison warnings


Commit: 6c8661d144ddf5f36b22d70169c3005f830f08d4
    https://github.com/scummvm/scummvm/commit/6c8661d144ddf5f36b22d70169c3005f830f08d4
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-07-01T15:54:27-05:00

Commit Message:
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.

Changed paths:
    engines/sci/graphics/celobj32.cpp
    engines/sci/graphics/screen_item32.cpp



diff --git a/engines/sci/graphics/celobj32.cpp b/engines/sci/graphics/celobj32.cpp
index befa5cd..f8cd5fd 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 c1644a5..ebaf132 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);


Commit: 948e4487389f6ae99d4a61e0cf6d995d16e62c82
    https://github.com/scummvm/scummvm/commit/948e4487389f6ae99d4a61e0cf6d995d16e62c82
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-07-01T15:58:43-05:00

Commit Message:
SCI32: Fix signed comparison warnings

Changed paths:
    engines/sci/graphics/frameout.cpp
    engines/sci/graphics/plane32.h



diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index fd37020..140c27a 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -696,7 +696,7 @@ void GfxFrameout::calcLists(ScreenItemListList &drawLists, EraseListList &eraseL
 	}
 
 	PlaneList::size_type planeCount = _planes.size();
-	for (int outerPlaneIndex = 0; outerPlaneIndex < planeCount; ++outerPlaneIndex) {
+	for (PlaneList::size_type outerPlaneIndex = 0; outerPlaneIndex < planeCount; ++outerPlaneIndex) {
 		const Plane *outerPlane = _planes[outerPlaneIndex];
 		const Plane *visiblePlane = _visiblePlanes.findByObject(outerPlane->_object);
 
@@ -742,7 +742,7 @@ void GfxFrameout::calcLists(ScreenItemListList &drawLists, EraseListList &eraseL
 		}
 
 		if (addedToEraseList) {
-			for (int rectIndex = 0; rectIndex < eraseList.size(); ++rectIndex) {
+			for (RectList::size_type rectIndex = 0; rectIndex < eraseList.size(); ++rectIndex) {
 				const Common::Rect &rect = *eraseList[rectIndex];
 				for (int innerPlaneIndex = planeCount - 1; innerPlaneIndex >= 0; --innerPlaneIndex) {
 					const Plane &innerPlane = *_planes[innerPlaneIndex];
@@ -813,7 +813,7 @@ void GfxFrameout::calcLists(ScreenItemListList &drawLists, EraseListList &eraseL
 
 			eraseList.add(outerPlane._screenRect.findIntersectingRect(visibleOuterPlane->_screenRect));
 
-			for (PlaneList::size_type innerIndex = planeCount - 1; innerIndex >= 0; --innerIndex) {
+			for (int innerIndex = (int)planeCount - 1; innerIndex >= 0; --innerIndex) {
 				// "inner" just refers to the inner loop
 				const Plane &innerPlane = *_planes[innerIndex];
 				const Plane *visibleInnerPlane = _visiblePlanes.findByObject(innerPlane._object);
@@ -903,7 +903,7 @@ void GfxFrameout::calcLists(ScreenItemListList &drawLists, EraseListList &eraseL
 			}
 
 			if (_planes[planeIndex]->_type == kPlaneTypeTransparent) {
-				for (PlaneList::size_type i = planeIndex - 1; i >= 0; --i) {
+				for (int i = (int)planeIndex - 1; i >= 0; --i) {
 					_planes[i]->filterDownEraseRects(drawLists[i], eraseLists[i], eraseLists[planeIndex]);
 				}
 
diff --git a/engines/sci/graphics/plane32.h b/engines/sci/graphics/plane32.h
index acd535e..a173926 100644
--- a/engines/sci/graphics/plane32.h
+++ b/engines/sci/graphics/plane32.h
@@ -495,8 +495,6 @@ private:
 	using PlaneListBase::push_back;
 
 public:
-	typedef int size_type;
-
 	// A method for finding the index of a plane inside a
 	// PlaneList is used because entries in the main plane
 	// list and visible plane list of GfxFrameout are






More information about the Scummvm-git-logs mailing list