[Scummvm-git-logs] scummvm master -> f1d6ad795aa07fa55b21a730b39761f9cbd1fcc0

OMGPizzaGuy noreply at scummvm.org
Fri Jan 27 00:53:21 UTC 2023


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

Summary:
6e3a288d93 ULTIMA8: Add contains point method for sort items
71bf75783d ULTIMA8: Replace sort item screenspace coords with rect
62b4aec959 ULTIMA8: Use box struct to set bounds of sort items
5935499ff1 ULTIMA8: update all sort item tests to set box bounds
f1d6ad795a ULTIMA8: Use shape frame screen rect in sort item occludes and overlaps checks in addition to world box. Fixes #14039


Commit: 6e3a288d93191c217fa678ec6acbe5d1e794edd9
    https://github.com/scummvm/scummvm/commit/6e3a288d93191c217fa678ec6acbe5d1e794edd9
Author: Matthew Jimenez (matthew.jimenez at outlook.com)
Date: 2023-01-26T18:53:01-06:00

Commit Message:
ULTIMA8: Add contains point method for sort items

Changed paths:
    engines/ultima/ultima8/world/sort_item.h
    test/engines/ultima/ultima8/world/sort_item.h


diff --git a/engines/ultima/ultima8/world/sort_item.h b/engines/ultima/ultima8/world/sort_item.h
index 65f351b7636..d0c5c635460 100644
--- a/engines/ultima/ultima8/world/sort_item.h
+++ b/engines/ultima/ultima8/world/sort_item.h
@@ -220,6 +220,9 @@ struct SortItem {
 	// Calculate screenspace box bounds at center point from worldspace bounds
 	inline void calculateBoxBounds(int32 sx, int32 sy);
 
+	// Check if the given point is inside the screenpace bounds.
+	inline bool contains(int32 sx, int32 sy) const;
+
 	// Screenspace check to see if this overlaps si2
 	inline bool overlap(const SortItem &si2) const;
 
@@ -257,6 +260,40 @@ inline void SortItem::calculateBoxBounds(int32 sx, int32 sy) {
 	_syBot = (_x + _y) / 8 - _z - sy;
 }
 
+inline bool SortItem::contains(int32 sx, int32 sy) const {
+	const int point_top_diff[2] = { _sxTop - sx, _syTop - sy };
+	const int point_bot_diff[2] = { _sxBot - sx, _syBot - sy };
+
+	// This function is a bit of a hack. It uses dot products between
+	// points and the lines. Nothing is normalized since that isn't
+	// important
+
+	// 'normal' of top  left line ( 2,-1) of the bounding box
+	const int32 dot_top_left = point_top_diff[0] + point_top_diff[1] * 2;
+
+	// 'normal' of top right line ( 2, 1) of the bounding box
+	const int32 dot_top_right = -point_top_diff[0] + point_top_diff[1] * 2;
+
+	// 'normal' of bot  left line (-2,-1) of the bounding box
+	const int32 dot_bot_left = point_bot_diff[0] - point_bot_diff[1] * 2;
+
+	// 'normal' of bot right line (-2, 1) of the bounding box
+	const int32 dot_bot_right = -point_bot_diff[0] - point_bot_diff[1] * 2;
+
+	const bool right_clear = _sxRight < sx;
+	const bool left_clear = _sxLeft > sx;
+	const bool top_left_clear = dot_top_left > 0;
+	const bool top_right_clear = dot_top_right > 0;
+	const bool bot_left_clear = dot_bot_left > 0;
+	const bool bot_right_clear = dot_bot_right > 0;
+
+	const bool clear = right_clear || left_clear ||
+					   (bot_right_clear || bot_left_clear) ||
+					   (top_right_clear || top_left_clear);
+
+	return !clear;
+}
+
 inline bool SortItem::overlap(const SortItem &si2) const {
 	const int point_top_diff[2] = { _sxTop - si2._sxBot, _syTop - si2._syBot };
 	const int point_bot_diff[2] = { _sxBot - si2._sxTop, _syBot - si2._syTop };
diff --git a/test/engines/ultima/ultima8/world/sort_item.h b/test/engines/ultima/ultima8/world/sort_item.h
index 54bec8e327b..55db6c6bd9b 100644
--- a/test/engines/ultima/ultima8/world/sort_item.h
+++ b/test/engines/ultima/ultima8/world/sort_item.h
@@ -11,7 +11,6 @@
  *  * overlapping in various dimensions
  *  * flat (z == zTop) items with various flags
  *  * special case for crusader inventory items
- *  * items that are flat in x or y (what should these do?)
  */
 class U8SortItemTestSuite : public CxxTest::TestSuite {
 	public:
@@ -297,4 +296,44 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		//TS_ASSERT(!si1.occludes(si2));
 		TS_ASSERT(!si2.occludes(si1));
 	}
+
+	void test_basic_contains() {
+		Ultima::Ultima8::SortItem si1(nullptr);
+
+		si1._xLeft = 0;
+		si1._yFar = 0;
+		si1._z  = 0;
+		si1._y = 128;
+		si1._x  = 128;
+		si1._zTop = 16;
+
+		si1.calculateBoxBounds(0, 0);
+
+		// Inside bounds
+		TS_ASSERT(si1.contains(si1._sxBot, si1._syBot - 1));
+		TS_ASSERT(si1.contains(si1._sxTop, si1._syTop + 1));
+		TS_ASSERT(si1.contains(si1._sxLeft + 1, (si1._syTop + si1._syBot) / 2));
+		TS_ASSERT(si1.contains(si1._sxRight - 1, (si1._syTop + si1._syBot) / 2));
+		TS_ASSERT(si1.contains((si1._sxLeft + si1._sxRight) / 2, (si1._syTop + si1._syBot) / 2));
+
+		// Currently inclusive of all edges
+		TS_ASSERT(si1.contains(si1._sxBot, si1._syBot));
+		TS_ASSERT(si1.contains(si1._sxTop, si1._syTop));
+		TS_ASSERT(si1.contains(si1._sxLeft, (si1._syTop + si1._syBot) / 2));
+		TS_ASSERT(si1.contains(si1._sxRight, (si1._syTop + si1._syBot) / 2));
+
+		// Outside bounds
+		TS_ASSERT(!si1.contains(si1._sxBot, si1._syBot + 1));
+		TS_ASSERT(!si1.contains(si1._sxTop, si1._syTop - 1));
+		TS_ASSERT(!si1.contains(si1._sxLeft - 1, (si1._syTop + si1._syBot) / 2));
+		TS_ASSERT(!si1.contains(si1._sxRight + 1, (si1._syTop + si1._syBot) / 2));
+		TS_ASSERT(!si1.contains(si1._sxLeft, si1._syTop));
+		TS_ASSERT(!si1.contains(si1._sxLeft, si1._syBot));
+		TS_ASSERT(!si1.contains(si1._sxRight, si1._syTop));
+		TS_ASSERT(!si1.contains(si1._sxRight, si1._syBot));
+		TS_ASSERT(!si1.contains(si1._sxBot + 1, si1._syBot));
+		TS_ASSERT(!si1.contains(si1._sxBot - 1, si1._syBot));
+		TS_ASSERT(!si1.contains(si1._sxTop + 1, si1._syTop));
+		TS_ASSERT(!si1.contains(si1._sxTop - 1, si1._syTop));
+	}
 };


Commit: 71bf75783d59d50e03ab92c250ad5eee01f41d4f
    https://github.com/scummvm/scummvm/commit/71bf75783d59d50e03ab92c250ad5eee01f41d4f
Author: Matthew Jimenez (matthew.jimenez at outlook.com)
Date: 2023-01-26T18:53:01-06:00

Commit Message:
ULTIMA8: Replace sort item screenspace coords with rect

Changed paths:
    engines/ultima/ultima8/misc/rect.h
    engines/ultima/ultima8/world/item_sorter.cpp
    engines/ultima/ultima8/world/item_sorter.h
    engines/ultima/ultima8/world/sort_item.h
    test/engines/ultima/ultima8/world/sort_item.h


diff --git a/engines/ultima/ultima8/misc/rect.h b/engines/ultima/ultima8/misc/rect.h
index 8f0062fd9a4..998ccd7cb91 100644
--- a/engines/ultima/ultima8/misc/rect.h
+++ b/engines/ultima/ultima8/misc/rect.h
@@ -72,6 +72,11 @@ struct Rect {
 		return (left <= x) && (x < right) && (top <= y) && (y < bottom);
 	}
 
+	// Check if the given Rect is contained inside this rectangle.
+	bool contains(const Rect &r) const {
+		return (left <= r.left) && (r.right <= right) && (top <= r.top) && (r.bottom <= bottom);
+	}
+
 	// Move the Rect (Relative)
 	void translate(int32 dx, int32 dy) {
 		left += dx;
diff --git a/engines/ultima/ultima8/world/item_sorter.cpp b/engines/ultima/ultima8/world/item_sorter.cpp
index 409188b06e2..6636e9581b5 100644
--- a/engines/ultima/ultima8/world/item_sorter.cpp
+++ b/engines/ultima/ultima8/world/item_sorter.cpp
@@ -140,18 +140,18 @@ void ItemSorter::AddItem(int32 x, int32 y, int32 z, uint32 shapeNum, uint32 fram
 	si->calculateBoxBounds(_camSx, _camSy);
 
 	// Real Screenspace coords
-	si->_sx = si->_sxBot - frame->_xoff;   // Left
-	si->_sy = si->_syBot - frame->_yoff;   // Top
-	si->_sx2 = si->_sx + frame->_width;    // Right
-	si->_sy2 = si->_sy + frame->_height;   // Bottom
+	si->_sr.left = si->_sxBot - frame->_xoff;
+	si->_sr.top = si->_syBot - frame->_yoff;
+	si->_sr.right = si->_sr.left + frame->_width;
+	si->_sr.bottom = si->_sr.top + frame->_height;
 
 	// Do Clipping here
-	int16 clipped = CheckClipped(Rect(si->_sx, si->_sy, si->_sx2, si->_sy2));
-	if (clipped < 0)
+	if (!_clipWindow.intersects(si->_sr)) {
 		// Clipped away entirely - don't add to the list.
 		return;
+	}
 
-	si->_clipped = (clipped != 0);
+	si->_clipped = !_clipWindow.contains(si->_sr);
 
 	// These help out with sorting. We calc them now, so it will be faster
 	si->_fbigsq = (xd == 128 && yd == 128) || (xd == 256 && yd == 256) || (xd == 512 && yd == 512);
@@ -388,11 +388,8 @@ uint16 ItemSorter::Trace(int32 x, int32 y, HitFace *face, bool item_highlight) {
 
 		for (it = _itemsTail; it != nullptr; it = it->_prev) {
 			if (!(it->_flags & (Item::FLG_DISPOSABLE | Item::FLG_FAST_ONLY)) && !it->_fixed) {
-
-				if (!it->_itemNum) continue;
-
-				// Doesn't Overlap
-				if (x < it->_sx || x >= it->_sx2 || y < it->_sy || y >= it->_sy2) continue;
+				if (!it->_itemNum || !it->contains(x, y))
+					continue;
 
 				// Now check the _frame itself
 				const ShapeFrame *_frame = it->_shape->getFrame(it->_frame);
@@ -418,10 +415,8 @@ uint16 ItemSorter::Trace(int32 x, int32 y, HitFace *face, bool item_highlight) {
 
 	if (!selected) {
 		for (it = _items; it != nullptr; it = it->_next) {
-			if (!it->_itemNum) continue;
-
-			// Doesn't Overlap
-			if (x < it->_sx || x >= it->_sx2 || y < it->_sy || y >= it->_sy2) continue;
+			if (!it->_itemNum || !it->contains(x, y))
+				continue;
 
 			// Now check the _frame itself
 			const ShapeFrame *_frame = it->_shape->getFrame(it->_frame);
@@ -489,24 +484,5 @@ void ItemSorter::IncSortLimit(int count) {
 		_sortLimit = 0;
 }
 
-//
-// int16 ItemSorter::CheckClipped(Rect &r)
-//
-// Desc: Check for a clipped rectangle
-// Returns: -1 if off screen,
-//           0 if not clipped,
-//           1 if clipped
-//
-int16 ItemSorter::CheckClipped(const Rect &c) const {
-	Rect r = c;
-	r.clip(_clipWindow);
-
-	// Clipped away to the void
-	if (r.isEmpty())
-		return -1;
-	else if (r == c) return 0;
-	else return 1;
-}
-
 } // End of namespace Ultima8
 } // End of namespace Ultima
diff --git a/engines/ultima/ultima8/world/item_sorter.h b/engines/ultima/ultima8/world/item_sorter.h
index a3bee29330a..8bc1db47a3a 100644
--- a/engines/ultima/ultima8/world/item_sorter.h
+++ b/engines/ultima/ultima8/world/item_sorter.h
@@ -70,10 +70,6 @@ public:
 
 private:
 	bool PaintSortItem(RenderSurface *surf, SortItem *si);
-
-	//! Check Clipped. -1 if off screen, 0 if not clipped, 1 if clipped
-	int16 CheckClipped(const Rect &) const;
-
 };
 
 } // End of namespace Ultima8
diff --git a/engines/ultima/ultima8/world/sort_item.h b/engines/ultima/ultima8/world/sort_item.h
index d0c5c635460..d0a2e5e9b4e 100644
--- a/engines/ultima/ultima8/world/sort_item.h
+++ b/engines/ultima/ultima8/world/sort_item.h
@@ -24,6 +24,7 @@
 
 #include "common/str.h"
 #include "ultima/ultima8/misc/common_types.h"
+#include "ultima/ultima8/misc/rect.h"
 
 namespace Ultima {
 namespace Ultima8 {
@@ -39,8 +40,8 @@ class Shape;
 struct SortItem {
 	SortItem(SortItem *n) : _next(n), _prev(nullptr), _itemNum(0),
 			_shape(nullptr), _order(-1), _depends(), _shapeNum(0),
-			_frame(0), _flags(0), _extFlags(0), _sx(0), _sy(0),
-			_sx2(0), _sy2(0), _x(0), _y(0), _z(0), _xLeft(0),
+			_frame(0), _flags(0), _extFlags(0), _sr(),
+			_x(0), _y(0), _z(0), _xLeft(0),
 			_yFar(0), _zTop(0), _sxLeft(0), _sxRight(0), _sxTop(0),
 			_syTop(0), _sxBot(0), _syBot(0),_fbigsq(false), _flat(false),
 			_occl(false), _solid(false), _draw(false), _roof(false),
@@ -59,9 +60,7 @@ struct SortItem {
 	uint32                  _flags;     // Item flags
 	uint32                  _extFlags;  // Item extended flags
 
-	int                     _sx, _sx2;  // Screenspace X coords
-	int                     _sy, _sy2;  // Screenspace Y coords
-
+	Rect                    _sr; // Screenspace rect for shape frame
 	/*
 	            Bounding Box layout
 
diff --git a/test/engines/ultima/ultima8/world/sort_item.h b/test/engines/ultima/ultima8/world/sort_item.h
index 55db6c6bd9b..7e50f810bf6 100644
--- a/test/engines/ultima/ultima8/world/sort_item.h
+++ b/test/engines/ultima/ultima8/world/sort_item.h
@@ -281,16 +281,16 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		si2.calculateBoxBounds(0, 0);
 
 		// ShapeFrame (240:1)
-		si1._sx = si1._sxBot - 32;
-		si1._sy = si1._syBot - 48;
-		si1._sx2 = si1._sx + 65;
-		si1._sy2 = si1._sy + 48;
+		si1._sr.left = si1._sxBot - 32;
+		si1._sr.top = si1._syBot - 48;
+		si1._sr.right = si1._sr.left + 65;
+		si1._sr.bottom = si1._sr.top + 48;
 
 		// ShapeFrame (301:1)
-		si2._sx = si2._sxBot - 31;
-		si2._sy = si2._syBot - 31;
-		si2._sx2 = si2._sx + 62;
-		si2._sy2 = si2._sy + 32;
+		si2._sr.left = si2._sxBot - 31;
+		si2._sr.top = si2._syBot - 31;
+		si2._sr.right = si2._sr.left + 62;
+		si2._sr.bottom = si2._sr.top + 32;
 
 		// FIXME: This case fails here currently
 		//TS_ASSERT(!si1.occludes(si2));


Commit: 62b4aec9591d8e43445864812ce29db03193f9bc
    https://github.com/scummvm/scummvm/commit/62b4aec9591d8e43445864812ce29db03193f9bc
Author: Matthew Jimenez (matthew.jimenez at outlook.com)
Date: 2023-01-26T18:53:01-06:00

Commit Message:
ULTIMA8: Use box struct to set bounds of sort items

Changed paths:
    engines/ultima/ultima8/world/item_sorter.cpp
    engines/ultima/ultima8/world/sort_item.h
    test/engines/ultima/ultima8/world/sort_item.h


diff --git a/engines/ultima/ultima8/world/item_sorter.cpp b/engines/ultima/ultima8/world/item_sorter.cpp
index 6636e9581b5..32096b9fad2 100644
--- a/engines/ultima/ultima8/world/item_sorter.cpp
+++ b/engines/ultima/ultima8/world/item_sorter.cpp
@@ -130,16 +130,10 @@ void ItemSorter::AddItem(int32 x, int32 y, int32 z, uint32 shapeNum, uint32 fram
 	info->getFootpadWorld(xd, yd, zd, flags & Item::FLG_FLIPPED);
 
 	// Worldspace bounding box
-	si->_x = x;
-	si->_y = y;
-	si->_z = z;
-	si->_xLeft = si->_x - xd;
-	si->_yFar = si->_y - yd;
-	si->_zTop = si->_z + zd;
+	Box box(x, y, z, xd, yd, zd);
+	si->setBoxBounds(box, _camSx, _camSy);
 
-	si->calculateBoxBounds(_camSx, _camSy);
-
-	// Real Screenspace coords
+	// Real Screenspace from shape frame
 	si->_sr.left = si->_sxBot - frame->_xoff;
 	si->_sr.top = si->_syBot - frame->_yoff;
 	si->_sr.right = si->_sr.left + frame->_width;
diff --git a/engines/ultima/ultima8/world/sort_item.h b/engines/ultima/ultima8/world/sort_item.h
index d0a2e5e9b4e..87ad3d82a66 100644
--- a/engines/ultima/ultima8/world/sort_item.h
+++ b/engines/ultima/ultima8/world/sort_item.h
@@ -25,6 +25,7 @@
 #include "common/str.h"
 #include "ultima/ultima8/misc/common_types.h"
 #include "ultima/ultima8/misc/rect.h"
+#include "ultima/ultima8/misc/box.h"
 
 namespace Ultima {
 namespace Ultima8 {
@@ -216,8 +217,8 @@ struct SortItem {
 
 	// Functions
 
-	// Calculate screenspace box bounds at center point from worldspace bounds
-	inline void calculateBoxBounds(int32 sx, int32 sy);
+	// Set worldspace bounds and calculate screenspace at center point
+	inline void setBoxBounds(const Box &box, int32 sx, int32 sy);
 
 	// Check if the given point is inside the screenpace bounds.
 	inline bool contains(int32 sx, int32 sy) const;
@@ -242,7 +243,14 @@ struct SortItem {
 	Common::String dumpInfo() const;
 };
 
-inline void SortItem::calculateBoxBounds(int32 sx, int32 sy) {
+inline void SortItem::setBoxBounds(const Box& box, int32 sx, int32 sy) {
+	_x = box._x;
+	_y = box._y;
+	_z = box._z;
+	_xLeft = _x - box._xd;
+	_yFar = _y - box._yd;
+	_zTop = _z + box._zd;
+
 	// Screenspace bounding box left extent    (LNT x coord)
 	_sxLeft = (_xLeft - _y) / 4 - sx;
 	// Screenspace bounding box right extent   (RFT x coord)
@@ -257,6 +265,12 @@ inline void SortItem::calculateBoxBounds(int32 sx, int32 sy) {
 	_sxBot = (_x - _y) / 4 - sx;
 	// Screenspace bounding box bottom extent  (RNB y coord)
 	_syBot = (_x + _y) / 8 - _z - sy;
+
+	// Screenspace rect - replace with shape frame calculations
+	_sr.left = _sxLeft;
+	_sr.top = _syTop;
+	_sr.right = _sxRight;
+	_sr.bottom = _syBot;
 }
 
 inline bool SortItem::contains(int32 sx, int32 sy) const {
diff --git a/test/engines/ultima/ultima8/world/sort_item.h b/test/engines/ultima/ultima8/world/sort_item.h
index 7e50f810bf6..1f0e85eb11b 100644
--- a/test/engines/ultima/ultima8/world/sort_item.h
+++ b/test/engines/ultima/ultima8/world/sort_item.h
@@ -247,15 +247,10 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._xLeft = si2._xLeft = 0;
-		si1._yFar = si2._yFar = 0;
-		si1._z = si2._z = 0;
-		si1._y = si2._y = 128;
-		si1._x = si2._x = 128;
-		si1._zTop = 16;
-		si2._zTop = 0;
-		si1.calculateBoxBounds(0, 0);
-		si2.calculateBoxBounds(0, 0);
+		Ultima::Ultima8::Box b1(0, 0, 0, 128, 128, 16);
+		Ultima::Ultima8::Box b2(0, 0, 0, 128, 128, 0);
+		si1.setBoxBounds(b1, 0, 0);
+		si2.setBoxBounds(b2, 0, 0);
 
 		TS_ASSERT(si1.occludes(si2));
 		TS_ASSERT(!si2.occludes(si1));
@@ -269,16 +264,10 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._xLeft = si2._xLeft = 0;
-		si1._yFar = si2._yFar = 0;
-		si1._z = si2._z = 0;
-		si1._y = si2._y = 128;
-		si1._x = si2._x = 128;
-		si1._zTop = 16;
-		si2._zTop = 0;
-
-		si1.calculateBoxBounds(0, 0);
-		si2.calculateBoxBounds(0, 0);
+		Ultima::Ultima8::Box b1(0, 0, 0, 128, 128, 16);
+		Ultima::Ultima8::Box b2(0, 0, 0, 128, 128, 0);
+		si1.setBoxBounds(b1, 0, 0);
+		si2.setBoxBounds(b2, 0, 0);
 
 		// ShapeFrame (240:1)
 		si1._sr.left = si1._sxBot - 32;
@@ -300,14 +289,8 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 	void test_basic_contains() {
 		Ultima::Ultima8::SortItem si1(nullptr);
 
-		si1._xLeft = 0;
-		si1._yFar = 0;
-		si1._z  = 0;
-		si1._y = 128;
-		si1._x  = 128;
-		si1._zTop = 16;
-
-		si1.calculateBoxBounds(0, 0);
+		Ultima::Ultima8::Box b1(0, 0, 0, 128, 128, 16);
+		si1.setBoxBounds(b1, 0, 0);
 
 		// Inside bounds
 		TS_ASSERT(si1.contains(si1._sxBot, si1._syBot - 1));


Commit: 5935499ff1ca97cf53c4cc782da101c02ce15413
    https://github.com/scummvm/scummvm/commit/5935499ff1ca97cf53c4cc782da101c02ce15413
Author: Matthew Jimenez (matthew.jimenez at outlook.com)
Date: 2023-01-26T18:53:01-06:00

Commit Message:
ULTIMA8: update all sort item tests to set box bounds

Changed paths:
    test/engines/ultima/ultima8/world/sort_item.h


diff --git a/test/engines/ultima/ultima8/world/sort_item.h b/test/engines/ultima/ultima8/world/sort_item.h
index 1f0e85eb11b..e0b3695dffa 100644
--- a/test/engines/ultima/ultima8/world/sort_item.h
+++ b/test/engines/ultima/ultima8/world/sort_item.h
@@ -22,11 +22,11 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._yFar = 0;
-		si1._y = 10;
-		si2._yFar = 20;
-		si2._y = 30;
-		si1._x = si2._x = 10;
+		Ultima::Ultima8::Box b1(0, 10, 0, 10, 10, 0);
+		Ultima::Ultima8::Box b2(0, 30, 0, 10, 10, 0);
+		si1.setBoxBounds(b1, 0, 0);
+		si2.setBoxBounds(b2, 0, 0);
+
 		TS_ASSERT(si1.below(si2));
 		TS_ASSERT(!si2.below(si1));
 	}
@@ -36,10 +36,11 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._y = si2._y = 10;
-		si1._x = 10;
-		si2._xLeft = 20;
-		si2._x = 30;
+		Ultima::Ultima8::Box b1(10, 0, 0, 10, 10, 0);
+		Ultima::Ultima8::Box b2(30, 0, 0, 10, 10, 0);
+		si1.setBoxBounds(b1, 0, 0);
+		si2.setBoxBounds(b2, 0, 0);
+
 		TS_ASSERT(si1.below(si2));
 		TS_ASSERT(!si2.below(si1));
 	}
@@ -49,10 +50,11 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._x = si2._x = si1._y = si2._y = 10;
-		si1._zTop = 10;
-		si2._z = 20;
-		si2._zTop = 30;
+		Ultima::Ultima8::Box b1(10, 10, 0, 10, 10, 10);
+		Ultima::Ultima8::Box b2(10, 10, 20, 10, 10, 10);
+		si1.setBoxBounds(b1, 0, 0);
+		si2.setBoxBounds(b2, 0, 0);
+
 		TS_ASSERT(si1.below(si2));
 		TS_ASSERT(!si2.below(si1));
 	}
@@ -62,13 +64,14 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si2._sprite = true;
-		TS_ASSERT(si1.below(si2));
-		TS_ASSERT(!si2.below(si1));
+		Ultima::Ultima8::Box b1(0, 0, 10, 10, 10, 10);
+		Ultima::Ultima8::Box b2(0, 0, 0, 10, 10, 10);
+		si1.setBoxBounds(b1, 0, 0);
+		si2.setBoxBounds(b2, 0, 0);
+		TS_ASSERT(!si1.below(si2));
+		TS_ASSERT(si2.below(si1));
 
-		si2._x = si2._xLeft = si1._y = si2._yFar = 10;
-		si2._z = 20;
-		si2._zTop = 30;
+		si2._sprite = true;
 		TS_ASSERT(si1.below(si2));
 		TS_ASSERT(!si2.below(si1));
 	}
@@ -78,7 +81,10 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._x = si2._x = si1._y = si2._y = 10;
+		Ultima::Ultima8::Box b1(0, 0, 0, 10, 10, 0);
+		Ultima::Ultima8::Box b2(0, 0, 0, 10, 10, 0);
+		si1.setBoxBounds(b1, 0, 0);
+		si2.setBoxBounds(b2, 0, 0);
 
 		si1._flat = true;
 		si2._flat = true;
@@ -133,23 +139,16 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 
 		// Land always gets drawn below
 		// MainActor::teleport 6 7642 19776 48
-		si1._x = 128;
-		si1._y = 32;
-		si1._z = 0;
-		si1._xLeft = 0;
-		si1._yFar = 0;
-		si1._zTop = 8;
+		Ultima::Ultima8::Box b1(36, 0, 0, 128, 32, 8);
+		si1.setBoxBounds(b1, 0, 0);
 		si1._occl = true;
 		si1._roof = true;
 		si1._land = true;
 
-		si2._x = 92;
-		si2._y = 64;
-		si2._z = 0;
-		si2._xLeft = 28;
-		si2._yFar = 0;
-		si2._zTop = 40;
+		Ultima::Ultima8::Box b2(0, 32, 0, 120, 64, 40);
+		si2.setBoxBounds(b2, 0, 0);
 		si2._solid = true;
+
 		TS_ASSERT(si1.below(si2));
 		TS_ASSERT(!si2.below(si1));
 	}
@@ -163,20 +162,12 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._x = 32;
-		si1._y = 96;
-		si1._z = 0;
-		si1._xLeft = 0;
-		si1._yFar = 0;
-		si1._zTop = 40;
+		Ultima::Ultima8::Box b1(17407, 16127, 48, 32, 96, 40);
+		si1.setBoxBounds(b1, 0, 0);
 		si1._solid = true;
 
-		si2._x = 32;
-		si2._y = 160;
-		si2._z = 0;
-		si2._xLeft = 0;
-		si2._yFar = 32;
-		si2._zTop = 40;
+		Ultima::Ultima8::Box b2(17407, 16191, 48, 32, 128, 40);
+		si2.setBoxBounds(b2, 0, 0);
 		si2._trans = true;
 		si2._solid = true;
 		si2._land = true;
@@ -193,20 +184,12 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._x = 64;
-		si1._y = 0;
-		si1._z = 16;
-		si1._xLeft = 0;
-		si1._yFar = 0;
-		si1._zTop = 32;
+		Ultima::Ultima8::Box b1(64, 0, 16, 64, 0, 16);
+		si1.setBoxBounds(b1, 0, 0);
 		si1._solid = true;
 
-		si2._x = 64;
-		si2._y = 64;
-		si2._z = 0;
-		si2._xLeft = 0;
-		si2._yFar = 0;
-		si2._zTop = 40;
+		Ultima::Ultima8::Box b2(64, 64, 0, 64, 64, 40);
+		si2.setBoxBounds(b2, 0, 0);
 		si2._solid = true;
 
 		TS_ASSERT(si1.below(si2));
@@ -221,21 +204,13 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		Ultima::Ultima8::SortItem si1(nullptr);
 		Ultima::Ultima8::SortItem si2(nullptr);
 
-		si1._x = 129;
-		si1._y = 32;
-		si1._z = 0;
-		si1._xLeft = 65;
-		si1._yFar = 0;
-		si1._zTop = 24;
+		Ultima::Ultima8::Box b1(129, 32, 0, 64, 64, 24);
+		si1.setBoxBounds(b1, 0, 0);
 		si1._anim = true;
 		si1._solid = true;
 
-		si2._x = 64;
-		si2._y = 69;
-		si2._z = 24;
-		si2._xLeft = 0;
-		si2._yFar = 5;
-		si2._zTop = 64;
+		Ultima::Ultima8::Box b2(64, 69, 24, 64, 64, 40);
+		si2.setBoxBounds(b2, 0, 0);
 		si2._solid = true;
 
 		TS_ASSERT(si1.below(si2));


Commit: f1d6ad795aa07fa55b21a730b39761f9cbd1fcc0
    https://github.com/scummvm/scummvm/commit/f1d6ad795aa07fa55b21a730b39761f9cbd1fcc0
Author: Matthew Jimenez (matthew.jimenez at outlook.com)
Date: 2023-01-26T18:53:01-06:00

Commit Message:
ULTIMA8: Use shape frame screen rect in sort item occludes and overlaps checks in addition to world box. Fixes #14039

Changed paths:
    engines/ultima/ultima8/world/sort_item.h
    test/engines/ultima/ultima8/world/sort_item.h


diff --git a/engines/ultima/ultima8/world/sort_item.h b/engines/ultima/ultima8/world/sort_item.h
index 87ad3d82a66..87cd1c6a3c6 100644
--- a/engines/ultima/ultima8/world/sort_item.h
+++ b/engines/ultima/ultima8/world/sort_item.h
@@ -274,6 +274,9 @@ inline void SortItem::setBoxBounds(const Box& box, int32 sx, int32 sy) {
 }
 
 inline bool SortItem::contains(int32 sx, int32 sy) const {
+	if (!_sr.contains(sx, sy))
+		return false;
+
 	const int point_top_diff[2] = { _sxTop - sx, _syTop - sy };
 	const int point_bot_diff[2] = { _sxBot - sx, _syBot - sy };
 
@@ -308,6 +311,9 @@ inline bool SortItem::contains(int32 sx, int32 sy) const {
 }
 
 inline bool SortItem::overlap(const SortItem &si2) const {
+	if (!_sr.intersects(si2._sr))
+		return false;
+
 	const int point_top_diff[2] = { _sxTop - si2._sxBot, _syTop - si2._syBot };
 	const int point_bot_diff[2] = { _sxBot - si2._sxTop, _syBot - si2._syTop };
 
@@ -342,6 +348,9 @@ inline bool SortItem::overlap(const SortItem &si2) const {
 }
 
 inline bool SortItem::occludes(const SortItem &si2) const {
+	if (!_sr.contains(si2._sr))
+		return false;
+
 	const int point_top_diff[2] = { _sxTop - si2._sxTop, _syTop - si2._syTop };
 	const int point_bot_diff[2] = { _sxBot - si2._sxBot, _syBot - si2._syBot };
 
diff --git a/test/engines/ultima/ultima8/world/sort_item.h b/test/engines/ultima/ultima8/world/sort_item.h
index e0b3695dffa..a2f9a44190e 100644
--- a/test/engines/ultima/ultima8/world/sort_item.h
+++ b/test/engines/ultima/ultima8/world/sort_item.h
@@ -256,8 +256,7 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		si2._sr.right = si2._sr.left + 62;
 		si2._sr.bottom = si2._sr.top + 32;
 
-		// FIXME: This case fails here currently
-		//TS_ASSERT(!si1.occludes(si2));
+		TS_ASSERT(!si1.occludes(si2));
 		TS_ASSERT(!si2.occludes(si1));
 	}
 
@@ -274,11 +273,13 @@ class U8SortItemTestSuite : public CxxTest::TestSuite {
 		TS_ASSERT(si1.contains(si1._sxRight - 1, (si1._syTop + si1._syBot) / 2));
 		TS_ASSERT(si1.contains((si1._sxLeft + si1._sxRight) / 2, (si1._syTop + si1._syBot) / 2));
 
-		// Currently inclusive of all edges
-		TS_ASSERT(si1.contains(si1._sxBot, si1._syBot));
+		// Inclusive of left and top
 		TS_ASSERT(si1.contains(si1._sxTop, si1._syTop));
 		TS_ASSERT(si1.contains(si1._sxLeft, (si1._syTop + si1._syBot) / 2));
-		TS_ASSERT(si1.contains(si1._sxRight, (si1._syTop + si1._syBot) / 2));
+
+		// Exclusive of right and bottom
+		TS_ASSERT(!si1.contains(si1._sxBot, si1._syBot));
+		TS_ASSERT(!si1.contains(si1._sxRight, (si1._syTop + si1._syBot) / 2));
 
 		// Outside bounds
 		TS_ASSERT(!si1.contains(si1._sxBot, si1._syBot + 1));




More information about the Scummvm-git-logs mailing list