[Scummvm-git-logs] scummvm branch-2-9 -> 7577f64b8dbfff89c96f507c68a39908112ea911
lephilousophe
noreply at scummvm.org
Sat Dec 28 12:56:18 UTC 2024
This automated email contains information about 4 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
41397ed2ce BLADERUNNER: Don't allocate the thumbnail before needing it
d75c81f297 BLADERUNNER: Don't read pixels using READ_UINT32
b70b3763ce BLADERUNNER: Avoid memory leak in overlays
7577f64b8d BLADERUNNER: Garbage collect the unused pages
Commit: 41397ed2ce602e88eb74dbdbe1e6d7f7f32796f7
https://github.com/scummvm/scummvm/commit/41397ed2ce602e88eb74dbdbe1e6d7f7f32796f7
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-12-28T13:55:38+01:00
Commit Message:
BLADERUNNER: Don't allocate the thumbnail before needing it
Graphics::loadThumbnail will overwrite it and it creates a memory leak.
Changed paths:
engines/bladerunner/savefile.cpp
diff --git a/engines/bladerunner/savefile.cpp b/engines/bladerunner/savefile.cpp
index 04d40d679b7..5a10db6b643 100644
--- a/engines/bladerunner/savefile.cpp
+++ b/engines/bladerunner/savefile.cpp
@@ -147,8 +147,6 @@ bool SaveFileManager::readHeader(Common::SeekableReadStream &in, SaveFileHeader
}
if (!skipThumbnail) {
- header._thumbnail = new Graphics::Surface(); // freed by ScummVM's smartptr
-
s.skip(4); //skip size;
if (header._version >= 4) {
@@ -160,6 +158,7 @@ bool SaveFileManager::readHeader(Common::SeekableReadStream &in, SaveFileHeader
thumbnailData[i] = s.readUint16LE() | alphamask; // We set all pixels to non-transparency
}
+ header._thumbnail = new Graphics::Surface(); // freed by ScummVM's smartptr
header._thumbnail->init(80, 60, 160, thumbnailData, gameDataPixelFormat());
}
Commit: d75c81f2974cfd5f3f915dd42b384e3d22fbe9a8
https://github.com/scummvm/scummvm/commit/d75c81f2974cfd5f3f915dd42b384e3d22fbe9a8
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-12-28T13:55:38+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: b70b3763ce7b54bb5fcdb0319be6fdc2370b8f00
https://github.com/scummvm/scummvm/commit/b70b3763ce7b54bb5fcdb0319be6fdc2370b8f00
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-12-28T13:55:38+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: 7577f64b8dbfff89c96f507c68a39908112ea911
https://github.com/scummvm/scummvm/commit/7577f64b8dbfff89c96f507c68a39908112ea911
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-12-28T13:55:38+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