[Scummvm-git-logs] scummvm master -> 48a1dfe68e1fc1cf23986a45e91cb7c303223e34

criezy noreply at scummvm.org
Wed Jun 15 01:22:28 UTC 2022


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:
01dd8d5abf AGS: Clean BufferedStream code
56154c8065 AGS: Made BufferedStream work as expected in writing mode
912071c893 AGS: Fixed game not closing while window is minimized
48a1dfe68e AGS: Fixed renderer error if room backgrounds are of different size


Commit: 01dd8d5abff3142a6d6af58c31468d9caab0cb62
    https://github.com/scummvm/scummvm/commit/01dd8d5abff3142a6d6af58c31468d9caab0cb62
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-15T02:22:03+01:00

Commit Message:
AGS: Clean BufferedStream code

Part of upstream 82a928cb07970acc5918905c5836c39ce11b7f32
(Tests: added BufferedStream tests)

This commit has no functional change but allows to keep the code
in ScummVM close to upstream code.

Changed paths:
    engines/ags/shared/util/buffered_stream.cpp
    engines/ags/shared/util/buffered_stream.h
    engines/ags/shared/util/file.cpp


diff --git a/engines/ags/shared/util/buffered_stream.cpp b/engines/ags/shared/util/buffered_stream.cpp
index 924a8c3feee..481308f4d52 100644
--- a/engines/ags/shared/util/buffered_stream.cpp
+++ b/engines/ags/shared/util/buffered_stream.cpp
@@ -29,13 +29,13 @@ namespace AGS3 {
 namespace AGS {
 namespace Shared {
 
+//-----------------------------------------------------------------------------
+// BufferedStream
+//-----------------------------------------------------------------------------
+
 BufferedStream::BufferedStream(const String &file_name, FileOpenMode open_mode, FileWorkMode work_mode, DataEndianess stream_endianess)
-	: FileStream(file_name, open_mode, work_mode, stream_endianess), _buffer(BufferStreamSize), _bufferPosition(0), _position(0) {
+	: FileStream(file_name, open_mode, work_mode, stream_endianess) {
 	if (IsValid()) {
-		if (FileStream::Seek(0, kSeekEnd) == false)
-			error("Error determining stream end.");
-
-		_end = -1;
 		if (FileStream::Seek(0, kSeekEnd)) {
 			_start = 0;
 			_end = FileStream::GetPosition();
@@ -48,15 +48,13 @@ BufferedStream::BufferedStream(const String &file_name, FileOpenMode open_mode,
 			error("Error determining stream end.");
 		}
 	}
-
-	_buffer.resize(0);
 }
 
 void BufferedStream::FillBufferFromPosition(soff_t position) {
 	FileStream::Seek(position, kSeekBegin);
 
-	_buffer.resize(BufferStreamSize);
-	auto sz = FileStream::Read(_buffer.data(), BufferStreamSize);
+	_buffer.resize(BufferSize);
+	auto sz = FileStream::Read(_buffer.data(), BufferSize);
 	_buffer.resize(sz);
 
 	_bufferPosition = position;
@@ -135,6 +133,10 @@ bool BufferedStream::Seek(soff_t offset, StreamSeek origin) {
 	return _position == want_pos;
 }
 
+//-----------------------------------------------------------------------------
+// BufferedSectionStream
+//-----------------------------------------------------------------------------
+
 BufferedSectionStream::BufferedSectionStream(const String &file_name, soff_t start_pos, soff_t end_pos,
 	FileOpenMode open_mode, FileWorkMode work_mode, DataEndianess stream_endianess)
 	: BufferedStream(file_name, open_mode, work_mode, stream_endianess) {
diff --git a/engines/ags/shared/util/buffered_stream.h b/engines/ags/shared/util/buffered_stream.h
index 59058b35af6..37ba2082171 100644
--- a/engines/ags/shared/util/buffered_stream.h
+++ b/engines/ags/shared/util/buffered_stream.h
@@ -19,6 +19,13 @@
  *
  */
 
+// BufferedStream represents a buffered file stream; uses memory buffer
+// during read and write operations to limit number reads and writes on disk
+// and thus improve i/o perfomance.
+//
+// BufferedSectionStream is a subclass stream that limits reading by an
+// arbitrary offset range.
+
 #ifndef AGS_SHARED_UTIL_BUFFEREDSTREAM_H
 #define AGS_SHARED_UTIL_BUFFEREDSTREAM_H
 
@@ -30,12 +37,11 @@ namespace AGS3 {
 namespace AGS {
 namespace Shared {
 
-// Needs tuning depending on the platform.
-const auto BufferStreamSize = 8 * 1024;
-
 class BufferedStream : public FileStream {
 public:
-	// Represents an open _buffered_ file object
+	// Needs tuning depending on the platform.
+	static const size_t BufferSize = 1024u * 8;
+
 	// The constructor may raise std::runtime_error if
 	// - there is an issue opening the file (does not exist, locked, permissions, etc)
 	// - the open mode could not be determined
@@ -54,15 +60,15 @@ public:
 	bool    Seek(soff_t offset, StreamSeek origin) override;
 
 protected:
-	soff_t _start;
-	soff_t _end;
+	soff_t _start = 0;
+	soff_t _end = -1;
 
 private:
 	void FillBufferFromPosition(soff_t position);
 
-	soff_t _position;
-	soff_t _bufferPosition;
-	std::vector<char> _buffer;
+	soff_t _position = 0;
+	soff_t _bufferPosition = 0;
+	std::vector<uint8_t> _buffer;
 };
 
 
diff --git a/engines/ags/shared/util/file.cpp b/engines/ags/shared/util/file.cpp
index 3b2bf8d0d17..b7c417f8eb3 100644
--- a/engines/ags/shared/util/file.cpp
+++ b/engines/ags/shared/util/file.cpp
@@ -265,7 +265,7 @@ Stream *File::OpenFileCI(const String &file_name, FileOpenMode open_mode, FileWo
 }
 
 Stream *File::OpenFile(const String &filename, soff_t start_off, soff_t end_off) {
-	FileStream *fs = new BufferedSectionStream(filename, start_off, end_off, kFile_Open, kFile_Read);
+	Stream *fs = new BufferedSectionStream(filename, start_off, end_off, kFile_Open, kFile_Read);
 	if (fs != nullptr && !fs->IsValid()) {
 		delete fs;
 		return nullptr;


Commit: 56154c8065bc52cfdcdbf78817722711c8cbc10c
    https://github.com/scummvm/scummvm/commit/56154c8065bc52cfdcdbf78817722711c8cbc10c
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-15T02:22:03+01:00

Commit Message:
AGS: Made BufferedStream work as expected in writing mode

>From upstream 97eebe7b91010ad9e507de5c8b9c2515bb8d6cfd

Changed paths:
    engines/ags/shared/util/buffered_stream.cpp
    engines/ags/shared/util/buffered_stream.h
    engines/ags/shared/util/file.cpp
    engines/ags/shared/util/file_stream.h


diff --git a/engines/ags/shared/util/buffered_stream.cpp b/engines/ags/shared/util/buffered_stream.cpp
index 481308f4d52..398302d3309 100644
--- a/engines/ags/shared/util/buffered_stream.cpp
+++ b/engines/ags/shared/util/buffered_stream.cpp
@@ -50,47 +50,75 @@ BufferedStream::BufferedStream(const String &file_name, FileOpenMode open_mode,
 	}
 }
 
+BufferedStream::~BufferedStream() {
+	Close();
+}
+
 void BufferedStream::FillBufferFromPosition(soff_t position) {
 	FileStream::Seek(position, kSeekBegin);
-
 	_buffer.resize(BufferSize);
 	auto sz = FileStream::Read(_buffer.data(), BufferSize);
 	_buffer.resize(sz);
-
 	_bufferPosition = position;
 }
 
+void BufferedStream::FlushBuffer(soff_t position) {
+	size_t sz = _buffer.size() > 0 ? FileStream::Write(_buffer.data(), _buffer.size()) : 0u;
+	_buffer.clear(); // will start from the clean buffer next time
+	_bufferPosition += sz;
+	if (position != _bufferPosition) {
+		FileStream::Seek(position, kSeekBegin);
+		_bufferPosition = position;
+	}
+}
+
 bool BufferedStream::EOS() const {
 	return _position == _end;
 }
 
+soff_t BufferedStream::GetLength() const {
+	return _end - _start;
+}
+
 soff_t BufferedStream::GetPosition() const {
-	return _position;
+	return _position - _start;
 }
 
-size_t BufferedStream::Read(void *toBuffer, size_t toSize) {
-	auto to = static_cast<char *>(toBuffer);
+void BufferedStream::Close() {
+	if (GetWorkMode() == kFile_Write)
+		FlushBuffer(_position);
+	FileStream::Close();
+}
 
-	while (toSize > 0) {
+bool BufferedStream::Flush() {
+	if (GetWorkMode() == kFile_Write)
+		FlushBuffer(_position);
+	return FileStream::Flush();
+}
+
+size_t BufferedStream::Read(void *buffer, size_t size) {
+	auto to = static_cast<uint8_t*>(buffer);
+
+	while(size > 0) {
 		if (_position < _bufferPosition || _position >= _bufferPosition + _buffer.size()) {
 			FillBufferFromPosition(_position);
 		}
 		if (_buffer.empty()) { break; } // reached EOS
-		assert(_position >= _bufferPosition && _position < _bufferPosition + _buffer.size());  // sanity check only, should be checked by above.
+		assert(_position >= _bufferPosition && _position < _bufferPosition + _buffer.size());
 
 		soff_t bufferOffset = _position - _bufferPosition;
 		assert(bufferOffset >= 0);
 		size_t bytesLeft = _buffer.size() - (size_t)bufferOffset;
-		size_t chunkSize = MIN<size_t>(bytesLeft, toSize);
+		size_t chunkSize = MIN<size_t>(bytesLeft, size);
 
 		memcpy(to, _buffer.data() + bufferOffset, chunkSize);
 
 		to += chunkSize;
 		_position += chunkSize;
-		toSize -= chunkSize;
+		size -= chunkSize;
 	}
 
-	return to - (char *)toBuffer;
+	return to - static_cast<uint8_t*>(buffer);
 }
 
 int32_t BufferedStream::ReadByte() {
@@ -103,12 +131,24 @@ int32_t BufferedStream::ReadByte() {
 }
 
 size_t BufferedStream::Write(const void *buffer, size_t size) {
-	FileStream::Seek(_position, kSeekBegin);
-	auto sz = FileStream::Write(buffer, size);
-	if (_position == _end)
-		_end += sz;
-	_position += sz;
-	return sz;
+	const uint8_t *from = static_cast<const uint8_t*>(buffer);
+	while (size > 0) {
+		if (_position < _bufferPosition || _position >= _bufferPosition + BufferSize) {
+			FlushBuffer(_position);
+		}
+		size_t pos_in_buff = static_cast<size_t>(_position - _bufferPosition);
+		size_t chunk_sz = std::min(size, BufferSize - pos_in_buff);
+		if (_buffer.size() < pos_in_buff + chunk_sz)
+			_buffer.resize(pos_in_buff + chunk_sz);
+		memcpy(_buffer.data() + pos_in_buff, from, chunk_sz);
+		_position += chunk_sz;
+		from += chunk_sz;
+		size -= chunk_sz;
+	}
+
+	_end = std::max(_end, _position);
+	return from - static_cast<const uint8_t*>(buffer);
+
 }
 
 int32_t BufferedStream::WriteByte(uint8_t val) {
@@ -147,13 +187,6 @@ BufferedSectionStream::BufferedSectionStream(const String &file_name, soff_t sta
 	Seek(0, kSeekBegin);
 }
 
-soff_t BufferedSectionStream::GetPosition() const {
-	return BufferedStream::GetPosition() - _start;
-}
-
-soff_t BufferedSectionStream::GetLength() const {
-	return _end - _start;
-}
 
 } // namespace Shared
 } // namespace AGS
diff --git a/engines/ags/shared/util/buffered_stream.h b/engines/ags/shared/util/buffered_stream.h
index 37ba2082171..4253d12e045 100644
--- a/engines/ags/shared/util/buffered_stream.h
+++ b/engines/ags/shared/util/buffered_stream.h
@@ -48,9 +48,17 @@ public:
 	// - could not determine the length of the stream
 	// It is recommended to use File::OpenFile to safely construct this object.
 	BufferedStream(const String &file_name, FileOpenMode open_mode, FileWorkMode work_mode, DataEndianess stream_endianess = kLittleEndian);
+	~BufferedStream();
 
-	bool    EOS() const override; ///< Is end of stream
-	soff_t  GetPosition() const override; ///< Current position (if known)
+	// Is end of stream
+	bool    EOS() const override;
+	// Total length of stream (if known)
+	soff_t  GetLength() const override;
+	// Current position (if known)
+	soff_t  GetPosition() const override;
+
+	void    Close() override;
+	bool    Flush() override;
 
 	size_t  Read(void *buffer, size_t size) override;
 	int32_t ReadByte() override;
@@ -60,14 +68,17 @@ public:
 	bool    Seek(soff_t offset, StreamSeek origin) override;
 
 protected:
-	soff_t _start = 0;
-	soff_t _end = -1;
+	soff_t _start = 0; // valid section starting offset
+	soff_t _end = -1; // valid section ending offset
 
 private:
+	// Reads a chunk of file into the buffer, starting from the given offset
 	void FillBufferFromPosition(soff_t position);
+	// Writes a buffer into the file, and reposition to the new offset
+	void FlushBuffer(soff_t position);
 
-	soff_t _position = 0;
-	soff_t _bufferPosition = 0;
+	soff_t _position = 0; // absolute read/write offset
+	soff_t _bufferPosition = 0; // buffer's location relative to file
 	std::vector<uint8_t> _buffer;
 };
 
@@ -77,9 +88,6 @@ class BufferedSectionStream : public BufferedStream {
 public:
 	BufferedSectionStream(const String &file_name, soff_t start_pos, soff_t end_pos,
 		FileOpenMode open_mode, FileWorkMode work_mode, DataEndianess stream_endianess = kLittleEndian);
-
-	soff_t  GetPosition() const override;
-	soff_t  GetLength() const override;
 };
 
 } // namespace Shared
diff --git a/engines/ags/shared/util/file.cpp b/engines/ags/shared/util/file.cpp
index b7c417f8eb3..e1c90506bdc 100644
--- a/engines/ags/shared/util/file.cpp
+++ b/engines/ags/shared/util/file.cpp
@@ -145,12 +145,9 @@ String File::GetCMode(FileOpenMode open_mode, FileWorkMode work_mode) {
 }
 
 Stream *File::OpenFile(const String &filename, FileOpenMode open_mode, FileWorkMode work_mode) {
-	FileStream *fs = nullptr;
+	Stream *fs = nullptr;
 	//  try {
-	if (work_mode == kFile_Read) // NOTE: BufferedStream does not work correctly in the write mode
-		fs = new BufferedStream(filename, open_mode, work_mode);
-	else
-		fs = new FileStream(filename, open_mode, work_mode);
+	fs = new BufferedStream(filename, open_mode, work_mode);
 	if (fs != nullptr && !fs->IsValid()) {
 		delete fs;
 		fs = nullptr;
diff --git a/engines/ags/shared/util/file_stream.h b/engines/ags/shared/util/file_stream.h
index 9ca2dd04eae..c2c72ec8993 100644
--- a/engines/ags/shared/util/file_stream.h
+++ b/engines/ags/shared/util/file_stream.h
@@ -41,6 +41,8 @@ public:
 			   DataEndianess stream_endianess = kLittleEndian);
 	~FileStream() override;
 
+	FileWorkMode GetWorkMode() const { return _workMode; }
+
 	bool HasErrors() const override;
 	void Close() override;
 	bool Flush() override;


Commit: 912071c89316f29a993def5fdf5d641b3752959f
    https://github.com/scummvm/scummvm/commit/912071c89316f29a993def5fdf5d641b3752959f
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-15T02:22:03+01:00

Commit Message:
AGS: Fixed game not closing while window is minimized

>From upstream 12354d791e7ad2c358c3d99e4678fd4cda6bb75e

In ScummVM this currently does not matter since _on_switchin_callback
and _on_switchout_callback are not called when the window gains or
loses the focus.

Changed paths:
    engines/ags/engine/ac/timer.cpp


diff --git a/engines/ags/engine/ac/timer.cpp b/engines/ags/engine/ac/timer.cpp
index 281d119ced4..834a0596dfa 100644
--- a/engines/ags/engine/ac/timer.cpp
+++ b/engines/ags/engine/ac/timer.cpp
@@ -63,7 +63,7 @@ void WaitForNextFrame() {
 	if (frameDuration <= std::chrono::milliseconds::zero()) {
 		_G(next_frame_timestamp) = now;
 		// suspend while the game is being switched out
-		while (_G(game_update_suspend)) {
+		while (_G(game_update_suspend) && !_G(want_exit) && !_G(abort_engine)) {
 			sys_evt_process_pending();
 			_G(platform)->YieldCPU();
 		}
@@ -83,7 +83,7 @@ void WaitForNextFrame() {
 	_G(next_frame_timestamp) += frameDuration;
 
 	// suspend while the game is being switched out
-	while (_G(game_update_suspend)) {
+	while (_G(game_update_suspend) && !_G(want_exit) && !_G(abort_engine)) {
 		sys_evt_process_pending();
 		_G(platform)->YieldCPU();
 	}


Commit: 48a1dfe68e1fc1cf23986a45e91cb7c303223e34
    https://github.com/scummvm/scummvm/commit/48a1dfe68e1fc1cf23986a45e91cb7c303223e34
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-15T02:22:03+01:00

Commit Message:
AGS: Fixed renderer error if room backgrounds are of different size

>From upstream 660857d193b9bcac3a7bb1bba70be8149b1e2d7e

Changed paths:
    engines/ags/engine/ac/draw.cpp


diff --git a/engines/ags/engine/ac/draw.cpp b/engines/ags/engine/ac/draw.cpp
index 7985020eb4a..830e6e22675 100644
--- a/engines/ags/engine/ac/draw.cpp
+++ b/engines/ags/engine/ac/draw.cpp
@@ -1697,12 +1697,11 @@ void prepare_room_sprites() {
 	// Background sprite is required for the non-software renderers always,
 	// and for software renderer in case there are overlapping viewports.
 	// Note that software DDB is just a tiny wrapper around bitmap, so overhead is negligible.
-	if (_G(roomBackgroundBmp) == nullptr) {
+	if (_G(current_background_is_dirty) || !_G(roomBackgroundBmp)) {
 		update_polled_stuff_if_runtime();
-		_G(roomBackgroundBmp) = _G(gfxDriver)->CreateDDBFromBitmap(_GP(thisroom).BgFrames[_GP(play).bg_frame].Graphic.get(), false, true);
-	} else if (_G(current_background_is_dirty)) {
-		update_polled_stuff_if_runtime();
-		_G(gfxDriver)->UpdateDDBFromBitmap(_G(roomBackgroundBmp), _GP(thisroom).BgFrames[_GP(play).bg_frame].Graphic.get(), false);
+		_G(roomBackgroundBmp) =
+			recycle_ddb_bitmap(_G(roomBackgroundBmp), _GP(thisroom).BgFrames[_GP(play).bg_frame].Graphic.get(), false, true);
+
 	}
 	if (_G(gfxDriver)->RequiresFullRedrawEachFrame()) {
 		if (_G(current_background_is_dirty) || _G(walkBehindsCachedForBgNum) != _GP(play).bg_frame) {




More information about the Scummvm-git-logs mailing list