[Scummvm-git-logs] scummvm branch-2-6 -> f4bbbe5786b61ee5a2df99d98eb42188a47d386c
sev-
noreply at scummvm.org
Sat Jul 2 19:54:39 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:
0d37dbe93e GUI: Move icon download callbacks to the DialogState
89853f9089 GUI: Add mutex for the GuiManager icon set
9dd4da4e4d GUI: Change the mechanism to update the grid launcher after upding the icons
f4bbbe5786 GUI: Improve thread-safety for icons set access
Commit: 0d37dbe93e908b9020a4d150c12ef1bc073e45bc
https://github.com/scummvm/scummvm/commit/0d37dbe93e908b9020a4d150c12ef1bc073e45bc
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-07-02T21:47:08+02:00
Commit Message:
GUI: Move icon download callbacks to the DialogState
This fixes a crash when hiding the DownloadIconsDialog while
downloading an icons pack.
Changed paths:
gui/downloadiconsdialog.cpp
gui/downloadiconsdialog.h
diff --git a/gui/downloadiconsdialog.cpp b/gui/downloadiconsdialog.cpp
index 300e9d27c04..849f56a8dab 100644
--- a/gui/downloadiconsdialog.cpp
+++ b/gui/downloadiconsdialog.cpp
@@ -56,8 +56,102 @@ struct DialogState {
uint32 startTime;
DialogState() { state = kDownloadStateNone; downloadedSize = totalSize = totalFiles = startTime = 0; dialog = nullptr; }
+
+ void downloadList();
+ void proceedDownload();
+
+ void downloadListCallback(Networking::DataResponse response);
+ void downloadFileCallback(Networking::DataResponse response);
+ void errorCallback(Networking::ErrorResponse error);
+
+private:
+ bool takeOneFile();
} static *g_state;
+
+void DialogState::downloadList() {
+ Networking::SessionRequest *rq = session.get("https://downloads.scummvm.org/frs/icons/LIST", "",
+ new Common::Callback<DialogState, Networking::DataResponse>(this, &DialogState::downloadListCallback),
+ new Common::Callback<DialogState, Networking::ErrorResponse>(this, &DialogState::errorCallback),
+ true);
+
+ rq->start();
+}
+
+void DialogState::proceedDownload() {
+ startTime = g_system->getMillis();
+ takeOneFile();
+}
+
+bool DialogState::takeOneFile() {
+ auto f = fileHash.begin();
+ if (f == fileHash.end())
+ return false;
+
+ Common::String fname = f->_key;
+ fileHash.erase(fname);
+
+ Common::String url = Common::String::format("https://downloads.scummvm.org/frs/icons/%s", fname.c_str());
+ Common::String localFile = normalizePath(ConfMan.get("iconspath") + "/" + fname, '/');
+
+ Networking::SessionRequest *rq = session.get(url, localFile,
+ new Common::Callback<DialogState, Networking::DataResponse>(this, &DialogState::downloadFileCallback),
+ new Common::Callback<DialogState, Networking::ErrorResponse>(this, &DialogState::errorCallback));
+
+ rq->start();
+ return true;
+}
+
+void DialogState::downloadListCallback(Networking::DataResponse r) {
+ Networking::SessionFileResponse *response = static_cast<Networking::SessionFileResponse *>(r.value);
+ Common::MemoryReadStream stream(response->buffer, response->len);
+
+ int nline = 0;
+ while (!stream.eos()) {
+ Common::String s = stream.readString('\n');
+ nline++;
+ if (s.empty())
+ continue;
+
+ size_t pos = s.findFirstOf(',');
+ if (pos == Common::String::npos) {
+ warning("DownloadIconsDialog: wrong string format at line %d: <%s>", nline, s.c_str());
+ continue;
+ }
+
+ fileHash.setVal(s.substr(0, pos), atol(s.substr(pos + 1).c_str()));
+ }
+
+ state = kDownloadStateListDownloaded;
+ if (dialog)
+ dialog->sendCommand(kListDownloadFinishedCmd, 0);
+}
+
+void DialogState::downloadFileCallback(Networking::DataResponse r) {
+ Networking::SessionFileResponse *response = static_cast<Networking::SessionFileResponse *>(r.value);
+
+ downloadedSize += response->len;
+ if (response->eos) {
+ if (!takeOneFile()) {
+ state = kDownloadComplete;
+ g_gui.initIconsSet();
+ if (dialog)
+ dialog->sendCommand(kDownloadEndedCmd, 0);
+ return;
+ }
+ }
+
+ if (dialog)
+ dialog->sendCommand(kDownloadProgressCmd, 0);
+}
+
+void DialogState::errorCallback(Networking::ErrorResponse error) {
+ Common::U32String message = Common::U32String::format(_("ERROR %d: %s"), error.httpResponseCode, error.response.c_str());
+
+ if (dialog)
+ dialog->setError(message);
+}
+
static uint32 getDownloadingProgress() {
if (!g_state || g_state->totalSize == 0)
return 0;
@@ -101,7 +195,7 @@ DownloadIconsDialog::DownloadIconsDialog() :
setState(kDownloadStateList);
refreshWidgets();
- downloadList();
+ g_state->downloadList();
} else {
g_state->dialog = this;
@@ -210,7 +304,6 @@ void DownloadIconsDialog::handleCommand(CommandSender *sender, uint32 cmd, uint3
break;
case kDownloadEndedCmd:
setState(kDownloadComplete);
- g_gui.initIconsSet();
setResult(1); // Need tell the options to refresh launcher
break;
case kListDownloadFinishedCmd:
@@ -219,7 +312,7 @@ void DownloadIconsDialog::handleCommand(CommandSender *sender, uint32 cmd, uint3
break;
case kDownloadProceedCmd:
setState(kDownloadStateDownloading);
- proceedDownload();
+ g_state->proceedDownload();
break;
default:
Dialog::handleCommand(sender, cmd, data);
@@ -269,34 +362,6 @@ void DownloadIconsDialog::refreshWidgets() {
_progressBar->setValue(progress);
}
-void DownloadIconsDialog::downloadListCallback(Networking::DataResponse r) {
- Networking::SessionFileResponse *response = static_cast<Networking::SessionFileResponse *>(r.value);
-
- Common::MemoryReadStream stream(response->buffer, response->len);
-
- int nline = 0;
-
- while (!stream.eos()) {
- Common::String s = stream.readString('\n');
-
- nline++;
-
- if (s.empty())
- continue;
-
- size_t pos = s.findFirstOf(',');
-
- if (pos == Common::String::npos) {
- warning("DownloadIconsDialog: wrong string format at line %d: <%s>", nline, s.c_str());
- continue;
- }
-
- g_state->fileHash.setVal(s.substr(0, pos), atol(s.substr(pos + 1).c_str()));
- }
-
- sendCommand(kListDownloadFinishedCmd, 0);
-}
-
void DownloadIconsDialog::setError(Common::U32String &msg) {
_errorText->setLabel(msg);
@@ -304,22 +369,6 @@ void DownloadIconsDialog::setError(Common::U32String &msg) {
_cancelButton->setCmd(kCleanupCmd);
}
-void DownloadIconsDialog::errorCallback(Networking::ErrorResponse error) {
- Common::U32String message = Common::U32String::format(_("ERROR %d: %s"), error.httpResponseCode, error.response.c_str());
-
- if (g_state->dialog)
- g_state->dialog->setError(message);
-}
-
-void DownloadIconsDialog::downloadList() {
- Networking::SessionRequest *rq = g_state->session.get("https://downloads.scummvm.org/frs/icons/LIST", "",
- new Common::Callback<DownloadIconsDialog, Networking::DataResponse>(this, &DownloadIconsDialog::downloadListCallback),
- new Common::Callback<DownloadIconsDialog, Networking::ErrorResponse>(this, &DownloadIconsDialog::errorCallback),
- true);
-
- rq->start();
-}
-
void DownloadIconsDialog::calculateList() {
if (!ConfMan.hasKey("iconspath")) {
Common::U32String str(_("ERROR: No icons path set"));
@@ -360,48 +409,5 @@ void DownloadIconsDialog::calculateList() {
setState(kDownloadStateListCalculated);
}
-bool DownloadIconsDialog::takeOneFile() {
- auto f = g_state->fileHash.begin();
-
- if (f == g_state->fileHash.end())
- return false;
-
- Common::String fname = f->_key;
-
- g_state->fileHash.erase(fname);
-
- Common::String url = Common::String::format("https://downloads.scummvm.org/frs/icons/%s", fname.c_str());
- Common::String localFile = normalizePath(ConfMan.get("iconspath") + "/" + fname, '/');
-
- Networking::SessionRequest *rq = g_state->session.get(url, localFile,
- new Common::Callback<DownloadIconsDialog, Networking::DataResponse>(this, &DownloadIconsDialog::downloadFileCallback),
- new Common::Callback<DownloadIconsDialog, Networking::ErrorResponse>(this, &DownloadIconsDialog::errorCallback));
-
- rq->start();
-
- return true;
-}
-
-void DownloadIconsDialog::downloadFileCallback(Networking::DataResponse r) {
- Networking::SessionFileResponse *response = static_cast<Networking::SessionFileResponse *>(r.value);
-
- g_state->downloadedSize += response->len;
-
- if (response->eos) {
- if (!takeOneFile()) {
- sendCommand(kDownloadEndedCmd, 0);
- return;
- }
- }
-
- sendCommand(kDownloadProgressCmd, 0);
-}
-
-void DownloadIconsDialog::proceedDownload() {
- g_state->startTime = g_system->getMillis();
-
- takeOneFile();
-}
-
} // End of namespace GUI
diff --git a/gui/downloadiconsdialog.h b/gui/downloadiconsdialog.h
index 1e717aeca07..d03193fcd91 100644
--- a/gui/downloadiconsdialog.h
+++ b/gui/downloadiconsdialog.h
@@ -73,18 +73,11 @@ public:
void handleTickle() override;
void reflowLayout() override;
- void downloadListCallback(Networking::DataResponse response);
- void downloadFileCallback(Networking::DataResponse response);
- void errorCallback(Networking::ErrorResponse error);
-
void setError(Common::U32String &msg);
private:
- void downloadList();
void calculateList();
- void proceedDownload();
void setState(IconProcessState state);
- bool takeOneFile();
};
} // End of namespace GUI
Commit: 89853f90894727e5801883e1f5b0b9170bf7ac0c
https://github.com/scummvm/scummvm/commit/89853f90894727e5801883e1f5b0b9170bf7ac0c
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-07-02T21:47:12+02:00
Commit Message:
GUI: Add mutex for the GuiManager icon set
GuiManager::initIconsSet is called from a callback of the iconset
download dialog, which runs in a separate thread. That means the
iconset can be accessed from two separate thread (the GUI thread,
and the download thread).
This was not an issue until the previous commit as closing the
download dialog while a download was ongoing would crash ScummVM.
But now that the crash is fixed, a race condition was possible.
Changed paths:
gui/gui-manager.cpp
gui/gui-manager.h
diff --git a/gui/gui-manager.cpp b/gui/gui-manager.cpp
index 534b7b4e625..33fffaab5b3 100644
--- a/gui/gui-manager.cpp
+++ b/gui/gui-manager.cpp
@@ -114,6 +114,7 @@ struct ArchiveMemberListBackComparator {
}
};
void GuiManager::initIconsSet() {
+ Common::StackLock lock(_iconsMutex);
Common::Archive *dat;
_iconsSet.clear();
diff --git a/gui/gui-manager.h b/gui/gui-manager.h
index 38d7865bc35..ce23ef8a02c 100644
--- a/gui/gui-manager.h
+++ b/gui/gui-manager.h
@@ -27,6 +27,7 @@
#include "common/stack.h"
#include "common/str.h"
#include "common/list.h"
+#include "common/mutex.h"
#include "gui/ThemeEngine.h"
#include "gui/widget.h"
@@ -91,7 +92,10 @@ public:
ThemeEval *xmlEval() { return _theme->getEvaluator(); }
- Common::SearchSet &getIconsSet() { return _iconsSet; }
+ Common::SearchSet &getIconsSet() {
+ Common::StackLock lock(_iconsMutex);
+ return _iconsSet;
+ }
int16 getGUIWidth() const { return _baseWidth; }
int16 getGUIHeight() const { return _baseHeight; }
@@ -162,6 +166,7 @@ protected:
int _topDialogLeftPadding;
int _topDialogRightPadding;
+ Common::Mutex _iconsMutex;
Common::SearchSet _iconsSet;
// position and time of last mouse click (used to detect double clicks)
Commit: 9dd4da4e4d52aaa9884008185570afb8ecb4ed87
https://github.com/scummvm/scummvm/commit/9dd4da4e4d52aaa9884008185570afb8ecb4ed87
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-07-02T21:47:12+02:00
Commit Message:
GUI: Change the mechanism to update the grid launcher after upding the icons
The icons download dialog was triggering the grid launcher update after
icons had been downloaded. But that means no update was done if the
dialog had been closed during the download.
Now the GUIManager triggers the update. This fixes the missing update
when hiding the download dialog while downloading an icon set.
Changed paths:
gui/downloadiconsdialog.cpp
gui/gui-manager.cpp
gui/gui-manager.h
gui/launcher.cpp
gui/options.cpp
diff --git a/gui/downloadiconsdialog.cpp b/gui/downloadiconsdialog.cpp
index 849f56a8dab..5c07714ed00 100644
--- a/gui/downloadiconsdialog.cpp
+++ b/gui/downloadiconsdialog.cpp
@@ -304,7 +304,6 @@ void DownloadIconsDialog::handleCommand(CommandSender *sender, uint32 cmd, uint3
break;
case kDownloadEndedCmd:
setState(kDownloadComplete);
- setResult(1); // Need tell the options to refresh launcher
break;
case kListDownloadFinishedCmd:
setState(kDownloadStateListDownloaded);
diff --git a/gui/gui-manager.cpp b/gui/gui-manager.cpp
index 33fffaab5b3..0ed7abdfdb4 100644
--- a/gui/gui-manager.cpp
+++ b/gui/gui-manager.cpp
@@ -59,7 +59,7 @@ enum {
};
// Constructor
-GuiManager::GuiManager() : _redrawStatus(kRedrawDisabled), _stateIsSaved(false),
+GuiManager::GuiManager() : CommandSender(nullptr), _redrawStatus(kRedrawDisabled), _stateIsSaved(false),
_cursorAnimateCounter(0), _cursorAnimateTimer(0) {
_theme = nullptr;
_useStdCursor = false;
@@ -73,6 +73,8 @@ GuiManager::GuiManager() : _redrawStatus(kRedrawDisabled), _stateIsSaved(false),
_useRTL = false;
+ _iconsSetChanged = false;
+
_topDialogLeftPadding = 0;
_topDialogRightPadding = 0;
@@ -170,6 +172,7 @@ void GuiManager::initIconsSet() {
}
_iconsSet.add(fname, dat);
+ _iconsSetChanged = true;
debug(2, "GUI: Loaded icon file: %s", fname);
}
@@ -506,6 +509,18 @@ void GuiManager::runLoop() {
processEvent(event, activeDialog);
}
+ // If iconsSet was modified, notify dialogs so that they can be updated if needed
+ _iconsMutex.lock();
+ bool iconsChanged = _iconsSetChanged;
+ _iconsSetChanged = false;
+ _iconsMutex.unlock();
+ if (iconsChanged) {
+ for (DialogStack::size_type i = 0; i < _dialogStack.size(); ++i) {
+ setTarget(_dialogStack[i]);
+ sendCommand(kIconsSetLoadedCmd, 0);
+ }
+ }
+
// Delete GuiObject that have been added to the trash for a delayed deletion
Common::List<GuiObjectTrashItem>::iterator it = _guiObjectTrash.begin();
while (it != _guiObjectTrash.end()) {
diff --git a/gui/gui-manager.h b/gui/gui-manager.h
index ce23ef8a02c..6f3e2081021 100644
--- a/gui/gui-manager.h
+++ b/gui/gui-manager.h
@@ -45,6 +45,10 @@ namespace Common {
namespace GUI {
+enum {
+ kIconsSetLoadedCmd = 'icns'
+};
+
class Dialog;
class ThemeEval;
class GuiObject;
@@ -65,7 +69,7 @@ typedef Common::FixedStack<Dialog *> DialogStack;
/**
* GUI manager singleton.
*/
-class GuiManager : public Common::Singleton<GuiManager> {
+class GuiManager : public Common::Singleton<GuiManager>, public CommandSender {
friend class Dialog;
friend class Common::Singleton<SingletonBaseType>;
GuiManager();
@@ -168,6 +172,7 @@ protected:
Common::Mutex _iconsMutex;
Common::SearchSet _iconsSet;
+ bool _iconsSetChanged;
// position and time of last mouse click (used to detect double clicks)
struct MousePos {
diff --git a/gui/launcher.cpp b/gui/launcher.cpp
index 5b3b0231772..b63991d2757 100644
--- a/gui/launcher.cpp
+++ b/gui/launcher.cpp
@@ -1477,6 +1477,9 @@ void LauncherGrid::handleCommand(CommandSender *sender, uint32 cmd, uint32 data)
ConfMan.flushToDisk();
reflowLayout();
break;
+ case kIconsSetLoadedCmd:
+ rebuild();
+ break;
default:
LauncherDialog::handleCommand(sender, cmd, data);
}
diff --git a/gui/options.cpp b/gui/options.cpp
index 2a9a7372268..6fd512e32d8 100644
--- a/gui/options.cpp
+++ b/gui/options.cpp
@@ -2991,12 +2991,7 @@ void GlobalOptionsDialog::handleCommand(CommandSender *sender, uint32 cmd, uint3
#ifdef USE_LIBCURL
case kUpdateIconsCmd: {
DownloadIconsDialog dia;
-
- if (dia.runModal() > 0) {
- if (_launcher && _launcher->getType() == kLauncherDisplayGrid)
- _launcher->rebuild();
- }
-
+ dia.runModal();
break;
}
#endif
Commit: f4bbbe5786b61ee5a2df99d98eb42188a47d386c
https://github.com/scummvm/scummvm/commit/f4bbbe5786b61ee5a2df99d98eb42188a47d386c
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-07-02T21:47:12+02:00
Commit Message:
GUI: Improve thread-safety for icons set access
The previous commit was not sufficient as getIconsSet() returns a
reference to the SearchSet and it could then be used after the
mutex had been unlocked and while it was being modified in
initIconsSet() called in another thread.
Changed paths:
gui/gui-manager.h
gui/launcher.cpp
gui/widgets/grid.cpp
diff --git a/gui/gui-manager.h b/gui/gui-manager.h
index 6f3e2081021..3127788b3e0 100644
--- a/gui/gui-manager.h
+++ b/gui/gui-manager.h
@@ -96,10 +96,9 @@ public:
ThemeEval *xmlEval() { return _theme->getEvaluator(); }
- Common::SearchSet &getIconsSet() {
- Common::StackLock lock(_iconsMutex);
- return _iconsSet;
- }
+ void lockIconsSet() { _iconsMutex.lock(); }
+ void unlockIconsSet() { _iconsMutex.unlock(); }
+ Common::SearchSet &getIconsSet() { return _iconsSet; }
int16 getGUIWidth() const { return _baseWidth; }
int16 getGUIHeight() const { return _baseHeight; }
diff --git a/gui/launcher.cpp b/gui/launcher.cpp
index b63991d2757..c163a8a47df 100644
--- a/gui/launcher.cpp
+++ b/gui/launcher.cpp
@@ -183,6 +183,7 @@ LauncherDialog::LauncherDialog(const Common::String &dialogName, LauncherChooser
Common::ArchiveMemberList mdFiles;
+ g_gui.lockIconsSet();
g_gui.getIconsSet().listMatchingMembers(mdFiles, "*.xml");
for (Common::ArchiveMemberList::iterator md = mdFiles.begin(); md != mdFiles.end(); ++md) {
if (_metadataParser.loadStream((*md)->createReadStream()) == false) {
@@ -194,6 +195,7 @@ LauncherDialog::LauncherDialog(const Common::String &dialogName, LauncherChooser
}
_metadataParser.close();
}
+ g_gui.unlockIconsSet();
}
LauncherDialog::~LauncherDialog() {
diff --git a/gui/widgets/grid.cpp b/gui/widgets/grid.cpp
index 8355133e9ae..ccae86ba5f2 100644
--- a/gui/widgets/grid.cpp
+++ b/gui/widgets/grid.cpp
@@ -291,9 +291,11 @@ Graphics::ManagedSurface *loadSurfaceFromFile(const Common::String &name, int re
#ifdef USE_PNG
const Graphics::Surface *srcSurface = nullptr;
Image::PNGDecoder decoder;
+ g_gui.lockIconsSet();
if (g_gui.getIconsSet().hasFile(name)) {
Common::SeekableReadStream *stream = g_gui.getIconsSet().createReadStreamForMember(name);
if (!decoder.loadStream(*stream)) {
+ g_gui.unlockIconsSet();
warning("Error decoding PNG");
return surf;
}
@@ -308,10 +310,12 @@ Graphics::ManagedSurface *loadSurfaceFromFile(const Common::String &name, int re
} else {
debug(5, "GridWidget: Cannot read file '%s'", name.c_str());
}
+ g_gui.unlockIconsSet();
#else
error("No PNG support compiled");
#endif
} else if (name.hasSuffix(".svg")) {
+ g_gui.lockIconsSet();
if (g_gui.getIconsSet().hasFile(name)) {
Common::SeekableReadStream *stream = g_gui.getIconsSet().createReadStreamForMember(name);
Graphics::SVGBitmap *image = nullptr;
@@ -325,6 +329,7 @@ Graphics::ManagedSurface *loadSurfaceFromFile(const Common::String &name, int re
} else {
debug(5, "GridWidget: Cannot read file '%s'", name.c_str());
}
+ g_gui.unlockIconsSet();
}
return surf;
}
More information about the Scummvm-git-logs
mailing list