[Scummvm-cvs-logs] scummvm master -> 2e75f4b2b898866ffeafca5c69709dd3e358dcb5

csnover csnover at users.noreply.github.com
Wed Mar 2 07:03:44 CET 2016


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

Summary:
e5b30fb9af SCI32: Fix bad reads of mirrored cels
97d5e92185 SCI32: Review rect rounding in Plane and ScreenItem
2e75f4b2b8 SCI32: Add consts and remove redundant conditional


Commit: e5b30fb9afa3a42e08525980257868a73660937d
    https://github.com/scummvm/scummvm/commit/e5b30fb9afa3a42e08525980257868a73660937d
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-03-01T21:12:31-06:00

Commit Message:
SCI32: Fix bad reads of mirrored cels

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



diff --git a/engines/sci/graphics/celobj32.cpp b/engines/sci/graphics/celobj32.cpp
index f8bce26..62db02b 100644
--- a/engines/sci/graphics/celobj32.cpp
+++ b/engines/sci/graphics/celobj32.cpp
@@ -109,8 +109,8 @@ struct SCALER_NoScale {
 	const int16 _lastIndex;
 
 	SCALER_NoScale(const CelObj &celObj, const int16 maxWidth) :
-	_reader(celObj, maxWidth),
-	_lastIndex(maxWidth - 1) {}
+	_reader(celObj, FLIP ? celObj._width : maxWidth),
+	_lastIndex(celObj._width - 1) {}
 
 	inline void setSource(const int16 x, const int16 y) {
 		_row = _reader.getRow(y);


Commit: 97d5e9218519049bf7d4403682129173ef5d44e3
    https://github.com/scummvm/scummvm/commit/97d5e9218519049bf7d4403682129173ef5d44e3
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-03-02T00:01:13-06:00

Commit Message:
SCI32: Review rect rounding in Plane and ScreenItem

These changes should cause ScummVM to be more accurate in edge
case rounding.

Changed paths:
    engines/sci/graphics/helpers.h
    engines/sci/graphics/plane32.cpp
    engines/sci/graphics/screen_item32.cpp



diff --git a/engines/sci/graphics/helpers.h b/engines/sci/graphics/helpers.h
index 1d16b49..19dddd7 100644
--- a/engines/sci/graphics/helpers.h
+++ b/engines/sci/graphics/helpers.h
@@ -154,13 +154,13 @@ inline void mulinc(Common::Rect &rect, const Common::Rational &ratioX, const Com
  * Multiplies a number by a rational number, rounding up to
  * the nearest whole number.
  */
-inline int mulru(const int value, const Common::Rational &ratio) {
-	int num = value * ratio.getNumerator();
+inline int mulru(const int value, const Common::Rational &ratio, const int extra = 0) {
+	int num = (value + extra) * ratio.getNumerator();
 	int result = num / ratio.getDenominator();
 	if (num > ratio.getDenominator() && num % ratio.getDenominator()) {
 		++result;
 	}
-	return result;
+	return result - extra;
 }
 
 /**
@@ -177,19 +177,12 @@ inline void mulru(Common::Point &point, const Common::Rational &ratioX, const Co
  * Multiplies a point by two rational numbers for X and Y,
  * rounding up to the nearest whole number. Modifies the
  * rect directly.
- *
- * @note In SCI engine, the bottom-right corner of rects
- * received an additional one pixel during the
- * multiplication in order to round up to include the
- * bottom-right corner. Since ScummVM rects do not include
- * the bottom-right corner, doing this ends up making rects
- * a pixel too wide/tall depending upon the remainder.
  */
-inline void mulru(Common::Rect &rect, const Common::Rational &ratioX, const Common::Rational &ratioY) {
+inline void mulru(Common::Rect &rect, const Common::Rational &ratioX, const Common::Rational &ratioY, const int extra) {
 	rect.left = mulru(rect.left, ratioX);
 	rect.top = mulru(rect.top, ratioY);
-	rect.right = mulru(rect.right, ratioX);
-	rect.bottom = mulru(rect.bottom, ratioY);
+	rect.right = mulru(rect.right - 1, ratioX, extra) + 1;
+	rect.bottom = mulru(rect.bottom - 1, ratioY, extra) + 1;
 }
 
 struct Buffer : public Graphics::Surface {
diff --git a/engines/sci/graphics/plane32.cpp b/engines/sci/graphics/plane32.cpp
index 96d57e3..1a1a789 100644
--- a/engines/sci/graphics/plane32.cpp
+++ b/engines/sci/graphics/plane32.cpp
@@ -136,7 +136,7 @@ void Plane::convertGameRectToPlaneRect() {
 	const Ratio ratioY = Ratio(screenHeight, scriptHeight);
 
 	_planeRect = _gameRect;
-	mulru(_planeRect, ratioX, ratioY);
+	mulru(_planeRect, ratioX, ratioY, 1);
 }
 
 void Plane::printDebugInfo(Console *con) const {
diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 0bbb056..a07dace 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -256,7 +256,7 @@ void ScreenItem::calcRects(const Plane &plane) {
 			if (_useInsetRect) {
 				Ratio celScriptXRatio(_celObj->_scaledWidth, scriptWidth);
 				Ratio celScriptYRatio(_celObj->_scaledHeight, scriptHeight);
-				mulru(_screenItemRect, celScriptXRatio, celScriptYRatio);
+				mulru(_screenItemRect, celScriptXRatio, celScriptYRatio, 0);
 
 				if (_screenItemRect.intersects(celRect)) {
 					_screenItemRect.clip(celRect);
@@ -273,7 +273,7 @@ void ScreenItem::calcRects(const Plane &plane) {
 			}
 
 			if (!newRatioX.isOne() || !newRatioY.isOne()) {
-				mulru(_screenItemRect, newRatioX, newRatioY);
+				mulinc(_screenItemRect, newRatioX, newRatioY);
 				displaceX = (displaceX * newRatioX).toInt();
 				displaceY = (displaceY * newRatioY).toInt();
 			}
@@ -284,7 +284,7 @@ void ScreenItem::calcRects(const Plane &plane) {
 			displaceX = (displaceX * celXRatio).toInt();
 			displaceY = (displaceY * celYRatio).toInt();
 
-			mulru(_screenItemRect, celXRatio, celYRatio);
+			mulinc(_screenItemRect, celXRatio, celYRatio);
 
 			if (/* TODO: dword_C6288 */ false && _celInfo.type == kCelTypePic) {
 				_scaledPosition.x = _position.x;
@@ -300,18 +300,16 @@ void ScreenItem::calcRects(const Plane &plane) {
 				Common::Rect temp(_insetRect);
 
 				if (!newRatioX.isOne()) {
-					mulru(temp, newRatioX, Ratio());
+					mulinc(temp, newRatioX, Ratio());
 				}
 
-				mulru(temp, celXRatio, Ratio());
+				mulinc(temp, celXRatio, Ratio());
 
 				CelObjPic *celObjPic = dynamic_cast<CelObjPic *>(_celObj);
-
 				temp.translate(celObjPic->_relativePosition.x * screenWidth / scriptWidth - displaceX, 0);
 
-				// TODO: This is weird, and probably wrong calculation of widths
-				// due to BR-inclusion
-				int deltaX = plane._planeRect.right - plane._planeRect.left + 1 - temp.right - 1 - temp.left;
+				// TODO: This is weird.
+				int deltaX = plane._planeRect.width() - temp.right - 1 - temp.left;
 
 				_scaledPosition.x += deltaX;
 				_screenItemRect.translate(deltaX, 0);
@@ -330,9 +328,9 @@ void ScreenItem::calcRects(const Plane &plane) {
 			}
 
 			if (!newRatioX.isOne() || !newRatioY.isOne()) {
-				mulru(_screenItemRect, newRatioX, newRatioY);
+				mulinc(_screenItemRect, newRatioX, newRatioY);
 				// TODO: This was in the original code, baked into the
-				// multiplication though it is  not immediately clear
+				// multiplication though it is not immediately clear
 				// why this is the only one that reduces the BR corner
 				_screenItemRect.right -= 1;
 				_screenItemRect.bottom -= 1;
@@ -346,16 +344,15 @@ void ScreenItem::calcRects(const Plane &plane) {
 				Common::Rect temp(_insetRect);
 
 				if (!newRatioX.isOne()) {
-					mulru(temp, newRatioX, Ratio());
+					mulinc(temp, newRatioX, Ratio());
 					temp.right -= 1;
 				}
 
 				CelObjPic *celObjPic = dynamic_cast<CelObjPic *>(_celObj);
 				temp.translate(celObjPic->_relativePosition.x - (displaceX * newRatioX).toInt(), celObjPic->_relativePosition.y - (_celObj->_displace.y * newRatioY).toInt());
 
-				// TODO: This is weird, and probably wrong calculation of widths
-				// due to BR-inclusion
-				int deltaX = plane._gameRect.right - plane._gameRect.left + 1 - temp.right - 1 - temp.left;
+				// TODO: This is weird.
+				int deltaX = plane._gameRect.width() - temp.right - 1 - temp.left;
 
 				_scaledPosition.x += deltaX;
 				_screenItemRect.translate(deltaX, 0);
@@ -369,7 +366,7 @@ void ScreenItem::calcRects(const Plane &plane) {
 				Ratio celXRatio(screenWidth, _celObj->_scaledWidth);
 				Ratio celYRatio(screenHeight, _celObj->_scaledHeight);
 				mulru(_scaledPosition, celXRatio, celYRatio);
-				mulru(_screenItemRect, celXRatio, celYRatio);
+				mulru(_screenItemRect, celXRatio, celYRatio, 1);
 			}
 
 			_ratioX = newRatioX * Ratio(screenWidth, _celObj->_scaledWidth);


Commit: 2e75f4b2b898866ffeafca5c69709dd3e358dcb5
    https://github.com/scummvm/scummvm/commit/2e75f4b2b898866ffeafca5c69709dd3e358dcb5
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-03-02T00:02:25-06:00

Commit Message:
SCI32: Add consts and remove redundant conditional

Adding consts to try to isolate rectangles that change during
draw list processing, to be investigated for incorrect mutations
causing lower screen items to be drawn over higher screen items at
their edges.

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



diff --git a/engines/sci/graphics/plane32.cpp b/engines/sci/graphics/plane32.cpp
index 1a1a789..5ae3ed9 100644
--- a/engines/sci/graphics/plane32.cpp
+++ b/engines/sci/graphics/plane32.cpp
@@ -388,14 +388,14 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 		bool v81 = false;
 
 		for (RectList::size_type i = 0; i < eraseList.size(); ++i) {
-			Common::Rect *rect = eraseList[i];
+			const Common::Rect *rect = eraseList[i];
 
 			for (ScreenItemList::size_type j = 0; j < _screenItemList.size(); ++j) {
 				ScreenItem *item = _screenItemList[j];
 
 				if (j < _screenItemList.size() && item != nullptr) {
 					if (rect->intersects(item->_screenRect)) {
-						Common::Rect intersection = rect->findIntersectingRect(item->_screenRect);
+						const Common::Rect intersection = rect->findIntersectingRect(item->_screenRect);
 						if (!item->_deleted) {
 							if (encounteredPic) {
 								if (item->_celInfo.type == kCelTypePic) {
@@ -428,7 +428,7 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 		for (RectList::size_type i = 0; i < eraseList.size(); ++i) {
 			for (ScreenItemList::size_type j = 0; j < _screenItemList.size(); ++j) {
 				ScreenItem *item = _screenItemList[j];
-				if (j < _screenItemList.size() && item != nullptr && !item->_updated && !item->_deleted && !item->_created && eraseList[i]->intersects(item->_screenRect)) {
+				if (item != nullptr && !item->_updated && !item->_deleted && !item->_created && eraseList[i]->intersects(item->_screenRect)) {
 					drawList.add(item, eraseList[i]->findIntersectingRect(item->_screenRect));
 				}
 			}
@@ -511,7 +511,7 @@ void Plane::decrementScreenItemArrayCounts(Plane *visiblePlane, const bool force
 void Plane::filterDownEraseRects(DrawList &drawList, RectList &eraseList, RectList &transparentEraseList) const {
 	if (_type == kPlaneTypeTransparent) {
 		for (RectList::size_type i = 0; i < transparentEraseList.size(); ++i) {
-			Common::Rect *r = transparentEraseList[i];
+			const Common::Rect *r = transparentEraseList[i];
 			for (ScreenItemList::size_type j = 0; j < _screenItemList.size(); ++j) {
 				ScreenItem *item = _screenItemList[j];
 				if (item != nullptr) {
@@ -539,7 +539,7 @@ void Plane::filterDownEraseRects(DrawList &drawList, RectList &eraseList, RectLi
 				}
 
 				Common::Rect ptr[4];
-				Common::Rect *r2 = transparentEraseList[i];
+				const Common::Rect *r2 = transparentEraseList[i];
 				int count = splitRects(*r2, *r, ptr);
 				for (int k = count - 1; k >= 0; --k) {
 					transparentEraseList.add(ptr[k]);
@@ -554,7 +554,7 @@ void Plane::filterDownEraseRects(DrawList &drawList, RectList &eraseList, RectLi
 
 void Plane::filterUpDrawRects(DrawList &transparentDrawList, const DrawList &drawList) const {
 	for (DrawList::size_type i = 0; i < drawList.size(); ++i) {
-		Common::Rect &r = drawList[i]->rect;
+		const Common::Rect &r = drawList[i]->rect;
 
 		for (ScreenItemList::size_type j = 0; j < _screenItemList.size(); ++j) {
 			ScreenItem *item = _screenItemList[j];
@@ -569,7 +569,7 @@ void Plane::filterUpDrawRects(DrawList &transparentDrawList, const DrawList &dra
 
 void Plane::filterUpEraseRects(DrawList &drawList, RectList &eraseList) const {
 	for (RectList::size_type i = 0; i < eraseList.size(); ++i) {
-		Common::Rect &r = *eraseList[i];
+		const Common::Rect &r = *eraseList[i];
 		for (ScreenItemList::size_type j = 0; j < _screenItemList.size(); ++j) {
 			ScreenItem *item = _screenItemList[j];
 
@@ -595,7 +595,7 @@ void Plane::mergeToDrawList(const DrawList::size_type index, const Common::Rect
 		r = *rects[i];
 
 		for (DrawList::size_type j = 0; j < drawList.size(); ++j) {
-			DrawItem *drawitem = drawList[j];
+			const DrawItem *drawitem = drawList[j];
 			if (item->_object == drawitem->screenItem->_object) {
 				if (drawitem->rect.contains(r)) {
 					rects.erase_at(i);
@@ -603,7 +603,7 @@ void Plane::mergeToDrawList(const DrawList::size_type index, const Common::Rect
 				}
 
 				Common::Rect outRects[4];
-				int count = splitRects(r, drawitem->rect, outRects);
+				const int count = splitRects(r, drawitem->rect, outRects);
 				if (count != -1) {
 					for (int k = count - 1; k >= 0; --k) {
 						rects.add(outRects[k]);
@@ -633,14 +633,14 @@ void Plane::mergeToRectList(const Common::Rect &rect, RectList &rectList) const
 		Common::Rect r = *temp[i];
 
 		for (RectList::size_type j = 0; j < rectList.size(); ++j) {
-			Common::Rect *innerRect = rectList[j];
+			const Common::Rect *innerRect = rectList[j];
 			if (innerRect->contains(r)) {
 				temp.erase_at(i);
 				break;
 			}
 
 			Common::Rect out[4];
-			int count = splitRects(r, *innerRect, out);
+			const int count = splitRects(r, *innerRect, out);
 			if (count != -1) {
 				for (int k = count - 1; k >= 0; --k) {
 					temp.add(out[k]);






More information about the Scummvm-git-logs mailing list