[Scummvm-git-logs] scummvm master -> e8c7f6f7e8962ddcc46cad1311eafa713360e168
bluegr
noreply at scummvm.org
Sat Sep 30 09:26:51 UTC 2023
This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
b016bcdf70 SCI32: Fix DrawList overflow with a dynamic array
e8c7f6f7e8 SCI32: Reuse DrawLists when rendering frames
Commit: b016bcdf707d75d7969a7d9a934f5e5c266ffc1f
https://github.com/scummvm/scummvm/commit/b016bcdf707d75d7969a7d9a934f5e5c266ffc1f
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2023-09-30T12:26:46+03:00
Commit Message:
SCI32: Fix DrawList overflow with a dynamic array
This is an engine limitation that also exists in SSCI,
but it can be exceeded under normal conditions.
Fixes crash at LSL7 pool entrance, bug #14632
Changed paths:
engines/sci/graphics/lists32.h
engines/sci/graphics/plane32.h
diff --git a/engines/sci/graphics/lists32.h b/engines/sci/graphics/lists32.h
index a666129a5e6..52bc02b0a60 100644
--- a/engines/sci/graphics/lists32.h
+++ b/engines/sci/graphics/lists32.h
@@ -28,10 +28,16 @@ namespace Sci {
/**
* StablePointerArray holds pointers in a fixed-size array that maintains
- * position of erased items until `pack` is called. It is used by DrawList,
- * RectList, and ScreenItemList. StablePointerArray takes ownership of all
- * pointers that are passed to it and deletes them when calling `erase` or when
+ * position of erased items until `pack` is called. It is used by RectList
+ * and ScreenItemList. StablePointerArray takes ownership of all pointers
+ * that are passed to it and deletes them when calling `erase` or when
* destroying the StablePointerArray.
+ *
+ * StablePointerArray used to be used for DrawList, until it was discovered
+ * that an LSL7 room with many screen items can overflow the fixed array.
+ * StablePointerDynamicArray was created below to handle DrawList, while
+ * StablePointerArray keeps the performance advantages of fixed arrays on
+ * the stack when rendering frames.
*/
template<class T, uint N>
class StablePointerArray {
@@ -182,6 +188,147 @@ public:
}
};
+/**
+ * StablePointerDynamicArray is like StablePointerArray above, except that
+ * it uses a Common::Array for storage instead of a fixed array.
+ * It is only used by DrawList, and was created upon discovering that LSL7
+ * room 301 can overflow DrawList when displaying a large menu. Bug #14632
+ */
+template<class T, uint N>
+class StablePointerDynamicArray {
+ Common::Array<T *> _items;
+
+public:
+ typedef T **iterator;
+ typedef T *const *const_iterator;
+ typedef T *value_type;
+ typedef uint size_type;
+
+ StablePointerDynamicArray() {
+ _items.reserve(N);
+ }
+ StablePointerDynamicArray(const StablePointerDynamicArray &other) {
+ _items.reserve(MAX(N, other.size()));
+ for (size_type i = 0; i < other.size(); ++i) {
+ if (other._items[i] == nullptr) {
+ _items.push_back(nullptr);
+ } else {
+ _items.push_back(new T(*other._items[i]));
+ }
+ }
+ }
+ ~StablePointerDynamicArray() {
+ for (size_type i = 0; i < _items.size(); ++i) {
+ delete _items[i];
+ }
+ }
+
+ void operator=(StablePointerDynamicArray &other) {
+ clear();
+ for (size_type i = 0; i < other.size(); ++i) {
+ if (other._items[i] == nullptr) {
+ _items.push_back(nullptr);
+ } else {
+ _items.push_back(new T(*other._items[i]));
+ }
+ }
+ }
+
+ T *const &operator[](size_type index) const {
+ return _items[index];
+ }
+
+ T *&operator[](size_type index) {
+ return _items[index];
+ }
+
+ /**
+ * Adds a new pointer to the array.
+ */
+ void add(T *item) {
+ _items.push_back(item);
+ }
+
+ iterator begin() {
+ return _items.begin();
+ }
+
+ const_iterator begin() const {
+ return _items.begin();
+ }
+
+ void clear() {
+ for (size_type i = 0; i < _items.size(); ++i) {
+ delete _items[i];
+ }
+ _items.resize(0);
+ }
+
+ iterator end() {
+ return _items.end();
+ }
+
+ const_iterator end() const {
+ return _items.end();
+ }
+
+ /**
+ * Erases the object pointed to by the given iterator.
+ */
+ void erase(T *item) {
+ for (iterator it = begin(); it != end(); ++it) {
+ if (*it == item) {
+ delete *it;
+ *it = nullptr;
+ break;
+ }
+ }
+ }
+
+ /**
+ * Erases the object pointed to by the given iterator.
+ */
+ void erase(iterator &it) {
+ assert(it >= begin() && it < end());
+ delete *it;
+ *it = nullptr;
+ }
+
+ /**
+ * Erases the object pointed to at the given index.
+ */
+ void erase_at(size_type index) {
+ delete _items[index];
+ _items[index] = nullptr;
+ }
+
+ /**
+ * Removes freed pointers from the pointer list.
+ */
+ size_type pack() {
+ iterator freePtr = begin();
+ size_type newSize = 0;
+
+ for (iterator it = begin(), last = end(); it != last; ++it) {
+ if (*it != nullptr) {
+ *freePtr = *it;
+ ++freePtr;
+ ++newSize;
+ }
+ }
+ _items.resize(newSize);
+ return newSize;
+ }
+
+ /**
+ * The number of populated slots in the array. The size
+ * of the array will only go down once `pack` is called.
+ */
+ size_type size() const {
+ return _items.size();
+ }
+};
+
template<typename T>
class FindByObject {
const reg_t &_object;
diff --git a/engines/sci/graphics/plane32.h b/engines/sci/graphics/plane32.h
index 2c1c8ff9309..7aa40b57e43 100644
--- a/engines/sci/graphics/plane32.h
+++ b/engines/sci/graphics/plane32.h
@@ -78,7 +78,7 @@ struct DrawItem {
}
};
-typedef StablePointerArray<DrawItem, 250> DrawListBase;
+typedef StablePointerDynamicArray<DrawItem, 250> DrawListBase;
class DrawList : public DrawListBase {
private:
inline static bool sortHelper(const DrawItem *a, const DrawItem *b) {
Commit: e8c7f6f7e8962ddcc46cad1311eafa713360e168
https://github.com/scummvm/scummvm/commit/e8c7f6f7e8962ddcc46cad1311eafa713360e168
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2023-09-30T12:26:46+03:00
Commit Message:
SCI32: Reuse DrawLists when rendering frames
When converting DrawList to use Common::Array in the previous commit,
I couldn't see a reason to not reuse them in frameOut() and avoid
the extra heap allocations on every frame.
Changed paths:
engines/sci/graphics/frameout.cpp
engines/sci/graphics/frameout.h
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 3fffd724a8e..e3d6d5c4bfb 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -130,6 +130,7 @@ void GfxFrameout::clear() {
_planes.clear();
_visiblePlanes.clear();
_showList.clear();
+ _screenItemLists.clear();
}
bool GfxFrameout::detectHiRes() const {
@@ -481,23 +482,23 @@ void GfxFrameout::frameOut(const bool shouldShowBits, const Common::Rect &eraseR
// SSCI allocated these as static arrays of 100 pointers to
// ScreenItemList / RectList
- ScreenItemListList screenItemLists;
- EraseListList eraseLists;
-
- screenItemLists.resize(_planes.size());
- eraseLists.resize(_planes.size());
+ _screenItemLists.resize(_planes.size());
+ for (DrawList::size_type i = 0; i < _screenItemLists.size(); ++i) {
+ _screenItemLists[i].clear();
+ }
+ EraseListList eraseLists(_planes.size());
if (g_sci->_gfxRemap32->getRemapCount() > 0 && _remapOccurred) {
remapMarkRedraw();
}
- calcLists(screenItemLists, eraseLists, eraseRect);
+ calcLists(_screenItemLists, eraseLists, eraseRect);
- for (ScreenItemListList::iterator list = screenItemLists.begin(); list != screenItemLists.end(); ++list) {
+ for (ScreenItemListList::iterator list = _screenItemLists.begin(); list != _screenItemLists.end(); ++list) {
list->sort();
}
- for (ScreenItemListList::iterator list = screenItemLists.begin(); list != screenItemLists.end(); ++list) {
+ for (ScreenItemListList::iterator list = _screenItemLists.begin(); list != _screenItemLists.end(); ++list) {
for (DrawList::iterator drawItem = list->begin(); drawItem != list->end(); ++drawItem) {
(*drawItem)->screenItem->getCelObj().submitPalette();
}
@@ -507,7 +508,7 @@ void GfxFrameout::frameOut(const bool shouldShowBits, const Common::Rect &eraseR
for (PlaneList::size_type i = 0; i < _planes.size(); ++i) {
drawEraseList(eraseLists[i], *_planes[i]);
- drawScreenItemList(screenItemLists[i]);
+ drawScreenItemList(_screenItemLists[i]);
}
if (robotIsActive) {
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 30e1cd383d1..9e38dfe9558 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -328,6 +328,12 @@ private:
*/
RectList _showList;
+ /**
+ * A list of DrawLists used by frameOut(). This is a field to avoid
+ * constructing and destroying DrawLists on every frame.
+ */
+ ScreenItemListList _screenItemLists;
+
/**
* The amount of extra overdraw that is acceptable when merging two show
* list rectangles together into a single larger rectangle.
More information about the Scummvm-git-logs
mailing list