SCI32: Enable optional explicit memory management of hunk entries

Bitmaps in ScrollWindow and Robot code are managed by the kernel
and not by game scripts, although they must be able to be
referenced through a reg_t. To prevent incorrect GC of bitmaps
that are in use but not referenced by any game script, explicit
memory management of hunk entries can be enabled.

8 files changed, 20 insertions, 33 deletions

diff --git a/engines/sci/graphics/controls32.cpp b/engines/sci/graphics/controls32.cpp
index f4e9d26df4..639a35c1a6 100644
--- a/engines/sci/graphics/controls32.cpp
+++ b/engines/sci/graphics/controls32.cpp
@@ -54,18 +54,6 @@ GfxControls32::~GfxControls32() {
 }
 
 #pragma mark -
-#pragma mark Garbage collection
-
-Common::Array<reg_t> GfxControls32::listObjectReferences() {
-	Common::Array<reg_t> ret;
-	ScrollWindowMap::const_iterator it;
-	for (it = _scrollWindows.begin(); it != _scrollWindows.end(); ++it)
-		ret.push_back(it->_value->getBitmap());
-
-	return ret;
-}
-
-#pragma mark -
 #pragma mark Text input control
 
 reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
@@ -130,7 +118,7 @@ reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
 
 		if (titleObject.isNull()) {
 			bool dimmed = readSelectorValue(_segMan, controlObject, SELECTOR(dimmed));
-			editor.bitmap = _gfxText32->createFontBitmap(width, height, editor.textRect, editor.text, editor.foreColor, editor.backColor, editor.skipColor, editor.fontId, alignment, editor.borderColor, dimmed, true);
+			editor.bitmap = _gfxText32->createFontBitmap(width, height, editor.textRect, editor.text, editor.foreColor, editor.backColor, editor.skipColor, editor.fontId, alignment, editor.borderColor, dimmed, true, false);
 		} else {
 			error("Titled bitmaps are not known to be used by any game. Please submit a bug report with details about the game you were playing and what you were doing that triggered this error. Thanks!");
 		}
@@ -383,6 +371,7 @@ void GfxControls32::flashCursor(TextEditor &editor) {
 #pragma mark Scrollable window control
 
 ScrollWindow::ScrollWindow(SegManager *segMan, const Common::Rect &gameRect, const Common::Point &position, const reg_t plane, const uint8 defaultForeColor, const uint8 defaultBackColor, const GuiResourceId defaultFontId, const TextAlign defaultAlignment, const int16 defaultBorderColor, const uint16 maxNumEntries) :
+	_segMan(segMan),
 	_gfxText32(segMan, g_sci->_gfxCache),
 	_maxNumEntries(maxNumEntries),
 	_firstVisibleChar(0),
@@ -424,13 +413,13 @@ ScrollWindow::ScrollWindow(SegManager *segMan, const Common::Rect &gameRect, con
 	}
 
 	assert(bitmapRect.width() > 0 && bitmapRect.height() > 0);
-	_bitmap = _gfxText32.createFontBitmap(bitmapRect.width(), bitmapRect.height(), _textRect, "", _foreColor, _backColor, skipColor, _fontId, _alignment, _borderColor, false, false);
+	_bitmap = _gfxText32.createFontBitmap(bitmapRect.width(), bitmapRect.height(), _textRect, "", _foreColor, _backColor, skipColor, _fontId, _alignment, _borderColor, false, false, false);
 
 	debugC(1, kDebugLevelGraphics, "New ScrollWindow: textRect size: %d x %d, bitmap: %04x:%04x", _textRect.width(), _textRect.height(), PRINT_REG(_bitmap));
 }
 
 ScrollWindow::~ScrollWindow() {
-	// _gfxText32._bitmap will get GCed once ScrollWindow is gone.
+	_segMan->freeHunkEntry(_bitmap);
 	// _screenItem will be deleted by GfxFrameout
 }
 
diff --git a/engines/sci/graphics/controls32.h b/engines/sci/graphics/controls32.h
index 460b0b5625..680c70d2d6 100644
--- a/engines/sci/graphics/controls32.h
+++ b/engines/sci/graphics/controls32.h
@@ -227,6 +227,8 @@ public:
 	const reg_t getBitmap() const { return _bitmap; }
 
 private:
+	SegManager *_segMan;
+
 	typedef Common::Array<ScrollWindowEntry> EntriesList;
 
 	/**
@@ -418,11 +420,6 @@ private:
 	GfxText32 *_gfxText32;
 
 #pragma mark -
-#pragma mark Garbage collection
-public:
-	Common::Array<reg_t> listObjectReferences();
-
-#pragma mark -
 #pragma mark Text input control
 public:
 	reg_t kernelEditText(const reg_t controlObject);
diff --git a/engines/sci/graphics/paint16.cpp b/engines/sci/graphics/paint16.cpp
index 6004e9ce7a..4517659283 100644
--- a/engines/sci/graphics/paint16.cpp
+++ b/engines/sci/graphics/paint16.cpp
@@ -334,7 +334,7 @@ reg_t GfxPaint16::bitsSave(const Common::Rect &rect, byte screenMask) {
 	// now actually ask _screen how much space it will need for saving
 	size = _screen->bitsGetDataSize(workerRect, screenMask);
 
-	memoryId = _segMan->allocateHunkEntry("SaveBits()", size);
+	memoryId = _segMan->allocateHunkEntry("SaveBits()", size, true);
 	memoryPtr = _segMan->getHunkPointer(memoryId);
 	if (memoryPtr)
 		_screen->bitsSave(workerRect, screenMask, memoryPtr);
diff --git a/engines/sci/graphics/paint32.cpp b/engines/sci/graphics/paint32.cpp
index 7773b9f4e1..8a9d9b9657 100644
--- a/engines/sci/graphics/paint32.cpp
+++ b/engines/sci/graphics/paint32.cpp
@@ -128,7 +128,7 @@ BitmapResource GfxPaint32::makeLineBitmap(const Common::Point &startPoint, const
 	outRect.right = (startPoint.x > endPoint.x ? startPoint.x : endPoint.x) + halfThickness + 1;
 	outRect.bottom = (startPoint.y > endPoint.y ? startPoint.y : endPoint.y) + halfThickness + 1;
 
-	BitmapResource bitmap(_segMan, outRect.width(), outRect.height(), skipColor, 0, 0, g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth, g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight, 0, false);
+	BitmapResource bitmap(_segMan, outRect.width(), outRect.height(), skipColor, 0, 0, g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth, g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight, 0, false, true);
 
 	byte *pixels = bitmap.getPixels();
 	memset(pixels, skipColor, bitmap.getWidth() * bitmap.getHeight());
diff --git a/engines/sci/graphics/palette.cpp b/engines/sci/graphics/palette.cpp
index 1514ad838f..a26c92c364 100644
--- a/engines/sci/graphics/palette.cpp
+++ b/engines/sci/graphics/palette.cpp
@@ -626,7 +626,7 @@ void GfxPalette::kernelAnimateSet() {
 
 reg_t GfxPalette::kernelSave() {
 	SegManager *segMan = g_sci->getEngineState()->_segMan;
-	reg_t memoryId = segMan->allocateHunkEntry("kPalette(save)", 1024);
+	reg_t memoryId = segMan->allocateHunkEntry("kPalette(save)", 1024, true);
 	byte *memoryPtr = segMan->getHunkPointer(memoryId);
 	if (memoryPtr) {
 		for (int colorNr = 0; colorNr < 256; colorNr++) {
diff --git a/engines/sci/graphics/text32.cpp b/engines/sci/graphics/text32.cpp
index 277e6e93d0..d612ddbfa3 100644
--- a/engines/sci/graphics/text32.cpp
+++ b/engines/sci/graphics/text32.cpp
@@ -59,7 +59,7 @@ GfxText32::GfxText32(SegManager *segMan, GfxCache *fonts) :
 		}
 	}
 
-reg_t GfxText32::createFontBitmap(int16 width, int16 height, const Common::Rect &rect, const Common::String &text, const uint8 foreColor, const uint8 backColor, const uint8 skipColor, const GuiResourceId fontId, const TextAlign alignment, const int16 borderColor, const bool dimmed, const bool doScaling) {
+reg_t GfxText32::createFontBitmap(int16 width, int16 height, const Common::Rect &rect, const Common::String &text, const uint8 foreColor, const uint8 backColor, const uint8 skipColor, const GuiResourceId fontId, const TextAlign alignment, const int16 borderColor, const bool dimmed, const bool doScaling, const bool gc) {
 
 	_borderColor = borderColor;
 	_text = text;
@@ -96,7 +96,7 @@ reg_t GfxText32::createFontBitmap(int16 width, int16 height, const Common::Rect
 		_textRect = Common::Rect();
 	}
 
-	BitmapResource bitmap(_segMan, _width, _height, _skipColor, 0, 0, _scaledWidth, _scaledHeight, 0, false);
+	BitmapResource bitmap(_segMan, _width, _height, _skipColor, 0, 0, _scaledWidth, _scaledHeight, 0, false, gc);
 	_bitmap = bitmap.getObject();
 
 	erase(bitmapRect, false);
@@ -109,7 +109,7 @@ reg_t GfxText32::createFontBitmap(int16 width, int16 height, const Common::Rect
 	return _bitmap;
 }
 
-reg_t GfxText32::createFontBitmap(const CelInfo32 &celInfo, const Common::Rect &rect, const Common::String &text, const int16 foreColor, const int16 backColor, const GuiResourceId fontId, const int16 skipColor, const int16 borderColor, const bool dimmed) {
+reg_t GfxText32::createFontBitmap(const CelInfo32 &celInfo, const Common::Rect &rect, const Common::String &text, const int16 foreColor, const int16 backColor, const GuiResourceId fontId, const int16 skipColor, const int16 borderColor, const bool dimmed, const bool gc) {
 	_borderColor = borderColor;
 	_text = text;
 	_textRect = rect;
@@ -135,7 +135,7 @@ reg_t GfxText32::createFontBitmap(const CelInfo32 &celInfo, const Common::Rect &
 		_textRect = Common::Rect();
 	}
 
-	BitmapResource bitmap(_segMan, _width, _height, _skipColor, 0, 0, _scaledWidth, _scaledHeight, 0, false);
+	BitmapResource bitmap(_segMan, _width, _height, _skipColor, 0, 0, _scaledWidth, _scaledHeight, 0, false, gc);
 	_bitmap = bitmap.getObject();
 
 	// NOTE: The engine filled the bitmap pixels with 11 here, which is silly
diff --git a/engines/sci/graphics/text32.h b/engines/sci/graphics/text32.h
index f57433f42d..40921563d0 100644
--- a/engines/sci/graphics/text32.h
+++ b/engines/sci/graphics/text32.h
@@ -117,8 +117,8 @@ public:
 	 * Allocates and initialises a new bitmap in the given
 	 * segment manager.
 	 */
-	inline BitmapResource(SegManager *segMan, const int16 width, const int16 height, const uint8 skipColor, const int16 displaceX, const int16 displaceY, const int16 scaledWidth, const int16 scaledHeight, const uint32 hunkPaletteOffset, const bool remap) {
-		_object = segMan->allocateHunkEntry("Bitmap()", getBitmapSize(width, height));
+	inline BitmapResource(SegManager *segMan, const int16 width, const int16 height, const uint8 skipColor, const int16 displaceX, const int16 displaceY, const int16 scaledWidth, const int16 scaledHeight, const uint32 paletteSize, const bool remap, const bool gc) {
+		_object = segMan->allocateHunkEntry("Bitmap()", getBitmapSize(width, height) + paletteSize, gc);
 		_bitmap = segMan->getHunkPointer(_object);
 
 		const uint16 bitmapHeaderSize = getBitmapHeaderSize();
@@ -132,7 +132,7 @@ public:
 		setRemap(remap);
 		setDataSize(width * height);
 		WRITE_SCI11ENDIAN_UINT32(_bitmap + 16, 0);
-		setHunkPaletteOffset(hunkPaletteOffset);
+		setHunkPaletteOffset(paletteSize > 0 ? (width * height) : 0);
 		setDataOffset(bitmapHeaderSize);
 		setUncompressedDataOffset(bitmapHeaderSize);
 		setControlOffset(0);
@@ -394,12 +394,12 @@ public:
 	 * Creates a plain font bitmap with a flat color
 	 * background.
 	 */
-	reg_t createFontBitmap(int16 width, int16 height, const Common::Rect &rect, const Common::String &text, const uint8 foreColor, const uint8 backColor, const uint8 skipColor, const GuiResourceId fontId, TextAlign alignment, const int16 borderColor, bool dimmed, const bool doScaling);
+	reg_t createFontBitmap(int16 width, int16 height, const Common::Rect &rect, const Common::String &text, const uint8 foreColor, const uint8 backColor, const uint8 skipColor, const GuiResourceId fontId, TextAlign alignment, const int16 borderColor, bool dimmed, const bool doScaling, const bool gc);
 
 	/**
 	 * Creates a font bitmap with a view background.
 	 */
-	reg_t createFontBitmap(const CelInfo32 &celInfo, const Common::Rect &rect, const Common::String &text, const int16 foreColor, const int16 backColor, const GuiResourceId fontId, const int16 skipColor, const int16 borderColor, const bool dimmed);
+	reg_t createFontBitmap(const CelInfo32 &celInfo, const Common::Rect &rect, const Common::String &text, const int16 foreColor, const int16 backColor, const GuiResourceId fontId, const int16 skipColor, const int16 borderColor, const bool dimmed, const bool gc);
 
 	inline int scaleUpWidth(int value) const {
 		const int scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index dd841f5b4c..46607066f6 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -117,6 +117,7 @@ VMDPlayer::IOStatus VMDPlayer::close() {
 
 	if (!_planeIsOwned && _screenItem != nullptr) {
 		g_sci->_gfxFrameout->deleteScreenItem(*_screenItem);
+		g_sci->getEngineState()->_segMan->freeHunkEntry(_screenItem->_celInfo.bitmap);
 		_screenItem = nullptr;
 	} else if (_plane != nullptr) {
 		g_sci->_gfxFrameout->deletePlane(*_plane);
@@ -231,7 +232,7 @@ VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
 		const int16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
 		const int16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
 
-		BitmapResource vmdBitmap(_segMan, vmdRect.width(), vmdRect.height(), 255, 0, 0, screenWidth, screenHeight, 0, false);
+		BitmapResource vmdBitmap(_segMan, vmdRect.width(), vmdRect.height(), 255, 0, 0, screenWidth, screenHeight, 0, false, false);
 
 		if (screenWidth != scriptWidth || screenHeight != scriptHeight) {
 			mulru(vmdRect, Ratio(scriptWidth, screenWidth), Ratio(scriptHeight, screenHeight), 1);