From 75ccabc325d56876dd34d4a55e2034ee66d33d0b Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Mon, 18 Jan 2016 00:12:47 -0600
Subject: SCI: Implement accurate renderer architecture for SCI32

---
 engines/sci/graphics/screen_item32.cpp | 534 +++++++++++++++++++++++++++++++++
 1 file changed, 534 insertions(+)
 create mode 100644 engines/sci/graphics/screen_item32.cpp

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
new file mode 100644
index 0000000000..300912f110
--- /dev/null
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -0,0 +1,534 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#include "sci/console.h"
+#include "sci/resource.h"
+#include "sci/engine/kernel.h"
+#include "sci/engine/selector.h"
+#include "sci/engine/state.h"
+#include "sci/graphics/celobj32.h"
+#include "sci/graphics/frameout.h"
+#include "sci/graphics/screen_item32.h"
+#include "sci/graphics/view.h"
+
+namespace Sci {
+#pragma mark ScreenItem
+
+uint16 ScreenItem::_nextObjectId = 20000;
+
+ScreenItem::ScreenItem(const reg_t object) :
+_mirrorX(false),
+_pictureId(-1),
+_celObj(nullptr),
+_object(object),
+_deleted(0),
+_updated(0),
+_created(g_sci->_gfxFrameout->getScreenCount()) {
+	SegManager *segMan = g_sci->getEngineState()->_segMan;
+
+	setFromObject(segMan, object, true, true);
+	_plane = readSelector(segMan, object, SELECTOR(plane));
+}
+
+ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo) :
+_position(0, 0),
+_z(0),
+_object(make_reg(0, _nextObjectId++)),
+_celInfo(celInfo),
+_plane(plane),
+_celObj(nullptr),
+_useInsetRect(false),
+_fixPriority(false),
+_mirrorX(false),
+_pictureId(-1),
+_updated(0),
+_deleted(0),
+_created(g_sci->_gfxFrameout->getScreenCount()) {}
+
+ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Rect &rect) :
+_position(rect.left, rect.top),
+_z(0),
+_object(make_reg(0, _nextObjectId++)),
+_celInfo(celInfo),
+_plane(plane),
+_celObj(nullptr),
+_useInsetRect(false),
+_fixPriority(false),
+_mirrorX(false),
+_pictureId(-1),
+_updated(0),
+_deleted(0),
+_created(g_sci->_gfxFrameout->getScreenCount()) {
+	if (celInfo.type == kCelTypeColor) {
+		_insetRect = rect;
+	}
+}
+
+ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Rect &rect, const ScaleInfo &scaleInfo) :
+_position(rect.left, rect.top),
+_z(0),
+_object(make_reg(0, _nextObjectId++)),
+_celInfo(celInfo),
+_plane(plane),
+_celObj(nullptr),
+_useInsetRect(false),
+_fixPriority(false),
+_mirrorX(false),
+_pictureId(-1),
+_updated(0),
+_deleted(0),
+_created(g_sci->_gfxFrameout->getScreenCount()),
+_scale(scaleInfo) {}
+
+ScreenItem::ScreenItem(const ScreenItem &other) :
+_object(other._object),
+_plane(other._plane),
+_celInfo(other._celInfo),
+_celObj(nullptr),
+_screenRect(other._screenRect),
+_mirrorX(other._mirrorX),
+_useInsetRect(other._useInsetRect),
+_scale(other._scale),
+_scaledPosition(other._scaledPosition) {
+	if (other._useInsetRect) {
+		_insetRect = other._insetRect;
+	}
+}
+
+void ScreenItem::operator=(const ScreenItem &other) {
+	_celInfo = other._celInfo;
+	_screenRect = other._screenRect;
+	_mirrorX = other._mirrorX;
+	_useInsetRect = other._useInsetRect;
+	if (other._useInsetRect) {
+		_insetRect = other._insetRect;
+	}
+	_scale = other._scale;
+	_scaledPosition = other._scaledPosition;
+}
+
+void ScreenItem::init() {
+	_nextObjectId = 20000;
+}
+
+void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const bool updateCel, const bool updateBitmap) {
+	_position.x = readSelectorValue(segMan, object, SELECTOR(x));
+	_position.y = readSelectorValue(segMan, object, SELECTOR(y));
+	_scale.x = readSelectorValue(segMan, object, SELECTOR(scaleX));
+	_scale.y = readSelectorValue(segMan, object, SELECTOR(scaleY));
+	_scale.max = readSelectorValue(segMan, object, SELECTOR(maxScale));
+	_scale.signal = (ScaleSignals32)(readSelectorValue(segMan, object, SELECTOR(scaleSignal)) & 3);
+
+	if (updateCel) {
+		_celInfo.resourceId = (GuiResourceId)readSelectorValue(segMan, object, SELECTOR(view));
+		_celInfo.loopNo = readSelectorValue(segMan, object, SELECTOR(loop));
+		_celInfo.celNo = readSelectorValue(segMan, object, SELECTOR(cel));
+
+		if (_celInfo.resourceId <= kPlanePic) {
+			// TODO: Enhance GfxView or ResourceManager to allow
+			// metadata for resources to be retrieved once, from a
+			// single location
+			Resource *view = g_sci->getResMan()->findResource(ResourceId(kResourceTypeView, _celInfo.resourceId), false);
+			if (!view) {
+				error("Failed to load resource %d", _celInfo.resourceId);
+			}
+
+			// NOTE: +2 because the header size field itself is excluded from
+			// the header size in the data
+			const uint16 headerSize = READ_SCI11ENDIAN_UINT16(view->data) + 2;
+			const uint8 loopCount = view->data[2];
+			const uint8 loopSize = view->data[12];
+
+			if (_celInfo.loopNo >= loopCount) {
+				const int maxLoopNo = loopCount - 1;
+				_celInfo.loopNo = maxLoopNo;
+				writeSelectorValue(segMan, object, SELECTOR(loop), maxLoopNo);
+			}
+
+			byte *loopData = view->data + headerSize + (_celInfo.loopNo * loopSize);
+			const int8 seekEntry = loopData[0];
+			if (seekEntry != -1) {
+				loopData = view->data + headerSize + (seekEntry * loopSize);
+			}
+			const uint8 celCount = loopData[2];
+			if (_celInfo.celNo >= celCount) {
+				const int maxCelNo = celCount - 1;
+				_celInfo.celNo = maxCelNo;
+				writeSelectorValue(segMan, object, SELECTOR(cel), maxCelNo);
+			}
+		}
+	}
+
+	if (updateBitmap) {
+		const reg_t bitmap = readSelector(segMan, object, SELECTOR(bitmap));
+		if (!bitmap.isNull()) {
+			_celInfo.bitmap = bitmap;
+			_celInfo.type = kCelTypeMem;
+		} else {
+			_celInfo.bitmap = NULL_REG;
+			_celInfo.type = kCelTypeView;
+		}
+	}
+
+	if (updateCel || updateBitmap) {
+		delete _celObj;
+		_celObj = nullptr;
+	}
+
+	if (readSelectorValue(segMan, object, SELECTOR(fixPriority))) {
+		_fixPriority = true;
+		_priority = readSelectorValue(segMan, object, SELECTOR(priority));
+	} else {
+		_fixPriority = false;
+		writeSelectorValue(segMan, object, SELECTOR(priority), _position.y);
+	}
+
+	_z = readSelectorValue(segMan, object, SELECTOR(z));
+	_position.y -= _z;
+
+	if (readSelectorValue(segMan, object, SELECTOR(useInsetRect))) {
+		_useInsetRect = true;
+		_insetRect.left = readSelectorValue(segMan, object, SELECTOR(inLeft));
+		_insetRect.top = readSelectorValue(segMan, object, SELECTOR(inTop));
+		_insetRect.right = readSelectorValue(segMan, object, SELECTOR(inRight));
+		_insetRect.bottom = readSelectorValue(segMan, object, SELECTOR(inBottom));
+	} else {
+		_useInsetRect = false;
+	}
+
+	// TODO: SCI2.1/SQ6 engine clears this flag any time ScreenItem::Update(MemID)
+	// or ScreenItem::ScreenItem(MemID) are called, but doing this breaks
+	// view cycling because the flag isn't being set again later. There are over
+	// 100 places in the engine code where this flag is set, so it is probably
+	// a matter of figuring out what all of those calls are that re-set it. For
+	// now, since these are the *only* calls that clear this flag, we can just
+	// leave it set all the time.
+	// segMan->getObject(object)->clearInfoSelectorFlag(kInfoFlagViewVisible);
+}
+
+void ScreenItem::calcRects(const Plane &plane) {
+	const int16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
+	const int16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
+	const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
+	const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
+
+	Common::Rect celRect(_celObj->_width, _celObj->_height);
+	if (_useInsetRect) {
+		if (_insetRect.intersects(celRect)) {
+			_insetRect.clip(celRect);
+		} else {
+			_insetRect = Common::Rect();
+		}
+	} else {
+		_insetRect = celRect;
+	}
+
+	Ratio newRatioX;
+	Ratio newRatioY;
+
+	if (_scale.signal & kScaleSignalDoScaling32) {
+		if (_scale.signal & kScaleSignalUseVanishingPoint) {
+			int num = _scale.max * (_position.y - plane._vanishingPoint.y) / (scriptWidth - plane._vanishingPoint.y);
+			newRatioX = Ratio(num, 128);
+			newRatioY = Ratio(num, 128);
+		} else {
+			newRatioX = Ratio(_scale.x, 128);
+			newRatioY = Ratio(_scale.y, 128);
+		}
+	}
+
+	if (newRatioX.getNumerator() && newRatioY.getNumerator()) {
+		_screenItemRect = _insetRect;
+
+		if (_celObj->_scaledWidth != scriptWidth || _celObj->_scaledHeight != scriptHeight) {
+			if (_useInsetRect) {
+				Ratio celScriptXRatio(_celObj->_scaledWidth, scriptWidth);
+				Ratio celScriptYRatio(_celObj->_scaledHeight, scriptHeight);
+				mulru(_screenItemRect, celScriptXRatio, celScriptYRatio);
+
+				if (_screenItemRect.intersects(celRect)) {
+					_screenItemRect.clip(celRect);
+				} else {
+					_screenItemRect = Common::Rect();
+				}
+			}
+
+			int displaceX = _celObj->_displace.x;
+			int displaceY = _celObj->_displace.y;
+
+			if (_mirrorX != _celObj->_mirrorX && _celInfo.type != kCelTypePic) {
+				displaceX = _celObj->_width - _celObj->_displace.x - 1;
+			}
+
+			if (!newRatioX.isOne() || !newRatioY.isOne()) {
+				mulru(_screenItemRect, newRatioX, newRatioY);
+				displaceX = (displaceX * newRatioX).toInt();
+				displaceY = (displaceY * newRatioY).toInt();
+			}
+
+			Ratio celXRatio(screenWidth, _celObj->_scaledWidth);
+			Ratio celYRatio(screenHeight, _celObj->_scaledHeight);
+
+			displaceX = (displaceX * celXRatio).toInt();
+			displaceY = (displaceY * celYRatio).toInt();
+
+			mulru(_screenItemRect, celXRatio, celYRatio);
+
+			if (/* TODO: dword_C6288 */ false && _celInfo.type == kCelTypePic) {
+				_scaledPosition.x = _position.x;
+				_scaledPosition.y = _position.y;
+			} else {
+				_scaledPosition.x = (_position.x * screenWidth / scriptWidth) - displaceX;
+				_scaledPosition.y = (_position.y * screenHeight / scriptHeight) - displaceY;
+			}
+
+			_screenItemRect.translate(_scaledPosition.x, _scaledPosition.y);
+
+			if (_mirrorX != _celObj->_mirrorX && _celInfo.type == kCelTypePic) {
+				Common::Rect temp(_insetRect);
+
+				if (!newRatioX.isOne()) {
+					mulru(temp, newRatioX, Ratio());
+				}
+
+				mulru(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;
+
+				_scaledPosition.x += deltaX;
+				_screenItemRect.translate(deltaX, 0);
+			}
+
+			_scaledPosition.x += plane._planeRect.left;
+			_scaledPosition.y += plane._planeRect.top;
+			_screenItemRect.translate(plane._planeRect.left, plane._planeRect.top);
+
+			_ratioX = newRatioX * Ratio(screenWidth, _celObj->_scaledWidth);
+			_ratioY = newRatioY * Ratio(screenHeight, _celObj->_scaledHeight);
+		} else {
+			int displaceX = _celObj->_displace.x;
+			if (_mirrorX != _celObj->_mirrorX && _celInfo.type != kCelTypePic) {
+				displaceX = _celObj->_width - _celObj->_displace.x - 1;
+			}
+
+			if (!newRatioX.isOne() || !newRatioY.isOne()) {
+				mulru(_screenItemRect, newRatioX, newRatioY);
+				// 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
+				_screenItemRect.right -= 1;
+				_screenItemRect.bottom -= 1;
+			}
+
+			_scaledPosition.x = _position.x - (displaceX * newRatioX).toInt();
+			_scaledPosition.y = _position.y - (_celObj->_displace.y * newRatioY).toInt();
+			_screenItemRect.translate(_scaledPosition.x, _scaledPosition.y);
+
+			if (_mirrorX != _celObj->_mirrorX && _celInfo.type == kCelTypePic) {
+				Common::Rect temp(_insetRect);
+
+				if (!newRatioX.isOne()) {
+					mulru(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;
+
+				_scaledPosition.x += deltaX;
+				_screenItemRect.translate(deltaX, 0);
+			}
+
+			_scaledPosition.x += plane._gameRect.left;
+			_scaledPosition.y += plane._gameRect.top;
+			_screenItemRect.translate(plane._gameRect.left, plane._gameRect.top);
+
+			if (screenWidth != _celObj->_scaledWidth || _celObj->_scaledHeight != screenHeight) {
+				Ratio celXRatio(screenWidth, _celObj->_scaledWidth);
+				Ratio celYRatio(screenHeight, _celObj->_scaledHeight);
+				mulru(_scaledPosition, celXRatio, celYRatio);
+				mulru(_screenItemRect, celXRatio, celYRatio, 1);
+			}
+
+			_ratioX = newRatioX * Ratio(screenWidth, _celObj->_scaledWidth);
+			_ratioY = newRatioY * Ratio(screenHeight, _celObj->_scaledHeight);
+		}
+
+		_screenRect = _screenItemRect;
+
+		if (_screenRect.intersects(plane._screenRect)) {
+			_screenRect.clip(plane._screenRect);
+		} else {
+			_screenRect.right = 0;
+			_screenRect.bottom = 0;
+			_screenRect.left = 0;
+			_screenRect.top = 0;
+		}
+
+		if (!_fixPriority) {
+			_priority = _z + _position.y;
+		}
+	} else {
+		_screenRect.left = 0;
+		_screenRect.top = 0;
+		_screenRect.right = 0;
+		_screenRect.bottom = 0;
+	}
+}
+
+CelObj &ScreenItem::getCelObj() {
+	if (_celObj == nullptr) {
+		switch (_celInfo.type) {
+			case kCelTypeView:
+				_celObj = new CelObjView(_celInfo.resourceId, _celInfo.loopNo, _celInfo.celNo);
+			break;
+			case kCelTypePic:
+				error("Internal error, pic screen item with no cel.");
+			break;
+			case kCelTypeMem:
+				_celObj = new CelObjMem(_celInfo.bitmap);
+			break;
+			case kCelTypeColor:
+				_celObj = new CelObjColor(_celInfo.color, _insetRect.width(), _insetRect.height());
+			break;
+		}
+	}
+
+	return *_celObj;
+}
+
+void ScreenItem::printDebugInfo(Console *con) const {
+	con->debugPrintf("prio %d, x %d, y %d, z: %d, scaledX: %d, scaledY: %d flags: %d\n",
+		_priority,
+		_position.x,
+		_position.y,
+		_z,
+		_scaledPosition.x,
+		_scaledPosition.y,
+		_created | (_updated << 1) | (_deleted << 2)
+	);
+	con->debugPrintf("    screen rect (%d, %d, %d, %d)\n", PRINT_RECT(_screenRect));
+	if (_useInsetRect) {
+		con->debugPrintf("    inset rect: (%d, %d, %d, %d)\n", PRINT_RECT(_insetRect));
+	}
+
+	Common::String celType;
+	switch (_celInfo.type) {
+		case kCelTypePic:
+			celType = "pic";
+			break;
+		case kCelTypeView:
+			celType = "view";
+			break;
+		case kCelTypeColor:
+			celType = "color";
+			break;
+		case kCelTypeMem:
+			celType = "mem";
+			break;
+	}
+
+	con->debugPrintf("    type: %s, res %d, loop %d, cel %d, bitmap %04x:%04x, color: %d\n",
+		celType.c_str(),
+		_celInfo.resourceId,
+		_celInfo.loopNo,
+		_celInfo.celNo,
+		PRINT_REG(_celInfo.bitmap),
+		_celInfo.color
+	);
+	if (_celObj != nullptr) {
+		con->debugPrintf("    width %d, height %d, scaledWidth %d, scaledHeight %d\n",
+			_celObj->_width,
+			_celObj->_height,
+			_celObj->_scaledWidth,
+			_celObj->_scaledHeight
+		);
+	}
+}
+
+void ScreenItem::update(const reg_t object) {
+	SegManager *segMan = g_sci->getEngineState()->_segMan;
+
+	const GuiResourceId view = readSelectorValue(segMan, object, SELECTOR(view));
+	const int16 loopNo = readSelectorValue(segMan, object, SELECTOR(loop));
+	const int16 celNo = readSelectorValue(segMan, object, SELECTOR(cel));
+
+	const bool updateCel = (
+		_celInfo.resourceId != view ||
+		_celInfo.loopNo != loopNo ||
+		_celInfo.celNo != celNo
+	);
+
+	const bool updateBitmap = !readSelector(segMan, object, SELECTOR(bitmap)).isNull();
+
+	setFromObject(segMan, object, updateCel, updateBitmap);
+
+	if (!_created) {
+		_updated = g_sci->_gfxFrameout->getScreenCount();
+	}
+
+	_deleted = 0;
+}
+
+#pragma mark -
+#pragma mark ScreenItemList
+ScreenItem *ScreenItemList::findByObject(const reg_t object) const {
+	const_iterator screenItemIt = Common::find_if(begin(), end(), FindByObject<ScreenItem *>(object));
+
+	if (screenItemIt == end()) {
+		return nullptr;
+	}
+
+	return *screenItemIt;
+}
+void ScreenItemList::sort() {
+	// TODO: SCI engine used _unsorted as an array of indexes into the
+	// list itself and then performed the same swap operations on the
+	// _unsorted array as the _storage array during sorting, but the
+	// only reason to do this would be if some of the pointers in the
+	// list were replaced so the pointer values themselves couldn’t
+	// simply be recorded and then restored later. It is not yet
+	// verified whether this simplification of the sort/unsort is
+	// safe.
+	for (size_type i = 0; i < size(); ++i) {
+		_unsorted[i] = (*this)[i];
+	}
+
+	Common::sort(begin(), end(), sortHelper);
+}
+void ScreenItemList::unsort() {
+	for (size_type i = 0; i < size(); ++i) {
+		(*this)[i] = _unsorted[i];
+	}
+}
+
+}
-- 
cgit v1.2.3


From 03e3f2c68cef429983787ddd38e74718db1ee8d1 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Thu, 18 Feb 2016 00:07:28 -0600
Subject: SCI: Fix some rect off-by-ones

---
 engines/sci/graphics/screen_item32.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 300912f110..ed9adcfc2a 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -209,8 +209,8 @@ void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const boo
 		_useInsetRect = true;
 		_insetRect.left = readSelectorValue(segMan, object, SELECTOR(inLeft));
 		_insetRect.top = readSelectorValue(segMan, object, SELECTOR(inTop));
-		_insetRect.right = readSelectorValue(segMan, object, SELECTOR(inRight));
-		_insetRect.bottom = readSelectorValue(segMan, object, SELECTOR(inBottom));
+		_insetRect.right = readSelectorValue(segMan, object, SELECTOR(inRight)) + 1;
+		_insetRect.bottom = readSelectorValue(segMan, object, SELECTOR(inBottom)) + 1;
 	} else {
 		_useInsetRect = false;
 	}
@@ -376,7 +376,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, 1);
+				mulru(_screenItemRect, celXRatio, celYRatio);
 			}
 
 			_ratioX = newRatioX * Ratio(screenWidth, _celObj->_scaledWidth);
-- 
cgit v1.2.3


From 1c4778d57196f0d0df9dfdb66fd890a23d6c69b4 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Thu, 18 Feb 2016 21:09:15 -0600
Subject: SCI: Minor cleanup

1. Reorder member initialisations to match class member order
2. Use #pragma mark instead of comments for annotating sections
3. Remove useless >=0 checks on unsigned types
---
 engines/sci/graphics/screen_item32.cpp | 56 +++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 28 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index ed9adcfc2a..6b169be6ac 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -36,13 +36,13 @@ namespace Sci {
 uint16 ScreenItem::_nextObjectId = 20000;
 
 ScreenItem::ScreenItem(const reg_t object) :
-_mirrorX(false),
-_pictureId(-1),
 _celObj(nullptr),
 _object(object),
-_deleted(0),
+_pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
-_created(g_sci->_gfxFrameout->getScreenCount()) {
+_deleted(0),
+_mirrorX(false) {
 	SegManager *segMan = g_sci->getEngineState()->_segMan;
 
 	setFromObject(segMan, object, true, true);
@@ -50,65 +50,65 @@ _created(g_sci->_gfxFrameout->getScreenCount()) {
 }
 
 ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo) :
-_position(0, 0),
+_plane(plane),
+_useInsetRect(false),
 _z(0),
-_object(make_reg(0, _nextObjectId++)),
 _celInfo(celInfo),
-_plane(plane),
 _celObj(nullptr),
-_useInsetRect(false),
 _fixPriority(false),
-_mirrorX(false),
+_position(0, 0),
+_object(make_reg(0, _nextObjectId++)),
 _pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
 _deleted(0),
-_created(g_sci->_gfxFrameout->getScreenCount()) {}
+_mirrorX(false) {}
 
 ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Rect &rect) :
-_position(rect.left, rect.top),
+_plane(plane),
+_useInsetRect(false),
 _z(0),
-_object(make_reg(0, _nextObjectId++)),
 _celInfo(celInfo),
-_plane(plane),
 _celObj(nullptr),
-_useInsetRect(false),
 _fixPriority(false),
-_mirrorX(false),
+_position(rect.left, rect.top),
+_object(make_reg(0, _nextObjectId++)),
 _pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
 _deleted(0),
-_created(g_sci->_gfxFrameout->getScreenCount()) {
+_mirrorX(false) {
 	if (celInfo.type == kCelTypeColor) {
 		_insetRect = rect;
 	}
 }
 
 ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Rect &rect, const ScaleInfo &scaleInfo) :
-_position(rect.left, rect.top),
+_plane(plane),
+_scale(scaleInfo),
+_useInsetRect(false),
 _z(0),
-_object(make_reg(0, _nextObjectId++)),
 _celInfo(celInfo),
-_plane(plane),
 _celObj(nullptr),
-_useInsetRect(false),
 _fixPriority(false),
-_mirrorX(false),
+_position(rect.left, rect.top),
+_object(make_reg(0, _nextObjectId++)),
 _pictureId(-1),
+_created(g_sci->_gfxFrameout->getScreenCount()),
 _updated(0),
 _deleted(0),
-_created(g_sci->_gfxFrameout->getScreenCount()),
-_scale(scaleInfo) {}
+_mirrorX(false) {}
 
 ScreenItem::ScreenItem(const ScreenItem &other) :
-_object(other._object),
 _plane(other._plane),
+_scale(other._scale),
+_useInsetRect(other._useInsetRect),
 _celInfo(other._celInfo),
 _celObj(nullptr),
-_screenRect(other._screenRect),
+_object(other._object),
 _mirrorX(other._mirrorX),
-_useInsetRect(other._useInsetRect),
-_scale(other._scale),
-_scaledPosition(other._scaledPosition) {
+_scaledPosition(other._scaledPosition),
+_screenRect(other._screenRect) {
 	if (other._useInsetRect) {
 		_insetRect = other._insetRect;
 	}
-- 
cgit v1.2.3


From b3b6de4fd0dedfbf913a433d1a93cf186a678e70 Mon Sep 17 00:00:00 2001
From: Filippos Karapetis
Date: Sat, 20 Feb 2016 13:56:13 +0200
Subject: SCI: Handle the "visible" object selector. Fixes the inventory in GK1

---
 engines/sci/graphics/screen_item32.cpp | 9 +++++++++
 1 file changed, 9 insertions(+)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 6b169be6ac..98d65406a6 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -202,6 +202,15 @@ void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const boo
 		writeSelectorValue(segMan, object, SELECTOR(priority), _position.y);
 	}
 
+	// Check if the entry should be hidden
+	// TODO: Verify this against disassembly!
+	if (lookupSelector(segMan, object, SELECTOR(visible), NULL, NULL) != kSelectorNone) {
+		if (readSelectorValue(segMan, object, SELECTOR(visible)) == 0) {
+			_fixPriority = true;
+			_priority = -1;
+		}
+	}
+
 	_z = readSelectorValue(segMan, object, SELECTOR(z));
 	_position.y -= _z;
 
-- 
cgit v1.2.3


From 988d5a20511adc5aa5551ea599e0ee56d2699dd5 Mon Sep 17 00:00:00 2001
From: Filippos Karapetis
Date: Sat, 20 Feb 2016 14:26:20 +0200
Subject: SCI: Add a better explanation of the visibility code used in GK1

---
 engines/sci/graphics/screen_item32.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 98d65406a6..58e0a1348a 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -202,8 +202,11 @@ void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const boo
 		writeSelectorValue(segMan, object, SELECTOR(priority), _position.y);
 	}
 
-	// Check if the entry should be hidden
-	// TODO: Verify this against disassembly!
+	// Check if the entry should be hidden (used in GK1, for the inventory items)
+	// TODO/FIXME: Verify this against disassembly! Check if GK1 checks this selector
+	// like we do here. The following bit of code is guesswork, but for now it fixes
+	// the inventory in GK1, and it's really only used in that game (the "visible"
+	// selector isn't present in any other SCI32 game)
 	if (lookupSelector(segMan, object, SELECTOR(visible), NULL, NULL) != kSelectorNone) {
 		if (readSelectorValue(segMan, object, SELECTOR(visible)) == 0) {
 			_fixPriority = true;
-- 
cgit v1.2.3


From 62546273ca7deaf03cb1cbad6d99cf34e81ed949 Mon Sep 17 00:00:00 2001
From: Filippos Karapetis
Date: Sat, 20 Feb 2016 15:30:11 +0200
Subject: SCI: Document and disable the unverified code used in GK1

---
 engines/sci/graphics/screen_item32.cpp | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 58e0a1348a..25a403a70f 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -202,17 +202,22 @@ void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const boo
 		writeSelectorValue(segMan, object, SELECTOR(priority), _position.y);
 	}
 
-	// Check if the entry should be hidden (used in GK1, for the inventory items)
-	// TODO/FIXME: Verify this against disassembly! Check if GK1 checks this selector
-	// like we do here. The following bit of code is guesswork, but for now it fixes
-	// the inventory in GK1, and it's really only used in that game (the "visible"
-	// selector isn't present in any other SCI32 game)
+	// TODO: At this point (needs checking), GK1 seems to check for the "visible"
+	// selector of a plane object. If the object has such a selector, and it's set
+	// to 0, then the object should be hidden.
+	//
+	// This is needed for the inventory in GK1, and seemed to be used only for that
+	// game - the "visible" selector isn't present in any other SCI32 game.
+	// Possible disabled and unverified code that checks for this follows. This fixes
+	// the inventory in GK1. Verify against disassembly!
+#if 0
 	if (lookupSelector(segMan, object, SELECTOR(visible), NULL, NULL) != kSelectorNone) {
 		if (readSelectorValue(segMan, object, SELECTOR(visible)) == 0) {
 			_fixPriority = true;
 			_priority = -1;
 		}
 	}
+#endif
 
 	_z = readSelectorValue(segMan, object, SELECTOR(z));
 	_position.y -= _z;
-- 
cgit v1.2.3


From 0941dcdb6751b870392155d7a8524aa1c1567f46 Mon Sep 17 00:00:00 2001
From: Martin Kiewitz
Date: Sun, 21 Feb 2016 13:58:51 +0100
Subject: SCI32: Add a bit more debug info to errors + screenitems

---
 engines/sci/graphics/screen_item32.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 25a403a70f..fe995a429a 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -444,7 +444,9 @@ CelObj &ScreenItem::getCelObj() {
 }
 
 void ScreenItem::printDebugInfo(Console *con) const {
-	con->debugPrintf("prio %d, x %d, y %d, z: %d, scaledX: %d, scaledY: %d flags: %d\n",
+	con->debugPrintf("%x:%x (%s), prio %d, x %d, y %d, z: %d, scaledX: %d, scaledY: %d flags: %d\n",
+		_object.getSegment(), _object.getOffset(),
+		g_sci->getEngineState()->_segMan->getObjectName(_object),
 		_priority,
 		_position.x,
 		_position.y,
-- 
cgit v1.2.3


From 93e99ba30d97e141bedd1b28aa7aac97faa23c0f Mon Sep 17 00:00:00 2001
From: Filippos Karapetis
Date: Sat, 27 Feb 2016 14:40:13 +0200
Subject: SCI: Remove a TODO in GK1 that has been resolved properly now

---
 engines/sci/graphics/screen_item32.cpp | 17 -----------------
 1 file changed, 17 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index fe995a429a..80d03086ae 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -202,23 +202,6 @@ void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const boo
 		writeSelectorValue(segMan, object, SELECTOR(priority), _position.y);
 	}
 
-	// TODO: At this point (needs checking), GK1 seems to check for the "visible"
-	// selector of a plane object. If the object has such a selector, and it's set
-	// to 0, then the object should be hidden.
-	//
-	// This is needed for the inventory in GK1, and seemed to be used only for that
-	// game - the "visible" selector isn't present in any other SCI32 game.
-	// Possible disabled and unverified code that checks for this follows. This fixes
-	// the inventory in GK1. Verify against disassembly!
-#if 0
-	if (lookupSelector(segMan, object, SELECTOR(visible), NULL, NULL) != kSelectorNone) {
-		if (readSelectorValue(segMan, object, SELECTOR(visible)) == 0) {
-			_fixPriority = true;
-			_priority = -1;
-		}
-	}
-#endif
-
 	_z = readSelectorValue(segMan, object, SELECTOR(z));
 	_position.y -= _z;
 
-- 
cgit v1.2.3


From 154f592f513b860d8305d3c866ab79899e8f3184 Mon Sep 17 00:00:00 2001
From: Willem Jan Palenstijn
Date: Thu, 25 Feb 2016 00:40:09 +0100
Subject: SCI32: Clear InfoFlagViewVisible after updating ScreenItem

---
 engines/sci/graphics/screen_item32.cpp | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 80d03086ae..0bbb056071 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -215,14 +215,7 @@ void ScreenItem::setFromObject(SegManager *segMan, const reg_t object, const boo
 		_useInsetRect = false;
 	}
 
-	// TODO: SCI2.1/SQ6 engine clears this flag any time ScreenItem::Update(MemID)
-	// or ScreenItem::ScreenItem(MemID) are called, but doing this breaks
-	// view cycling because the flag isn't being set again later. There are over
-	// 100 places in the engine code where this flag is set, so it is probably
-	// a matter of figuring out what all of those calls are that re-set it. For
-	// now, since these are the *only* calls that clear this flag, we can just
-	// leave it set all the time.
-	// segMan->getObject(object)->clearInfoSelectorFlag(kInfoFlagViewVisible);
+	segMan->getObject(object)->clearInfoSelectorFlag(kInfoFlagViewVisible);
 }
 
 void ScreenItem::calcRects(const Plane &plane) {
-- 
cgit v1.2.3


From 97d5e9218519049bf7d4403682129173ef5d44e3 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Wed, 2 Mar 2016 00:00:28 -0600
Subject: SCI32: Review rect rounding in Plane and ScreenItem

These changes should cause ScummVM to be more accurate in edge
case rounding.
---
 engines/sci/graphics/screen_item32.cpp | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 0bbb056071..a07dacee83 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);
-- 
cgit v1.2.3


From 1337cd3dec86d64476bc4248e8368848d70b56e5 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Sat, 5 Mar 2016 23:56:38 -0600
Subject: SCI32: Implement kEditText

---
 engines/sci/graphics/screen_item32.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index a07dacee83..c8c9cbea0f 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -83,7 +83,7 @@ _mirrorX(false) {
 	}
 }
 
-ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Rect &rect, const ScaleInfo &scaleInfo) :
+ScreenItem::ScreenItem(const reg_t plane, const CelInfo32 &celInfo, const Common::Point &position, const ScaleInfo &scaleInfo) :
 _plane(plane),
 _scale(scaleInfo),
 _useInsetRect(false),
@@ -91,7 +91,7 @@ _z(0),
 _celInfo(celInfo),
 _celObj(nullptr),
 _fixPriority(false),
-_position(rect.left, rect.top),
+_position(position),
 _object(make_reg(0, _nextObjectId++)),
 _pictureId(-1),
 _created(g_sci->_gfxFrameout->getScreenCount()),
-- 
cgit v1.2.3


From e8d6ad1d0bcfef12bdf468c9a6a80cd14ae2d8f7 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Sat, 5 Mar 2016 23:57:28 -0600
Subject: SCI32: Fix missing digits in plane item list debug output

---
 engines/sci/graphics/screen_item32.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index c8c9cbea0f..ae0b83bcff 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -417,7 +417,7 @@ CelObj &ScreenItem::getCelObj() {
 }
 
 void ScreenItem::printDebugInfo(Console *con) const {
-	con->debugPrintf("%x:%x (%s), prio %d, x %d, y %d, z: %d, scaledX: %d, scaledY: %d flags: %d\n",
+	con->debugPrintf("%04x:%04x (%s), prio %d, x %d, y %d, z: %d, scaledX: %d, scaledY: %d flags: %d\n",
 		_object.getSegment(), _object.getOffset(),
 		g_sci->getEngineState()->_segMan->getObjectName(_object),
 		_priority,
-- 
cgit v1.2.3


From 36800b701764222ec0c527f68504d1d3dc2e1fcc Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Sun, 6 Mar 2016 16:40:23 -0600
Subject: SCI32: Fix memory leaks

---
 engines/sci/graphics/screen_item32.cpp | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index ae0b83bcff..f3f397d632 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -126,6 +126,10 @@ void ScreenItem::operator=(const ScreenItem &other) {
 	_scaledPosition = other._scaledPosition;
 }
 
+ScreenItem::~ScreenItem() {
+	delete _celObj;
+}
+
 void ScreenItem::init() {
 	_nextObjectId = 20000;
 }
-- 
cgit v1.2.3


From a2e9cc4965340add73db19c3d602ef5a910fe336 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Tue, 8 Mar 2016 10:27:15 -0600
Subject: SCI32: Clean up kIsOnMe and fix rounding bug

The implementation was not correctly rounding the scaled position
with mulru, leading to occasionally incorrect hit detection at
the boundaries of boxes.
---
 engines/sci/graphics/screen_item32.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index f3f397d632..138a0e47b0 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -399,7 +399,7 @@ void ScreenItem::calcRects(const Plane &plane) {
 	}
 }
 
-CelObj &ScreenItem::getCelObj() {
+CelObj &ScreenItem::getCelObj() const {
 	if (_celObj == nullptr) {
 		switch (_celInfo.type) {
 			case kCelTypeView:
-- 
cgit v1.2.3


From 693f8dc295dc43e3bccd954a8a243f7aedd5da22 Mon Sep 17 00:00:00 2001
From: Johannes Schickel
Date: Tue, 8 Mar 2016 20:13:05 +0100
Subject: SCI: Add missing namespace comments in graphics/.

---
 engines/sci/graphics/screen_item32.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 138a0e47b0..4a42221875 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -527,4 +527,4 @@ void ScreenItemList::unsort() {
 	}
 }
 
-}
+} // End of namespace Sci
-- 
cgit v1.2.3


From 4a16ebc970bfe1eae948b5d59accc321b092db8c Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Thu, 10 Mar 2016 14:05:27 -0600
Subject: SCI32: Implement kSetNowSeen

---
 engines/sci/graphics/screen_item32.cpp | 103 +++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 4a42221875..9f6dc7a2a2 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -495,6 +495,109 @@ void ScreenItem::update(const reg_t object) {
 	_deleted = 0;
 }
 
+// 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();
+
+	Common::Rect celObjRect(celObj._width, celObj._height);
+	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) {
+			nsRect = _insetRect;
+			nsRect.clip(celObjRect);
+		} else {
+			nsRect = Common::Rect();
+		}
+	} else {
+		nsRect = celObjRect;
+	}
+
+	const uint16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
+	const uint16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
+
+	Ratio scaleX, scaleY;
+	if (_scale.signal & kScaleSignalDoScaling32) {
+		if (_scale.signal & kScaleSignalUseVanishingPoint) {
+			int num = _scale.max * (_position.y - plane._vanishingPoint.y) / (scriptWidth - plane._vanishingPoint.y);
+			scaleX = Ratio(num, 128);
+			scaleY = Ratio(num, 128);
+		} else {
+			scaleX = Ratio(_scale.x, 128);
+			scaleY = Ratio(_scale.y, 128);
+		}
+	}
+
+	if (scaleX.getNumerator() == 0 || scaleY.getNumerator() == 0) {
+		return Common::Rect();
+	}
+
+	int16 displaceX = celObj._displace.x;
+	int16 displaceY = celObj._displace.y;
+
+	if (_mirrorX != celObj._mirrorX && _celInfo.type != kCelTypePic) {
+		displaceX = celObj._width - displaceX - 1;
+	}
+
+	if (celObj._scaledWidth != scriptWidth || celObj._scaledHeight != scriptHeight) {
+		if (_useInsetRect) {
+			Ratio scriptToScaledX(celObj._scaledWidth, scriptWidth);
+			Ratio scriptToScaledY(celObj._scaledHeight, scriptHeight);
+			mulru(nsRect, scriptToScaledX, scriptToScaledY, 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) {
+				nsRect.clip(celObjRect);
+			} else {
+				nsRect = Common::Rect();
+			}
+		}
+
+		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;
+		}
+
+		Ratio scaledToScriptX(scriptWidth, celObj._scaledWidth);
+		Ratio scaledToScriptY(scriptHeight, celObj._scaledHeight);
+
+		displaceX = (displaceX * scaleX * scaledToScriptX).toInt();
+		displaceY = (displaceY * scaleY * scaledToScriptY).toInt();
+
+		mulinc(nsRect, scaledToScriptX, scaledToScriptY);
+		nsRect.translate(_position.x - displaceX, _position.y - displaceY);
+	} else {
+		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;
+		}
+
+		displaceX = (displaceX * scaleX).toInt();
+		displaceY = (displaceY * scaleY).toInt();
+		nsRect.translate(_position.x - displaceX, _position.y - displaceY);
+
+		if (_mirrorX != celObj._mirrorX && _celInfo.type != kCelTypePic) {
+			nsRect.translate(plane._gameRect.width() - nsRect.width(), 0);
+		}
+	}
+
+	return nsRect;
+}
+
 #pragma mark -
 #pragma mark ScreenItemList
 ScreenItem *ScreenItemList::findByObject(const reg_t object) const {
-- 
cgit v1.2.3


From 695e5db9a7a051a1b4a37c4bbc1c89c333bbac22 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Thu, 10 Mar 2016 14:13:57 -0600
Subject: SCI32: Remove side-effect-abusing calls to ScreenItem::getCelObj

---
 engines/sci/graphics/screen_item32.cpp | 50 ++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 24 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 9f6dc7a2a2..41771c802c 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -228,7 +228,9 @@ void ScreenItem::calcRects(const Plane &plane) {
 	const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
 	const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
 
-	Common::Rect celRect(_celObj->_width, _celObj->_height);
+	const CelObj &celObj = getCelObj();
+
+	Common::Rect celRect(celObj._width, celObj._height);
 	if (_useInsetRect) {
 		if (_insetRect.intersects(celRect)) {
 			_insetRect.clip(celRect);
@@ -256,10 +258,10 @@ void ScreenItem::calcRects(const Plane &plane) {
 	if (newRatioX.getNumerator() && newRatioY.getNumerator()) {
 		_screenItemRect = _insetRect;
 
-		if (_celObj->_scaledWidth != scriptWidth || _celObj->_scaledHeight != scriptHeight) {
+		if (celObj._scaledWidth != scriptWidth || celObj._scaledHeight != scriptHeight) {
 			if (_useInsetRect) {
-				Ratio celScriptXRatio(_celObj->_scaledWidth, scriptWidth);
-				Ratio celScriptYRatio(_celObj->_scaledHeight, scriptHeight);
+				Ratio celScriptXRatio(celObj._scaledWidth, scriptWidth);
+				Ratio celScriptYRatio(celObj._scaledHeight, scriptHeight);
 				mulru(_screenItemRect, celScriptXRatio, celScriptYRatio, 0);
 
 				if (_screenItemRect.intersects(celRect)) {
@@ -269,11 +271,11 @@ void ScreenItem::calcRects(const Plane &plane) {
 				}
 			}
 
-			int displaceX = _celObj->_displace.x;
-			int displaceY = _celObj->_displace.y;
+			int displaceX = celObj._displace.x;
+			int displaceY = celObj._displace.y;
 
-			if (_mirrorX != _celObj->_mirrorX && _celInfo.type != kCelTypePic) {
-				displaceX = _celObj->_width - _celObj->_displace.x - 1;
+			if (_mirrorX != celObj._mirrorX && _celInfo.type != kCelTypePic) {
+				displaceX = celObj._width - celObj._displace.x - 1;
 			}
 
 			if (!newRatioX.isOne() || !newRatioY.isOne()) {
@@ -282,8 +284,8 @@ void ScreenItem::calcRects(const Plane &plane) {
 				displaceY = (displaceY * newRatioY).toInt();
 			}
 
-			Ratio celXRatio(screenWidth, _celObj->_scaledWidth);
-			Ratio celYRatio(screenHeight, _celObj->_scaledHeight);
+			Ratio celXRatio(screenWidth, celObj._scaledWidth);
+			Ratio celYRatio(screenHeight, celObj._scaledHeight);
 
 			displaceX = (displaceX * celXRatio).toInt();
 			displaceY = (displaceY * celYRatio).toInt();
@@ -300,7 +302,7 @@ void ScreenItem::calcRects(const Plane &plane) {
 
 			_screenItemRect.translate(_scaledPosition.x, _scaledPosition.y);
 
-			if (_mirrorX != _celObj->_mirrorX && _celInfo.type == kCelTypePic) {
+			if (_mirrorX != celObj._mirrorX && _celInfo.type == kCelTypePic) {
 				Common::Rect temp(_insetRect);
 
 				if (!newRatioX.isOne()) {
@@ -323,12 +325,12 @@ void ScreenItem::calcRects(const Plane &plane) {
 			_scaledPosition.y += plane._planeRect.top;
 			_screenItemRect.translate(plane._planeRect.left, plane._planeRect.top);
 
-			_ratioX = newRatioX * Ratio(screenWidth, _celObj->_scaledWidth);
-			_ratioY = newRatioY * Ratio(screenHeight, _celObj->_scaledHeight);
+			_ratioX = newRatioX * Ratio(screenWidth, celObj._scaledWidth);
+			_ratioY = newRatioY * Ratio(screenHeight, celObj._scaledHeight);
 		} else {
-			int displaceX = _celObj->_displace.x;
-			if (_mirrorX != _celObj->_mirrorX && _celInfo.type != kCelTypePic) {
-				displaceX = _celObj->_width - _celObj->_displace.x - 1;
+			int displaceX = celObj._displace.x;
+			if (_mirrorX != celObj._mirrorX && _celInfo.type != kCelTypePic) {
+				displaceX = celObj._width - celObj._displace.x - 1;
 			}
 
 			if (!newRatioX.isOne() || !newRatioY.isOne()) {
@@ -341,10 +343,10 @@ void ScreenItem::calcRects(const Plane &plane) {
 			}
 
 			_scaledPosition.x = _position.x - (displaceX * newRatioX).toInt();
-			_scaledPosition.y = _position.y - (_celObj->_displace.y * newRatioY).toInt();
+			_scaledPosition.y = _position.y - (celObj._displace.y * newRatioY).toInt();
 			_screenItemRect.translate(_scaledPosition.x, _scaledPosition.y);
 
-			if (_mirrorX != _celObj->_mirrorX && _celInfo.type == kCelTypePic) {
+			if (_mirrorX != celObj._mirrorX && _celInfo.type == kCelTypePic) {
 				Common::Rect temp(_insetRect);
 
 				if (!newRatioX.isOne()) {
@@ -353,7 +355,7 @@ void ScreenItem::calcRects(const Plane &plane) {
 				}
 
 				CelObjPic *celObjPic = dynamic_cast<CelObjPic *>(_celObj);
-				temp.translate(celObjPic->_relativePosition.x - (displaceX * newRatioX).toInt(), celObjPic->_relativePosition.y - (_celObj->_displace.y * newRatioY).toInt());
+				temp.translate(celObjPic->_relativePosition.x - (displaceX * newRatioX).toInt(), celObjPic->_relativePosition.y - (celObj._displace.y * newRatioY).toInt());
 
 				// TODO: This is weird.
 				int deltaX = plane._gameRect.width() - temp.right - 1 - temp.left;
@@ -366,15 +368,15 @@ void ScreenItem::calcRects(const Plane &plane) {
 			_scaledPosition.y += plane._gameRect.top;
 			_screenItemRect.translate(plane._gameRect.left, plane._gameRect.top);
 
-			if (screenWidth != _celObj->_scaledWidth || _celObj->_scaledHeight != screenHeight) {
-				Ratio celXRatio(screenWidth, _celObj->_scaledWidth);
-				Ratio celYRatio(screenHeight, _celObj->_scaledHeight);
+			if (screenWidth != celObj._scaledWidth || celObj._scaledHeight != screenHeight) {
+				Ratio celXRatio(screenWidth, celObj._scaledWidth);
+				Ratio celYRatio(screenHeight, celObj._scaledHeight);
 				mulru(_scaledPosition, celXRatio, celYRatio);
 				mulru(_screenItemRect, celXRatio, celYRatio, 1);
 			}
 
-			_ratioX = newRatioX * Ratio(screenWidth, _celObj->_scaledWidth);
-			_ratioY = newRatioY * Ratio(screenHeight, _celObj->_scaledHeight);
+			_ratioX = newRatioX * Ratio(screenWidth, celObj._scaledWidth);
+			_ratioY = newRatioY * Ratio(screenHeight, celObj._scaledHeight);
 		}
 
 		_screenRect = _screenItemRect;
-- 
cgit v1.2.3


From ee8615d6b1a81a7f0c72d06921397deb28844243 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Sat, 12 Mar 2016 13:23:00 -0600
Subject: SCI32: Clarify the purpose of scaling ratios used in ScreenItem

---
 engines/sci/graphics/screen_item32.cpp | 102 +++++++++++++++++----------------
 1 file changed, 52 insertions(+), 50 deletions(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index 41771c802c..c44d3e96f1 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -241,28 +241,33 @@ void ScreenItem::calcRects(const Plane &plane) {
 		_insetRect = celRect;
 	}
 
-	Ratio newRatioX;
-	Ratio newRatioY;
+	Ratio scaleX, scaleY;
 
 	if (_scale.signal & kScaleSignalDoScaling32) {
 		if (_scale.signal & kScaleSignalUseVanishingPoint) {
 			int num = _scale.max * (_position.y - plane._vanishingPoint.y) / (scriptWidth - plane._vanishingPoint.y);
-			newRatioX = Ratio(num, 128);
-			newRatioY = Ratio(num, 128);
+			scaleX = Ratio(num, 128);
+			scaleY = Ratio(num, 128);
 		} else {
-			newRatioX = Ratio(_scale.x, 128);
-			newRatioY = Ratio(_scale.y, 128);
+			scaleX = Ratio(_scale.x, 128);
+			scaleY = Ratio(_scale.y, 128);
 		}
 	}
 
-	if (newRatioX.getNumerator() && newRatioY.getNumerator()) {
+	if (scaleX.getNumerator() && scaleY.getNumerator()) {
 		_screenItemRect = _insetRect;
 
+		const Ratio celToScreenX(screenWidth, celObj._scaledWidth);
+		const Ratio celToScreenY(screenHeight, celObj._scaledHeight);
+
+		// Cel may use a coordinate system that is not the same size as the
+		// script coordinate system (usually this means high-resolution
+		// pictures with low-resolution scripts)
 		if (celObj._scaledWidth != scriptWidth || celObj._scaledHeight != scriptHeight) {
 			if (_useInsetRect) {
-				Ratio celScriptXRatio(celObj._scaledWidth, scriptWidth);
-				Ratio celScriptYRatio(celObj._scaledHeight, scriptHeight);
-				mulru(_screenItemRect, celScriptXRatio, celScriptYRatio, 0);
+				const Ratio scriptToCelX(celObj._scaledWidth, scriptWidth);
+				const Ratio scriptToCelY(celObj._scaledHeight, scriptHeight);
+				mulru(_screenItemRect, scriptToCelX, scriptToCelY, 0);
 
 				if (_screenItemRect.intersects(celRect)) {
 					_screenItemRect.clip(celRect);
@@ -278,26 +283,25 @@ void ScreenItem::calcRects(const Plane &plane) {
 				displaceX = celObj._width - celObj._displace.x - 1;
 			}
 
-			if (!newRatioX.isOne() || !newRatioY.isOne()) {
-				mulinc(_screenItemRect, newRatioX, newRatioY);
-				displaceX = (displaceX * newRatioX).toInt();
-				displaceY = (displaceY * newRatioY).toInt();
+			if (!scaleX.isOne() || !scaleY.isOne()) {
+				mulinc(_screenItemRect, scaleX, scaleY);
+				displaceX = (displaceX * scaleX).toInt();
+				displaceY = (displaceY * scaleY).toInt();
 			}
 
-			Ratio celXRatio(screenWidth, celObj._scaledWidth);
-			Ratio celYRatio(screenHeight, celObj._scaledHeight);
-
-			displaceX = (displaceX * celXRatio).toInt();
-			displaceY = (displaceY * celYRatio).toInt();
+			mulinc(_screenItemRect, celToScreenX, celToScreenY);
+			displaceX = (displaceX * celToScreenX).toInt();
+			displaceY = (displaceY * celToScreenY).toInt();
 
-			mulinc(_screenItemRect, celXRatio, celYRatio);
+			const Ratio scriptToScreenX = Ratio(screenWidth, scriptWidth);
+			const Ratio scriptToScreenY = Ratio(screenHeight, scriptHeight);
 
 			if (/* TODO: dword_C6288 */ false && _celInfo.type == kCelTypePic) {
 				_scaledPosition.x = _position.x;
 				_scaledPosition.y = _position.y;
 			} else {
-				_scaledPosition.x = (_position.x * screenWidth / scriptWidth) - displaceX;
-				_scaledPosition.y = (_position.y * screenHeight / scriptHeight) - displaceY;
+				_scaledPosition.x = (_position.x * scriptToScreenX).toInt() - displaceX;
+				_scaledPosition.y = (_position.y * scriptToScreenY).toInt() - displaceY;
 			}
 
 			_screenItemRect.translate(_scaledPosition.x, _scaledPosition.y);
@@ -305,14 +309,14 @@ void ScreenItem::calcRects(const Plane &plane) {
 			if (_mirrorX != celObj._mirrorX && _celInfo.type == kCelTypePic) {
 				Common::Rect temp(_insetRect);
 
-				if (!newRatioX.isOne()) {
-					mulinc(temp, newRatioX, Ratio());
+				if (!scaleX.isOne()) {
+					mulinc(temp, scaleX, Ratio());
 				}
 
-				mulinc(temp, celXRatio, Ratio());
+				mulinc(temp, celToScreenX, Ratio());
 
 				CelObjPic *celObjPic = dynamic_cast<CelObjPic *>(_celObj);
-				temp.translate(celObjPic->_relativePosition.x * screenWidth / scriptWidth - displaceX, 0);
+				temp.translate((celObjPic->_relativePosition.x * scriptToScreenX).toInt() - displaceX, 0);
 
 				// TODO: This is weird.
 				int deltaX = plane._planeRect.width() - temp.right - 1 - temp.left;
@@ -325,16 +329,16 @@ void ScreenItem::calcRects(const Plane &plane) {
 			_scaledPosition.y += plane._planeRect.top;
 			_screenItemRect.translate(plane._planeRect.left, plane._planeRect.top);
 
-			_ratioX = newRatioX * Ratio(screenWidth, celObj._scaledWidth);
-			_ratioY = newRatioY * Ratio(screenHeight, celObj._scaledHeight);
+			_ratioX = scaleX * celToScreenX;
+			_ratioY = scaleY * celToScreenY;
 		} else {
 			int displaceX = celObj._displace.x;
 			if (_mirrorX != celObj._mirrorX && _celInfo.type != kCelTypePic) {
 				displaceX = celObj._width - celObj._displace.x - 1;
 			}
 
-			if (!newRatioX.isOne() || !newRatioY.isOne()) {
-				mulinc(_screenItemRect, newRatioX, newRatioY);
+			if (!scaleX.isOne() || !scaleY.isOne()) {
+				mulinc(_screenItemRect, 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
@@ -342,20 +346,20 @@ void ScreenItem::calcRects(const Plane &plane) {
 				_screenItemRect.bottom -= 1;
 			}
 
-			_scaledPosition.x = _position.x - (displaceX * newRatioX).toInt();
-			_scaledPosition.y = _position.y - (celObj._displace.y * newRatioY).toInt();
+			_scaledPosition.x = _position.x - (displaceX * scaleX).toInt();
+			_scaledPosition.y = _position.y - (celObj._displace.y * scaleY).toInt();
 			_screenItemRect.translate(_scaledPosition.x, _scaledPosition.y);
 
 			if (_mirrorX != celObj._mirrorX && _celInfo.type == kCelTypePic) {
 				Common::Rect temp(_insetRect);
 
-				if (!newRatioX.isOne()) {
-					mulinc(temp, newRatioX, Ratio());
+				if (!scaleX.isOne()) {
+					mulinc(temp, scaleX, 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());
+				temp.translate(celObjPic->_relativePosition.x - (displaceX * scaleX).toInt(), celObjPic->_relativePosition.y - (celObj._displace.y * scaleY).toInt());
 
 				// TODO: This is weird.
 				int deltaX = plane._gameRect.width() - temp.right - 1 - temp.left;
@@ -368,15 +372,13 @@ void ScreenItem::calcRects(const Plane &plane) {
 			_scaledPosition.y += plane._gameRect.top;
 			_screenItemRect.translate(plane._gameRect.left, plane._gameRect.top);
 
-			if (screenWidth != celObj._scaledWidth || celObj._scaledHeight != screenHeight) {
-				Ratio celXRatio(screenWidth, celObj._scaledWidth);
-				Ratio celYRatio(screenHeight, celObj._scaledHeight);
-				mulru(_scaledPosition, celXRatio, celYRatio);
-				mulru(_screenItemRect, celXRatio, celYRatio, 1);
+			if (celObj._scaledWidth != screenWidth || celObj._scaledHeight != screenHeight) {
+				mulru(_scaledPosition, celToScreenX, celToScreenY);
+				mulru(_screenItemRect, celToScreenX, celToScreenY, 1);
 			}
 
-			_ratioX = newRatioX * Ratio(screenWidth, celObj._scaledWidth);
-			_ratioY = newRatioY * Ratio(screenHeight, celObj._scaledHeight);
+			_ratioX = scaleX * celToScreenX;
+			_ratioY = scaleY * celToScreenY;
 		}
 
 		_screenRect = _screenItemRect;
@@ -547,9 +549,9 @@ Common::Rect ScreenItem::getNowSeenRect(const Plane &plane) const {
 
 	if (celObj._scaledWidth != scriptWidth || celObj._scaledHeight != scriptHeight) {
 		if (_useInsetRect) {
-			Ratio scriptToScaledX(celObj._scaledWidth, scriptWidth);
-			Ratio scriptToScaledY(celObj._scaledHeight, scriptHeight);
-			mulru(nsRect, scriptToScaledX, scriptToScaledY, 0);
+			Ratio scriptToCelX(celObj._scaledWidth, scriptWidth);
+			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
@@ -570,13 +572,13 @@ Common::Rect ScreenItem::getNowSeenRect(const Plane &plane) const {
 			nsRect.bottom -= 1;
 		}
 
-		Ratio scaledToScriptX(scriptWidth, celObj._scaledWidth);
-		Ratio scaledToScriptY(scriptHeight, celObj._scaledHeight);
+		Ratio celToScriptX(scriptWidth, celObj._scaledWidth);
+		Ratio celToScriptY(scriptHeight, celObj._scaledHeight);
 
-		displaceX = (displaceX * scaleX * scaledToScriptX).toInt();
-		displaceY = (displaceY * scaleY * scaledToScriptY).toInt();
+		displaceX = (displaceX * scaleX * celToScriptX).toInt();
+		displaceY = (displaceY * scaleY * celToScriptY).toInt();
 
-		mulinc(nsRect, scaledToScriptX, scaledToScriptY);
+		mulinc(nsRect, celToScriptX, celToScriptY);
 		nsRect.translate(_position.x - displaceX, _position.y - displaceY);
 	} else {
 		if (!scaleX.isOne() || !scaleY.isOne()) {
-- 
cgit v1.2.3


From e25d928a9cb82d458c7f592ec81576f0d57d8eca Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Fri, 18 Mar 2016 13:08:37 -0500
Subject: SCI32: Fix crashes in kIsOnMe caused by stale CelObjs

---
 engines/sci/graphics/screen_item32.cpp | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

(limited to 'engines/sci/graphics/screen_item32.cpp')

diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index c44d3e96f1..c3fdbb6845 100644
--- a/engines/sci/graphics/screen_item32.cpp
+++ b/engines/sci/graphics/screen_item32.cpp
@@ -115,7 +115,17 @@ _screenRect(other._screenRect) {
 }
 
 void ScreenItem::operator=(const ScreenItem &other) {
-	_celInfo = other._celInfo;
+	// NOTE: The original engine did not check for differences in `_celInfo`
+	// to clear `_celObj` here; instead, it unconditionally set `_celInfo`,
+	// didn't clear `_celObj`, and did hacky stuff in `kIsOnMe` to avoid
+	// testing a mismatched `_celObj`. See `GfxFrameout::kernelIsOnMe` for
+	// more detail.
+	if (_celInfo != other._celInfo) {
+		_celInfo = other._celInfo;
+		delete _celObj;
+		_celObj = nullptr;
+	}
+
 	_screenRect = other._screenRect;
 	_mirrorX = other._mirrorX;
 	_useInsetRect = other._useInsetRect;
-- 
cgit v1.2.3