[Scummvm-cvs-logs] scummvm master -> 4297305dd50d0c4e22bbb8fe4636bbce88dae684

csnover csnover at users.noreply.github.com
Mon Mar 7 23:47:45 CET 2016


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

Summary:
13f2a2c3bd SCI32: Add debugger command to view screen items in the visible plane list
d486921820 SCI32: Add reg_t comparisons for graphics sorting
4297305dd5 SCI32: Clean up renderer code a bit more


Commit: 13f2a2c3bd889efba0009e205e3bfb811790956e
    https://github.com/scummvm/scummvm/commit/13f2a2c3bd889efba0009e205e3bfb811790956e
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-03-07T16:46:25-06:00

Commit Message:
SCI32: Add debugger command to view screen items in the visible plane list

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



diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 6cd072e..a092e06 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -141,6 +141,8 @@ Console::Console(SciEngine *engine) : GUI::Debugger(),
 	registerCmd("vpl",                WRAP_METHOD(Console, cmdVisiblePlaneList));	// alias
 	registerCmd("plane_items",        WRAP_METHOD(Console, cmdPlaneItemList));
 	registerCmd("pi",                 WRAP_METHOD(Console, cmdPlaneItemList));	// alias
+	registerCmd("visible_plane_items", WRAP_METHOD(Console, cmdVisiblePlaneItemList));
+	registerCmd("vpi",                WRAP_METHOD(Console, cmdVisiblePlaneItemList));	// alias
 	registerCmd("saved_bits",         WRAP_METHOD(Console, cmdSavedBits));
 	registerCmd("show_saved_bits",    WRAP_METHOD(Console, cmdShowSavedBits));
 	// Segments
@@ -386,6 +388,7 @@ bool Console::cmdHelp(int argc, const char **argv) {
 	debugPrintf(" plane_list / pl - Shows a list of all the planes in the draw list (SCI2+)\n");
 	debugPrintf(" visible_plane_list / vpl - Shows a list of all the planes in the visible draw list (SCI2+)\n");
 	debugPrintf(" plane_items / pi - Shows a list of all items for a plane (SCI2+)\n");
+	debugPrintf(" visible_plane_items / vpi - Shows a list of all items for a plane in the visible draw list (SCI2+)\n");
 	debugPrintf(" saved_bits - List saved bits on the hunk\n");
 	debugPrintf(" show_saved_bits - Display saved bits\n");
 	debugPrintf("\n");
@@ -1817,6 +1820,34 @@ bool Console::cmdPlaneItemList(int argc, const char **argv) {
 	return true;
 }
 
+bool Console::cmdVisiblePlaneItemList(int argc, const char **argv) {
+	if (argc != 2) {
+		debugPrintf("Shows the list of items for a plane\n");
+		debugPrintf("Usage: %s <plane address>\n", argv[0]);
+		return true;
+	}
+
+	reg_t planeObject = NULL_REG;
+
+	if (parse_reg_t(_engine->_gamestate, argv[1], &planeObject, false)) {
+		debugPrintf("Invalid address passed.\n");
+		debugPrintf("Check the \"addresses\" command on how to use addresses\n");
+		return true;
+	}
+
+#ifdef ENABLE_SCI32
+	if (_engine->_gfxFrameout) {
+		debugPrintf("Visible plane item list:\n");
+		_engine->_gfxFrameout->printVisiblePlaneItemList(this, planeObject);
+	} else {
+		debugPrintf("This SCI version does not have a list of plane items\n");
+	}
+#else
+	debugPrintf("SCI32 isn't included in this compiled executable\n");
+#endif
+	return true;
+}
+
 bool Console::cmdSavedBits(int argc, const char **argv) {
 	SegManager *segman = _engine->_gamestate->_segMan;
 	SegmentId id = segman->findSegmentByType(SEG_TYPE_HUNK);
diff --git a/engines/sci/console.h b/engines/sci/console.h
index b20f1f7..cf85def 100644
--- a/engines/sci/console.h
+++ b/engines/sci/console.h
@@ -98,6 +98,7 @@ private:
 	bool cmdPlaneList(int argc, const char **argv);
 	bool cmdVisiblePlaneList(int argc, const char **argv);
 	bool cmdPlaneItemList(int argc, const char **argv);
+	bool cmdVisiblePlaneItemList(int argc, const char **argv);
 	bool cmdSavedBits(int argc, const char **argv);
 	bool cmdShowSavedBits(int argc, const char **argv);
 	// Segments
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 19eaeef..9cd1346 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -2064,6 +2064,15 @@ void GfxFrameout::printVisiblePlaneList(Console *con) const {
 	printPlaneListInternal(con, _visiblePlanes);
 }
 
+void GfxFrameout::printPlaneItemListInternal(Console *con, const ScreenItemList &screenItemList) const {
+	ScreenItemList::size_type i = 0;
+	for (ScreenItemList::const_iterator sit = screenItemList.begin(); sit != screenItemList.end(); sit++) {
+		ScreenItem *screenItem = *sit;
+		con->debugPrintf("%2d: ", i++);
+		screenItem->printDebugInfo(con);
+	}
+}
+
 void GfxFrameout::printPlaneItemList(Console *con, const reg_t planeObject) const {
 	Plane *p = _planes.findByObject(planeObject);
 
@@ -2072,12 +2081,18 @@ void GfxFrameout::printPlaneItemList(Console *con, const reg_t planeObject) cons
 		return;
 	}
 
-	ScreenItemList::size_type i = 0;
-	for (ScreenItemList::iterator sit = p->_screenItemList.begin(); sit != p->_screenItemList.end(); sit++) {
-		ScreenItem *screenItem = *sit;
-		con->debugPrintf("%2d: ", i++);
-		screenItem->printDebugInfo(con);
+	printPlaneItemListInternal(con, p->_screenItemList);
+}
+
+void GfxFrameout::printVisiblePlaneItemList(Console *con, const reg_t planeObject) const {
+	Plane *p = _visiblePlanes.findByObject(planeObject);
+
+	if (p == nullptr) {
+		con->debugPrintf("Plane does not exist");
+		return;
 	}
+
+	printPlaneItemListInternal(con, p->_screenItemList);
 }
 
 } // End of namespace Sci
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 738011a..2931fc0 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -512,6 +512,8 @@ public:
 	void printVisiblePlaneList(Console *con) const;
 	void printPlaneListInternal(Console *con, const PlaneList &planeList) const;
 	void printPlaneItemList(Console *con, const reg_t planeObject) const;
+	void printVisiblePlaneItemList(Console *con, const reg_t planeObject) const;
+	void printPlaneItemListInternal(Console *con, const ScreenItemList &screenItemList) const;
 };
 
 } // End of namespace Sci


Commit: d4869218200a3dd165c2f1c156f3c1620c813241
    https://github.com/scummvm/scummvm/commit/d4869218200a3dd165c2f1c156f3c1620c813241
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-03-07T16:46:25-06:00

Commit Message:
SCI32: Add reg_t comparisons for graphics sorting

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



diff --git a/engines/sci/engine/vm_types.cpp b/engines/sci/engine/vm_types.cpp
index 53a5a5c..00a67fc 100644
--- a/engines/sci/engine/vm_types.cpp
+++ b/engines/sci/engine/vm_types.cpp
@@ -230,6 +230,10 @@ int reg_t::cmp(const reg_t right, bool treatAsUnsigned) const {
 			return toUint16() - right.toUint16();
 		else
 			return toSint16() - right.toSint16();
+#ifdef ENABLE_SCI32
+	} else if (getSciVersion() >= SCI_VERSION_2) {
+		return sci32Comparison(right);
+#endif
 	} else if (pointerComparisonWithInteger(right)) {
 		return 1;
 	} else if (right.pointerComparisonWithInteger(*this)) {
@@ -238,6 +242,24 @@ int reg_t::cmp(const reg_t right, bool treatAsUnsigned) const {
 		return lookForWorkaround(right, "comparison").toSint16();
 }
 
+int reg_t::sci32Comparison(const reg_t right) const {
+	// In SCI32, MemIDs are normally indexes into the memory manager's handle
+	// list, but the engine reserves indexes at and above 20000 for objects
+	// that were created inside the engine (as opposed to inside the VM). The
+	// engine compares these as a tiebreaker for graphics objects that are at
+	// the same priority, and it is necessary to at least minimally handle
+	// this situation.
+	// This is obviously a bogus comparision, but then, this entire thing is
+	// bogus. For the moment, it just needs to be deterministic.
+	if (isNumber() && !right.isNumber()) {
+		return 1;
+	} else if (right.isNumber() && !isNumber()) {
+		return -1;
+	}
+
+	return getOffset() - right.getOffset();
+}
+
 bool reg_t::pointerComparisonWithInteger(const reg_t right) const {
 	// This function handles the case where a script tries to compare a pointer
 	// to a number. Normally, we would not want to allow that. However, SCI0 -
diff --git a/engines/sci/engine/vm_types.h b/engines/sci/engine/vm_types.h
index a646478..e60f52e 100644
--- a/engines/sci/engine/vm_types.h
+++ b/engines/sci/engine/vm_types.h
@@ -160,6 +160,10 @@ private:
 	int cmp(const reg_t right, bool treatAsUnsigned) const;
 	reg_t lookForWorkaround(const reg_t right, const char *operation) const;
 	bool pointerComparisonWithInteger(const reg_t right) const;
+
+#ifdef ENABLE_SCI32
+	int sci32Comparison(const reg_t right) const;
+#endif
 };
 
 static inline reg_t make_reg(SegmentId segment, uint16 offset) {
diff --git a/engines/sci/graphics/plane32.h b/engines/sci/graphics/plane32.h
index f60f0cc..d3427f1 100644
--- a/engines/sci/graphics/plane32.h
+++ b/engines/sci/graphics/plane32.h
@@ -254,34 +254,12 @@ public:
 	void operator=(const Plane &other);
 
 	inline bool operator<(const Plane &other) const {
-		// TODO: In SCI engine, _object is actually a uint16 and can either
-		// contain a MemID (a handle to MemoryMgr, similar to reg_t) or
-		// a serial (Plane::_nextObjectId). These numbers can be compared
-		// directly in the real engine and the lowest MemID wins, but in
-		// ScummVM reg_t pointers are not comparable so we have to use a
-		// different strategy when two planes generated by scripts conflict.
-		// For now we just don't check if the priority is below 0, since
-		// that priority is used to represent hidden planes and is guaranteed
-		// to generate conflicts with script-generated planes. If there are
-		// other future conflicts with script-generated planes then we need
-		// to come up with a solution that works, similar to
-		// reg_t::pointerComparisonWithInteger used by SCI16.
-		//
-		// For now, we check the object offsets, as this will likely work
-		// like in the original SCI engine, without comparing objects.
-		// However, this whole comparison is quite ugly, and if it still
-		// fails, we should try to change it to something equivalent, to avoid
-		// adding loads of workarounds just for this
 		if (_priority < other._priority) {
 			return true;
 		}
 
 		if (_priority == other._priority) {
-			if (_object.isNumber() && other._object.isNumber()) {
-				return _object < other._object;
-			} else if (other._object.isNumber()) {
-				return true;
-			}
+			return _object < other._object;
 		}
 
 		return false;
diff --git a/engines/sci/graphics/screen_item32.h b/engines/sci/graphics/screen_item32.h
index e86efda..0840570 100644
--- a/engines/sci/graphics/screen_item32.h
+++ b/engines/sci/graphics/screen_item32.h
@@ -229,14 +229,7 @@ public:
 			}
 
 			if (_position.y + _z == other._position.y + other._z) {
-				// TODO: Failure in SQ6 room 220 when using SCI logic
-				// to compare pointer and numeric memory handles
-
-				if (_object.isNumber() && other._object.isNumber()) {
-					return _object < other._object;
-				} else if (other._object.isNumber()) {
-					return true;
-				}
+				return _object < other._object;
 			}
 		}
 


Commit: 4297305dd50d0c4e22bbb8fe4636bbce88dae684
    https://github.com/scummvm/scummvm/commit/4297305dd50d0c4e22bbb8fe4636bbce88dae684
Author: Colin Snover (github.com at zetafleet.com)
Date: 2016-03-07T16:46:25-06:00

Commit Message:
SCI32: Clean up renderer code a bit more

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



diff --git a/engines/sci/graphics/plane32.cpp b/engines/sci/graphics/plane32.cpp
index 09d8dc0..d955632 100644
--- a/engines/sci/graphics/plane32.cpp
+++ b/engines/sci/graphics/plane32.cpp
@@ -297,7 +297,7 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 	ScreenItemList::size_type planeItemCount = _screenItemList.size();
 	ScreenItemList::size_type visiblePlaneItemCount = visiblePlane._screenItemList.size();
 
-	for (PlaneList::size_type i = 0; i < planeItemCount; ++i) {
+	for (ScreenItemList::size_type i = 0; i < planeItemCount; ++i) {
 		ScreenItem *vitem = nullptr;
 		// NOTE: The original engine used an array without bounds checking
 		// so could just get the visible screen item directly; we need to
@@ -312,13 +312,15 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 		if (i < _screenItemList.size() && item != nullptr) {
 			if (item->_deleted) {
 				// add item's rect to erase list
-				if (i < visiblePlane._screenItemList.size() && vitem != nullptr) {
-					if (!vitem->_screenRect.isEmpty()) {
-						if (/* TODO: g_Remap_numActiveRemaps */ false) { // active remaps?
-							mergeToRectList(vitem->_screenRect, eraseList);
-						} else {
-							eraseList.add(vitem->_screenRect);
-						}
+				if (
+					i < visiblePlane._screenItemList.size() &&
+					vitem != nullptr &&
+					!vitem->_screenRect.isEmpty()
+				) {
+					if (/* TODO: g_Remap_numActiveRemaps */ false) { // active remaps?
+						mergeToRectList(vitem->_screenRect, eraseList);
+					} else {
+						eraseList.add(vitem->_screenRect);
 					}
 				}
 			} else if (item->_created) {
@@ -351,7 +353,11 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 							drawList.add(item, item->_screenRect);
 							mergeToRectList(item->_screenRect, eraseList);
 						}
-						if (i < visiblePlaneItemCount && vitem != nullptr && !vitem->_screenRect.isEmpty()) {
+						if (
+							i < visiblePlaneItemCount &&
+							vitem != nullptr &&
+							!vitem->_screenRect.isEmpty()
+						) {
 							mergeToRectList(vitem->_screenRect, eraseList);
 						}
 					} else {
@@ -359,10 +365,11 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 						// and item to draw list
 
 						// TODO: This was changed from disasm, verify please!
-						Common::Rect extendedScreenItem = vitem->_screenRect;
-						extendedScreenItem.extend(item->_screenRect);
+						Common::Rect extendedScreenRect = vitem->_screenRect;
+						extendedScreenRect.extend(item->_screenRect);
+
 						drawList.add(item, item->_screenRect);
-						mergeToRectList(extendedScreenItem, eraseList);
+						mergeToRectList(extendedScreenRect, eraseList);
 					}
 				} else {
 					// if no active remaps, just add item to draw list and old rect
@@ -370,7 +377,11 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 					if (!item->_screenRect.isEmpty()) {
 						drawList.add(item, item->_screenRect);
 					}
-					if (i < visiblePlaneItemCount && vitem != nullptr && !vitem->_screenRect.isEmpty()) {
+					if (
+						i < visiblePlaneItemCount &&
+						vitem != nullptr &&
+						!vitem->_screenRect.isEmpty()
+					) {
 						eraseList.add(vitem->_screenRect);
 					}
 				}
@@ -433,12 +444,17 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 		for (RectList::size_type i = 0; i < eraseList.size(); ++i) {
 			for (ScreenItemList::size_type j = 0; j < _screenItemList.size(); ++j) {
 				ScreenItem *item = _screenItemList[j];
-				if (item != nullptr && !item->_updated && !item->_deleted && !item->_created && eraseList[i]->intersects(item->_screenRect)) {
+				if (
+					item != nullptr &&
+					!item->_created && !item->_updated && !item->_deleted &&
+					eraseList[i]->intersects(item->_screenRect)
+				) {
 					drawList.add(item, eraseList[i]->findIntersectingRect(item->_screenRect));
 				}
 			}
 		}
 	}
+
 	if (/* TODO: g_Remap_numActiveRemaps == 0 */ true) { // no remaps active?
 		// Add all items that overlap with items in the drawlist and have higher
 		// priority.
@@ -449,32 +465,21 @@ void Plane::calcLists(Plane &visiblePlane, const PlaneList &planeList, DrawList
 		for (DrawList::size_type i = 0; i < drawListSizePrimary; ++i) {
 			DrawItem *dli = drawList[i];
 
-			for (PlaneList::size_type j = 0; j < planeItemCount; ++j) {
+			for (ScreenItemList::size_type j = 0; j < planeItemCount; ++j) {
 				ScreenItem *sli = _screenItemList[j];
 
-				if (i < drawList.size() && dli) {
-					if (j < _screenItemList.size() && sli) {
-						if (!sli->_updated && !sli->_deleted && !sli->_created) {
-							ScreenItem *item = dli->screenItem;
+				if (
+					i < drawList.size() && dli != nullptr &&
+					j < _screenItemList.size() && sli != nullptr &&
+					!sli->_created && !sli->_updated && !sli->_deleted
+				) {
+					ScreenItem *item = dli->screenItem;
 
-							bool isAbove = false;
-							if (sli->_priority > item->_priority) {
-								isAbove = true;
-							}
-							else if (sli->_priority == item->_priority) {
-								if (sli->_object.isNumber() && item->_object.isNumber()) {
-									isAbove = sli->_object > item->_object;
-								} else if (sli->_object.isNumber()) {
-									isAbove = true;
-								}
-							}
-
-							if (isAbove) {
-								if (dli->rect.intersects(sli->_screenRect)) {
-									drawList.add(sli, dli->rect.findIntersectingRect(sli->_screenRect));
-								}
-							}
-						}
+					if (
+						(sli->_priority > item->_priority || (sli->_priority == item->_priority && sli->_object > item->_object)) &&
+						dli->rect.intersects(sli->_screenRect)
+					) {
+						drawList.add(sli, dli->rect.findIntersectingRect(sli->_screenRect));
 					}
 				}
 			}
@@ -513,8 +518,7 @@ void Plane::decrementScreenItemArrayCounts(Plane *visiblePlane, const bool force
 			if (item->_created) {
 				item->_created--;
 				if (visiblePlane != nullptr) {
-					ScreenItem *n = new ScreenItem(*item);
-					visiblePlane->_screenItemList.add(n);
+					visiblePlane->_screenItemList.add(new ScreenItem(*item));
 				}
 			}
 
@@ -604,14 +608,13 @@ void Plane::filterUpEraseRects(DrawList &drawList, RectList &eraseList) const {
 	}
 }
 
-void Plane::mergeToDrawList(const DrawList::size_type index, const Common::Rect &rect, DrawList &drawList) const {
+void Plane::mergeToDrawList(const ScreenItemList::size_type index, const Common::Rect &rect, DrawList &drawList) const {
 	RectList rects;
 
-	Common::Rect r = _screenItemList[index]->_screenRect;
+	ScreenItem *item = _screenItemList[index];
+	Common::Rect r = item->_screenRect;
 	r.clip(rect);
-
 	rects.add(r);
-	ScreenItem *item = _screenItemList[index];
 
 	for (RectList::size_type i = 0; i < rects.size(); ++i) {
 		r = *rects[i];






More information about the Scummvm-git-logs mailing list