[Scummvm-git-logs] scummvm master -> 3e433ad94362008916bb7cb2bcdad360c6bb8a1e

peterkohaut noreply at scummvm.org
Wed Dec 11 12:24:55 UTC 2024


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:
5be3b9e1d7 BLADERUNNER: Don't read pixels using READ_UINT32
a81bfbe805 BLADERUNNER: Avoid memory leak in overlays
3e433ad943 BLADERUNNER: Garbage collect the unused pages


Commit: 5be3b9e1d74cc8855153d24477bee9e94778c124
    https://github.com/scummvm/scummvm/commit/5be3b9e1d74cc8855153d24477bee9e94778c124
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-12-11T13:24:50+01:00

Commit Message:
BLADERUNNER: Don't read pixels using READ_UINT32

This breaks when the pixel format is 16 bits and we are reading the last
pixel of the surface.

Changed paths:
    engines/bladerunner/slice_renderer.cpp


diff --git a/engines/bladerunner/slice_renderer.cpp b/engines/bladerunner/slice_renderer.cpp
index 5cd88ac2120..87331edde9f 100644
--- a/engines/bladerunner/slice_renderer.cpp
+++ b/engines/bladerunner/slice_renderer.cpp
@@ -732,8 +732,10 @@ void SliceRenderer::drawShadowPolygon(int transparency, Graphics::Surface &surfa
 			if (z >= zMin) {
 				int index = (x & 3) + ((y & 3) << 2);
 				if (transparency - ditheringFactor[index] <= 0) {
+					uint32 color;
 					uint8 r, g, b;
-					surface.format.colorToRGB(READ_UINT32(pixel), r, g, b);
+					getPixel(surface, pixel, color);
+					surface.format.colorToRGB(color, r, g, b);
 					r *= 0.75f;
 					g *= 0.75f;
 					b *= 0.75f;


Commit: a81bfbe80577ae94138d705019bad3170e320c79
    https://github.com/scummvm/scummvm/commit/a81bfbe80577ae94138d705019bad3170e320c79
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-12-11T13:24:50+01:00

Commit Message:
BLADERUNNER: Avoid memory leak in overlays

Sometimes, the overlay is played twice in a row which causes the player
to reload the stream.

Changed paths:
    engines/bladerunner/vqa_decoder.cpp
    engines/bladerunner/vqa_decoder.h
    engines/bladerunner/vqa_player.cpp


diff --git a/engines/bladerunner/vqa_decoder.cpp b/engines/bladerunner/vqa_decoder.cpp
index d126b323423..9919b4b020a 100644
--- a/engines/bladerunner/vqa_decoder.cpp
+++ b/engines/bladerunner/vqa_decoder.cpp
@@ -148,17 +148,32 @@ VQADecoder::VQADecoder() {
 }
 
 VQADecoder::~VQADecoder() {
+	close();
+}
+
+void VQADecoder::close() {
 	for (uint i = _codebooks.size(); i != 0; --i) {
 		delete[] _codebooks[i - 1].data;
 	}
+	_codebooks.clear();
+
 	delete _audioTrack;
+	_audioTrack = nullptr;
+
 	delete _videoTrack;
+	_videoTrack = nullptr;
+
 	delete[] _frameInfo;
+	_frameInfo = nullptr;
+
+	_loopInfo.close();
+
 	deleteVQPTable();
 }
 
 bool VQADecoder::loadStream(Common::SeekableReadStream *s) {
-	// close();
+	close();
+
 	_s = s;
 
 	IFFChunkHeader chd;
diff --git a/engines/bladerunner/vqa_decoder.h b/engines/bladerunner/vqa_decoder.h
index 6dd9d9a67c8..545ac72a178 100644
--- a/engines/bladerunner/vqa_decoder.h
+++ b/engines/bladerunner/vqa_decoder.h
@@ -64,6 +64,7 @@ public:
 	~VQADecoder();
 
 	bool loadStream(Common::SeekableReadStream *s);
+	void close();
 
 	void readFrame(int frame, uint readFlags = kVQAReadAll);
 
@@ -135,7 +136,14 @@ public:
 
 		LoopInfo() : loopCount(0), loops(nullptr), flags(0) {}
 		~LoopInfo() {
+			close();
+		}
+
+		void close() {
 			delete[] loops;
+			loops = nullptr;
+			loopCount = 0;
+			flags = 0;
 		}
 	};
 
diff --git a/engines/bladerunner/vqa_player.cpp b/engines/bladerunner/vqa_player.cpp
index 0a3c75d6a0d..e591269ff66 100644
--- a/engines/bladerunner/vqa_player.cpp
+++ b/engines/bladerunner/vqa_player.cpp
@@ -36,6 +36,8 @@
 namespace BladeRunner {
 
 bool VQAPlayer::open() {
+	close();
+
 	_s = _vm->getResourceStream(_vm->_enhancedEdition ? ("video/" + _name) : _name);
 	if (!_s) {
 		return false;
@@ -109,6 +111,7 @@ bool VQAPlayer::open() {
 }
 
 void VQAPlayer::close() {
+	_decoder.close();
 	_vm->_mixer->stopHandle(_soundHandle);
 	delete _s;
 	_s = nullptr;


Commit: 3e433ad94362008916bb7cb2bcdad360c6bb8a1e
    https://github.com/scummvm/scummvm/commit/3e433ad94362008916bb7cb2bcdad360c6bb8a1e
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-12-11T13:24:50+01:00

Commit Message:
BLADERUNNER: Garbage collect the unused pages

This avoids to accmulate all the pages from the previous scenes and
filling the memory on platform without much (like Wii).

This is implemented by adding a doubly circular linked list to the Page
structure.
Every page is moved at the end of the list when it is accessed and a
garbage collection process happens everytime a new page is allocated.
At this time, all pages (starting by the end) unused for 60 seconds are
discarded.

Changed paths:
    engines/bladerunner/slice_animations.cpp
    engines/bladerunner/slice_animations.h


diff --git a/engines/bladerunner/slice_animations.cpp b/engines/bladerunner/slice_animations.cpp
index e43a5399cd7..f20471e7da7 100644
--- a/engines/bladerunner/slice_animations.cpp
+++ b/engines/bladerunner/slice_animations.cpp
@@ -226,9 +226,9 @@ void *SliceAnimations::PageFile::loadPage(uint32 pageNumber) {
 
 	uint32 pageSize = _sliceAnimations->_pageSize;
 
-	// TODO: Retire oldest pages if we exceed some memory limit
-
 	void *data = malloc(pageSize);
+	assert(data);
+
 	_files[_pageOffsetsFileIdx[pageNumber]].seek(_pageOffsets[pageNumber], SEEK_SET);
 	uint32 r = _files[_pageOffsetsFileIdx[pageNumber]].read(data, pageSize);
 	assert(r == pageSize);
@@ -261,24 +261,110 @@ void *SliceAnimations::getFramePtr(uint32 animation, uint32 frame) {
 #endif // BLADERUNNER_ORIGINAL_BUGS
 
 	uint32 frameOffset = _animations[animation].offset + frame * _animations[animation].frameSize;
-	uint32 page        = frameOffset / _pageSize;
+	uint32 pageId      = frameOffset / _pageSize;
 	uint32 pageOffset  = frameOffset % _pageSize;
 
-	if (_pages[page]._data == nullptr) {                          // if not cached already
-		_pages[page]._data = _coreAnimPageFile.loadPage(page);    // look in COREANIM first
+	Page &page = _pages[pageId];
+
+	bool newPage = false;
+
+	if (page._data == nullptr) {                          // if not cached already
+		newPage = true;
+		page._data = _coreAnimPageFile.loadPage(pageId);    // look in COREANIM first
 
-		if (_pages[page]._data == nullptr) {                      // if not in COREAMIM
-			_pages[page]._data = _framesPageFile.loadPage(page);  // Look in CDFRAMES or HDFRAMES loaded data
+		if (page._data == nullptr) {                      // if not in COREAMIM
+			page._data = _framesPageFile.loadPage(pageId);  // Look in CDFRAMES or HDFRAMES loaded data
 
-			if (_pages[page]._data == nullptr) {
-				error("Unable to locate page %d for animation %d frame %d", page, animation, frame);
+			if (page._data == nullptr) {
+				error("Unable to locate page %d for animation %d frame %d", pageId, animation, frame);
 			}
 		}
 	}
 
-	_pages[page]._lastAccess = _vm->_time->currentSystem();
+	page._lastAccess = _vm->_time->currentSystem();
+	updatePagesList(page, newPage);
+
+	return (byte *)page._data + pageOffset;
+}
+
+void SliceAnimations::updatePagesList(Page &page, bool newPage) {
+	// We are already at the end, nothing to update
+	// Only cleanup old pages if any
+	if (_lastUsedPage == &page) {
+		if (newPage) {
+			cleanupOutdatedPages();
+		}
+		return;
+	}
+
+	// As our list is circular, it's either full nullptr or non nullptr
+	assert((page._prevPage == nullptr) == (page._nextPage == nullptr));
+
+	// If the page was in the doubly chained list, unhook it
+	if (page._prevPage) {
+		// On a fully generic circular linked list, if we were the only item
+		// we would have to clear _lastUsedPage.
+		// But if we are alone, we are at the end of the list and we were handled above
+		page._prevPage->_nextPage = page._nextPage;
+		page._nextPage->_prevPage = page._prevPage;
+		page._prevPage = nullptr;
+		page._nextPage = nullptr;
+	}
+
+	// Now hook it at the end
+	if (!_lastUsedPage) {
+		// Empty list
+		page._prevPage = &page;
+		page._nextPage = &page;
+	} else {
+		page._nextPage = _lastUsedPage->_nextPage;
+		page._prevPage = _lastUsedPage;
+		_lastUsedPage->_nextPage->_prevPage = &page;
+		_lastUsedPage->_nextPage = &page;
+	}
+	_lastUsedPage = &page;
 
-	return (byte *)_pages[page]._data + pageOffset;
+	// Don't cleanup everytime, only when allocating new pages
+	if (newPage) {
+		cleanupOutdatedPages();
+	}
+}
+
+void SliceAnimations::cleanupOutdatedPages() {
+	if (!_lastUsedPage) {
+		return;
+	}
+
+	// If we are too young, no cleanup to do
+	// The counter will wrap in approx. 49 days
+	if (_lastUsedPage->_lastAccess < 60000) {
+		return;
+	}
+
+	// Keep all pages used in the last 60s
+	uint32 deadline = _lastUsedPage->_lastAccess - 60000;
+
+	// _lastUsedPage->_nextNode is the oldest page in the list
+	Page *page = _lastUsedPage->_nextPage;
+	while(page != _lastUsedPage) {
+		if (page->_lastAccess >= deadline) {
+			break;
+		}
+		Page *next = page->_nextPage;
+
+		// As we delete from the start of the list,
+		// the previous page is always the end of the list
+		_lastUsedPage->_nextPage = next;
+		next->_prevPage = _lastUsedPage;
+
+		free(page->_data);
+		page->_data = nullptr;
+		page->_lastAccess = 0;
+		page->_prevPage = nullptr;
+		page->_nextPage = nullptr;
+
+		page = next;
+	}
 }
 
 Vector3 SliceAnimations::getPositionChange(int animation) const {
diff --git a/engines/bladerunner/slice_animations.h b/engines/bladerunner/slice_animations.h
index a0fd8091a0c..82bb022628b 100644
--- a/engines/bladerunner/slice_animations.h
+++ b/engines/bladerunner/slice_animations.h
@@ -58,8 +58,11 @@ class SliceAnimations {
 	struct Page {
 		void   *_data;
 		uint32 _lastAccess;
+		// Use a doubly linked list to sort pages by access time
+		Page   *_prevPage;
+		Page   *_nextPage;
 
-		Page() : _data(nullptr), _lastAccess(0) {}
+		Page() : _data(nullptr), _lastAccess(0), _prevPage(nullptr), _nextPage(nullptr) {}
 	};
 
 	struct PageFile {
@@ -86,10 +89,14 @@ class SliceAnimations {
 	Common::Array<Palette>      _palettes;
 	Common::Array<Animation>    _animations;
 	Common::Array<Page>         _pages;
+	Page                       *_lastUsedPage;
 
 	PageFile _coreAnimPageFile;
 	PageFile _framesPageFile;
 
+	void updatePagesList(Page &page, bool newPage);
+	void cleanupOutdatedPages();
+
 public:
 	SliceAnimations(BladeRunnerEngine *vm)
 		: _vm(vm)
@@ -98,7 +105,8 @@ public:
 		, _timestamp(0)
 		, _pageSize(0)
 		, _pageCount(0)
-		, _paletteCount(0) {}
+		, _paletteCount(0)
+		, _lastUsedPage(nullptr) {}
 	~SliceAnimations();
 
 	bool open(const Common::String &name);




More information about the Scummvm-git-logs mailing list