[Scummvm-cvs-logs] scummvm master -> a2e9cc4965340add73db19c3d602ef5a910fe336

csnover csnover at users.noreply.github.com
Tue Mar 8 17:29:10 CET 2016


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

Summary:
a2e9cc4965 SCI32: Clean up kIsOnMe and fix rounding bug


Commit: a2e9cc4965340add73db19c3d602ef5a910fe336
    https://github.com/scummvm/scummvm/commit/a2e9cc4965340add73db19c3d602ef5a910fe336
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-03-08T10:29:05-06:00

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

Changed paths:
    engines/sci/engine/kgraphics32.cpp
    engines/sci/graphics/frameout.cpp
    engines/sci/graphics/frameout.h
    engines/sci/graphics/screen_item32.cpp
    engines/sci/graphics/screen_item32.h



diff --git a/engines/sci/engine/kgraphics32.cpp b/engines/sci/engine/kgraphics32.cpp
index 2a9005d..3a59ea5 100644
--- a/engines/sci/engine/kgraphics32.cpp
+++ b/engines/sci/engine/kgraphics32.cpp
@@ -152,12 +152,12 @@ reg_t kObjectIntersect(EngineState *s, int argc, reg_t *argv) {
 
 // Tests if the coordinate is on the passed object
 reg_t kIsOnMe(EngineState *s, int argc, reg_t *argv) {
-	uint16 x = argv[0].toUint16();
-	uint16 y = argv[1].toUint16();
-	reg_t targetObject = argv[2];
-	uint16 checkPixels = argv[3].getOffset();
+	int16 x = argv[0].toSint16();
+	int16 y = argv[1].toSint16();
+	reg_t object = argv[2];
+	bool checkPixel = argv[3].toSint16();
 
-	return make_reg(0, g_sci->_gfxFrameout->kernelIsOnMe(x, y, checkPixels, targetObject));
+	return g_sci->_gfxFrameout->kernelIsOnMe(object, Common::Point(x, y), checkPixel);
 }
 
 reg_t kCreateTextBitmap(EngineState *s, int argc, reg_t *argv) {
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 329b680..655e59d 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -1977,73 +1977,55 @@ void GfxFrameout::kernelFrameOut(const bool shouldShowBits) {
 	}
 }
 
-uint16 GfxFrameout::kernelIsOnMe(int16 x, int16 y, uint16 checkPixels, reg_t screenObject) {
-	reg_t planeObject = readSelector(_segMan, screenObject, SELECTOR(plane));
-	Plane *screenItemPlane = _visiblePlanes.findByObject(planeObject); // Search for plane in visible planes
-	ScreenItem *screenItem = nullptr;
+#pragma mark -
+#pragma mark Mouse cursor
 
-	if (!screenItemPlane) {
-		// Specified plane not found
-		return 0;
+reg_t GfxFrameout::kernelIsOnMe(const reg_t object, const Common::Point &position, bool checkPixel) const {
+	reg_t planeObject = readSelector(_segMan, object, SELECTOR(plane));
+	Plane *plane = _visiblePlanes.findByObject(planeObject);
+	if (plane == nullptr) {
+		return make_reg(0, 0);
 	}
 
-	screenItem = screenItemPlane->_screenItemList.findByObject(screenObject);
-	if (!screenItem) {
-		// Specified screen object not in item list
-		return 0;
+	ScreenItem *screenItem = plane->_screenItemList.findByObject(object);
+	if (screenItem == nullptr) {
+		return make_reg(0, 0);
 	}
 
-	// Original SCI32 seems to have made a copy (?) of the screenitem?
-	// there is also a "or [ebp+arg_56], 1 - not sure what that did
-	return isOnMe(screenItemPlane, screenItem, x, y, checkPixels);
+	return make_reg(0, isOnMe(*screenItem, *plane, position, checkPixel));
 }
 
-uint16 GfxFrameout::isOnMe(Plane *screenItemPlane, ScreenItem *screenItem, int16 x, int16 y, uint16 checkPixels) {
-	// adjust coordinate according to resolution 
-	int32 adjustedX = x * getCurrentBuffer().screenWidth / getCurrentBuffer().scriptWidth;
-	int32 adjustedY = y * getCurrentBuffer().screenHeight / getCurrentBuffer().scriptHeight;
-
-	adjustedX += screenItemPlane->_planeRect.left;
-	adjustedY += screenItemPlane->_planeRect.top;
+bool GfxFrameout::isOnMe(const ScreenItem &screenItem, const Plane &plane, const Common::Point &position, const bool checkPixel) const {
 
-	//warning("kIsOnMe %s %d (%d, %d -> %d, %d) mouse %d, %d", _segMan->getObjectName(screenObject), checkPixels, screenItem->_screenRect.left, screenItem->_screenRect.top, screenItem->_screenRect.right, screenItem->_screenRect.bottom, adjustedX, adjustedY);
+	Common::Point scaledPosition(position);
+	mulru(scaledPosition, Ratio(_currentBuffer.screenWidth, _currentBuffer.scriptWidth), Ratio(_currentBuffer.screenHeight, _currentBuffer.scriptHeight));
+	scaledPosition.x += plane._planeRect.left;
+	scaledPosition.y += plane._planeRect.top;
 
-	if (!screenItem->_screenRect.contains(adjustedX, adjustedY)) {
-		// Specified coordinates are not within screen item
-		return 0;
+	if (!screenItem._screenRect.contains(scaledPosition)) {
+		return false;
 	}
 
-	//warning("HIT!");
-	if (checkPixels) {
-		//warning("Check Pixels");
-		CelObj &screenItemCelObject = screenItem->getCelObj();
+	if (checkPixel) {
+		CelObj &celObj = screenItem.getCelObj();
 
-		Common::Point celAdjustedPoint(adjustedX, adjustedY);
-		bool  celMirrored = screenItem->_mirrorX ^ screenItemCelObject._mirrorX;
+		bool mirrorX = screenItem._mirrorX ^ celObj._mirrorX;
 
-		celAdjustedPoint.x -= screenItem->_scaledPosition.x;
-		celAdjustedPoint.y -= screenItem->_scaledPosition.y;
+		scaledPosition.x -= screenItem._scaledPosition.x;
+		scaledPosition.y -= screenItem._scaledPosition.y;
 
-		Ratio celAdjustXRatio(screenItemCelObject._scaledWidth, getCurrentBuffer().screenWidth);
-		Ratio celAdjustYRatio(screenItemCelObject._scaledHeight, getCurrentBuffer().screenHeight);
-		mulru(celAdjustedPoint, celAdjustXRatio, celAdjustYRatio);
+		mulru(scaledPosition, Ratio(celObj._scaledWidth, _currentBuffer.screenWidth), Ratio(celObj._scaledHeight, _currentBuffer.screenHeight));
 
-		if ((screenItem->_scale.signal) && (screenItem->_scale.x) && (screenItem->_scale.y)) {
-			// Apply scaling
-			celAdjustedPoint.x = celAdjustedPoint.x * 128 / screenItem->_scale.x;
-			celAdjustedPoint.y = celAdjustedPoint.y * 128 / screenItem->_scale.y;
+		if (screenItem._scale.signal != kScaleSignalNone && screenItem._scale.x && screenItem._scale.y) {
+			scaledPosition.x = scaledPosition.x * 128 / screenItem._scale.x;
+			scaledPosition.y = scaledPosition.y * 128 / screenItem._scale.y;
 		}
 
-		byte coordinateColor = screenItemCelObject.readPixel(celAdjustedPoint.x, celAdjustedPoint.y, celMirrored);
-		byte transparentColor = screenItemCelObject._transparentColor;
-
-		if (coordinateColor == transparentColor) {
-			// Coordinate is transparent
-			//warning("TRANSPARENT!");
-			return 0;
-		}
+		uint8 pixel = celObj.readPixel(scaledPosition.x, scaledPosition.y, mirrorX);
+		return pixel != celObj._transparentColor;
 	}
-	return 1;
+
+	return true;
 }
 
 #pragma mark -
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 2931fc0..f864abc 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -502,8 +502,17 @@ public:
 		return 1;
 	};
 
-	uint16 kernelIsOnMe(int16 x, int16 y, uint16 checkPixels, reg_t screenObject);
-	uint16 isOnMe(Plane *screenItemPlane, ScreenItem *screenItem, int16 x, int16 y, uint16 checkPixels);
+#pragma mark -
+#pragma mark Mouse cursor
+private:
+	/**
+	 * Determines whether or not the point given by
+	 * `position` is inside of the given screen item.
+	 */
+	bool isOnMe(const ScreenItem &screenItem, const Plane &plane, const Common::Point &position, const bool checkPixel) const;
+
+public:
+	reg_t kernelIsOnMe(const reg_t object, const Common::Point &position, const bool checkPixel) const;
 
 #pragma mark -
 #pragma mark Debugging
diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp
index f3f397d..138a0e4 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:
diff --git a/engines/sci/graphics/screen_item32.h b/engines/sci/graphics/screen_item32.h
index 0840570..06e1f60 100644
--- a/engines/sci/graphics/screen_item32.h
+++ b/engines/sci/graphics/screen_item32.h
@@ -125,7 +125,7 @@ public:
 	 * item. This member is populated by calling
 	 * `getCelObj`.
 	 */
-	CelObj *_celObj;
+	mutable CelObj *_celObj;
 
 	/**
 	 * If set, the priority for this screen item is fixed
@@ -250,7 +250,7 @@ public:
 	 * screen item. If a cel object does not already exist,
 	 * one will be created and assigned.
 	 */
-	CelObj &getCelObj();
+	CelObj &getCelObj() const;
 
 	void printDebugInfo(Console *con) const;
 






More information about the Scummvm-git-logs mailing list