From 6d9e7bea25fb1dce27ea6614bdb8e40b37fb828f Mon Sep 17 00:00:00 2001 From: Megamouse Date: Sun, 6 Jun 2021 10:00:43 +0200 Subject: [PATCH 1/3] Qt: simplify zero padding in trophy icon path --- rpcs3/rpcs3qt/trophy_manager_dialog.cpp | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp index 3ce63ebc55be..f689005839cd 100644 --- a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp +++ b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp @@ -372,28 +372,19 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name) if (trophy_count == 0) { - gui_log.error("Warning game %s in trophy folder %s usr file reports zero trophies. Cannot load in trophy manager.", game_trophy_data->game_name, game_trophy_data->path); + gui_log.error("Warning game %s in trophy folder %s usr file reports zero trophies. Cannot load in trophy manager.", game_trophy_data->game_name, game_trophy_data->path); return false; } for (u32 trophy_id = 0; trophy_id < trophy_count; ++trophy_id) { - // Figure out how many zeros are needed for padding. (either 0, 1, or 2) - QString padding = ""; - if (trophy_id < 10) - { - padding = "00"; - } - else if (trophy_id < 100) - { - padding = "0"; - } + // A trophy icon has 3 digits from 000 to 999, for example TROP001.PNG + const QString path = qstr(fmt::format("%sTROP%03d.PNG", game_trophy_data->path, trophy_id)); QPixmap trophy_icon; - const QString path = qstr(game_trophy_data->path) + "TROP" + padding + QString::number(trophy_id) + ".PNG"; if (!trophy_icon.load(path)) { - gui_log.error("Failed to load trophy icon for trophy %d %s", trophy_id, game_trophy_data->path); + gui_log.error("Failed to load trophy icon for trophy %d %s", trophy_id, sstr(path)); } game_trophy_data->trophy_images.emplace_back(std::move(trophy_icon)); } From 80f811730e6034ce6dd55cee97ad13e099e82e7c Mon Sep 17 00:00:00 2001 From: Megamouse Date: Sun, 6 Jun 2021 10:26:44 +0200 Subject: [PATCH 2/3] Qt: Only load trophy icons on a need to know basis Also cache existing trophy icons --- rpcs3/rpcs3qt/trophy_manager_dialog.cpp | 29 ++++++++++++++++--------- rpcs3/rpcs3qt/trophy_manager_dialog.h | 6 ++++- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp index f689005839cd..8617ae9f0be8 100644 --- a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp +++ b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp @@ -379,14 +379,7 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name) for (u32 trophy_id = 0; trophy_id < trophy_count; ++trophy_id) { // A trophy icon has 3 digits from 000 to 999, for example TROP001.PNG - const QString path = qstr(fmt::format("%sTROP%03d.PNG", game_trophy_data->path, trophy_id)); - - QPixmap trophy_icon; - if (!trophy_icon.load(path)) - { - gui_log.error("Failed to load trophy icon for trophy %d %s", trophy_id, sstr(path)); - } - game_trophy_data->trophy_images.emplace_back(std::move(trophy_icon)); + game_trophy_data->trophy_image_paths << qstr(fmt::format("%sTROP%03d.PNG", game_trophy_data->path, trophy_id)); } // Get game name @@ -542,7 +535,7 @@ void trophy_manager_dialog::ResizeTrophyIcons() for (int i = 0; i < m_trophy_table->rowCount(); ++i) indices.append(i); - const std::function get_scaled = [this, db_pos, dpr, new_height](const int& i) + const std::function get_scaled = [this, data = m_trophies_db.at(db_pos).get(), dpr, new_height](const int& i) { QTableWidgetItem* item = m_trophy_table->item(i, TrophyColumns::Id); QTableWidgetItem* icon_item = m_trophy_table->item(i, TrophyColumns::Icon); @@ -550,8 +543,24 @@ void trophy_manager_dialog::ResizeTrophyIcons() { return QPixmap(); } + const int trophy_id = item->text().toInt(); - const QPixmap icon = m_trophies_db[db_pos]->trophy_images[trophy_id]; + QPixmap icon; + { + std::scoped_lock lock(data->mtx); + if (data->trophy_images.contains(trophy_id)) + { + icon = data->trophy_images[trophy_id]; + } + else if (const QString& path = data->trophy_image_paths[trophy_id]; !icon.load(path)) + { + gui_log.error("Failed to load trophy icon for trophy %d (icon='%s')", trophy_id, sstr(path)); + } + else + { + data->trophy_images[trophy_id] = icon; + } + } QPixmap new_icon = QPixmap(icon.size() * dpr); new_icon.setDevicePixelRatio(dpr); diff --git a/rpcs3/rpcs3qt/trophy_manager_dialog.h b/rpcs3/rpcs3qt/trophy_manager_dialog.h index 4f3257612258..7c96a39c27cd 100644 --- a/rpcs3/rpcs3qt/trophy_manager_dialog.h +++ b/rpcs3/rpcs3qt/trophy_manager_dialog.h @@ -11,6 +11,8 @@ #include #include +#include +#include class game_list; class gui_settings; @@ -20,9 +22,11 @@ struct GameTrophiesData { std::unique_ptr trop_usr; rXmlDocument trop_config; // I'd like to use unique but the protocol inside of the function passes around shared pointers.. - std::vector trophy_images; + std::map trophy_images; // Cache trophy images to avoid loading from disk as much as possible. + QStringList trophy_image_paths; std::string game_name; std::string path; + std::mutex mtx; }; enum TrophyColumns From 3aa0227e7a7506c7913e926dc94c22fcc59609a3 Mon Sep 17 00:00:00 2001 From: Megamouse Date: Sun, 6 Jun 2021 10:43:39 +0200 Subject: [PATCH 3/3] Qt: Fix concurrency bug in trophy manager The missing mutex frequently caused a crash after I improved the individual trophy folder parsing by deferring icon loading to when it is actually needed. --- rpcs3/rpcs3qt/trophy_manager_dialog.cpp | 77 ++++++++++++++----------- rpcs3/rpcs3qt/trophy_manager_dialog.h | 8 +-- 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp index 8617ae9f0be8..457a7b42bfb1 100644 --- a/rpcs3/rpcs3qt/trophy_manager_dialog.cpp +++ b/rpcs3/rpcs3qt/trophy_manager_dialog.cpp @@ -210,7 +210,7 @@ trophy_manager_dialog::trophy_manager_dialog(std::shared_ptr gui_s setLayout(all_layout); // Make connects - connect(m_icon_slider, &QSlider::valueChanged, this, [=, this](int val) + connect(m_icon_slider, &QSlider::valueChanged, this, [this, trophy_slider_label](int val) { m_icon_height = val; if (trophy_slider_label) @@ -225,12 +225,12 @@ trophy_manager_dialog::trophy_manager_dialog(std::shared_ptr gui_s } }); - connect(m_icon_slider, &QSlider::sliderReleased, this, [&]() + connect(m_icon_slider, &QSlider::sliderReleased, this, [this]() { m_gui_settings->SetValue(gui::tr_icon_height, m_icon_slider->value()); }); - connect(m_icon_slider, &QSlider::actionTriggered, [&](int action) + connect(m_icon_slider, &QSlider::actionTriggered, this, [this](int action) { if (action != QAbstractSlider::SliderNoAction && action != QAbstractSlider::SliderMove) { // we only want to save on mouseclicks or slider release (the other connect handles this) @@ -238,7 +238,7 @@ trophy_manager_dialog::trophy_manager_dialog(std::shared_ptr gui_s } }); - connect(m_game_icon_slider, &QSlider::valueChanged, this, [=, this](int val) + connect(m_game_icon_slider, &QSlider::valueChanged, this, [this, game_slider_label](int val) { m_game_icon_size_index = val; m_game_icon_size = gui_settings::SizeFromSlider(val); @@ -254,12 +254,12 @@ trophy_manager_dialog::trophy_manager_dialog(std::shared_ptr gui_s } }); - connect(m_game_icon_slider, &QSlider::sliderReleased, this, [&]() + connect(m_game_icon_slider, &QSlider::sliderReleased, this, [this]() { m_gui_settings->SetValue(gui::tr_game_iconSize, m_game_icon_slider->value()); }); - connect(m_game_icon_slider, &QSlider::actionTriggered, [&](int action) + connect(m_game_icon_slider, &QSlider::actionTriggered, this, [this](int action) { if (action != QAbstractSlider::SliderNoAction && action != QAbstractSlider::SliderMove) { // we only want to save on mouseclicks or slider release (the other connect handles this) @@ -267,49 +267,49 @@ trophy_manager_dialog::trophy_manager_dialog(std::shared_ptr gui_s } }); - connect(check_lock_trophy, &QCheckBox::clicked, [this](bool checked) + connect(check_lock_trophy, &QCheckBox::clicked, this, [this](bool checked) { m_show_locked_trophies = checked; ApplyFilter(); m_gui_settings->SetValue(gui::tr_show_locked, checked); }); - connect(check_unlock_trophy, &QCheckBox::clicked, [this](bool checked) + connect(check_unlock_trophy, &QCheckBox::clicked, this, [this](bool checked) { m_show_unlocked_trophies = checked; ApplyFilter(); m_gui_settings->SetValue(gui::tr_show_unlocked, checked); }); - connect(check_hidden_trophy, &QCheckBox::clicked, [this](bool checked) + connect(check_hidden_trophy, &QCheckBox::clicked, this, [this](bool checked) { m_show_hidden_trophies = checked; ApplyFilter(); m_gui_settings->SetValue(gui::tr_show_hidden, checked); }); - connect(check_bronze_trophy, &QCheckBox::clicked, [this](bool checked) + connect(check_bronze_trophy, &QCheckBox::clicked, this, [this](bool checked) { m_show_bronze_trophies = checked; ApplyFilter(); m_gui_settings->SetValue(gui::tr_show_bronze, checked); }); - connect(check_silver_trophy, &QCheckBox::clicked, [this](bool checked) + connect(check_silver_trophy, &QCheckBox::clicked, this, [this](bool checked) { m_show_silver_trophies = checked; ApplyFilter(); m_gui_settings->SetValue(gui::tr_show_silver, checked); }); - connect(check_gold_trophy, &QCheckBox::clicked, [this](bool checked) + connect(check_gold_trophy, &QCheckBox::clicked, this, [this](bool checked) { m_show_gold_trophies = checked; ApplyFilter(); m_gui_settings->SetValue(gui::tr_show_gold, checked); }); - connect(check_platinum_trophy, &QCheckBox::clicked, [this](bool checked) + connect(check_platinum_trophy, &QCheckBox::clicked, this, [this](bool checked) { m_show_platinum_trophies = checked; ApplyFilter(); @@ -318,13 +318,13 @@ trophy_manager_dialog::trophy_manager_dialog(std::shared_ptr gui_s connect(m_trophy_table, &QTableWidget::customContextMenuRequested, this, &trophy_manager_dialog::ShowContextMenu); - connect(m_game_combo, &QComboBox::currentTextChanged, [this] + connect(m_game_combo, &QComboBox::currentTextChanged, this, [this] { PopulateTrophyTable(); ApplyFilter(); }); - connect(m_game_table, &QTableWidget::itemSelectionChanged, [this] + connect(m_game_table, &QTableWidget::itemSelectionChanged, this, [this] { if (m_game_table->selectedItems().isEmpty()) { @@ -356,11 +356,11 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name) game_trophy_data->path = vfs::get(trophyPath + "/"); game_trophy_data->trop_usr.reset(new TROPUSRLoader()); - const std::string trophyUsrPath = trophyPath + "/TROPUSR.DAT"; - const std::string trophyConfPath = trophyPath + "/TROPCONF.SFM"; - const bool success = game_trophy_data->trop_usr->Load(trophyUsrPath, trophyConfPath).success; + const std::string tropusr_path = trophyPath + "/TROPUSR.DAT"; + const std::string tropconf_path = trophyPath + "/TROPCONF.SFM"; + const bool success = game_trophy_data->trop_usr->Load(tropusr_path, tropconf_path).success; - fs::file config(vfs::get(trophyConfPath)); + fs::file config(vfs::get(tropconf_path)); if (!success || !config) { @@ -379,7 +379,7 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name) for (u32 trophy_id = 0; trophy_id < trophy_count; ++trophy_id) { // A trophy icon has 3 digits from 000 to 999, for example TROP001.PNG - game_trophy_data->trophy_image_paths << qstr(fmt::format("%sTROP%03d.PNG", game_trophy_data->path, trophy_id)); + game_trophy_data->trophy_image_paths[trophy_id] = qstr(fmt::format("%sTROP%03d.PNG", game_trophy_data->path, trophy_id)); } // Get game name @@ -398,7 +398,10 @@ bool trophy_manager_dialog::LoadTrophyFolderToDB(const std::string& trop_name) } } - m_trophies_db.push_back(std::move(game_trophy_data)); + { + std::scoped_lock lock(m_trophies_db_mtx); + m_trophies_db.push_back(std::move(game_trophy_data)); + } config.release(); return true; @@ -531,23 +534,20 @@ void trophy_manager_dialog::ResizeTrophyIcons() const qreal dpr = devicePixelRatioF(); const int new_height = m_icon_height * dpr; - QList indices; + QList trophy_ids; for (int i = 0; i < m_trophy_table->rowCount(); ++i) - indices.append(i); - - const std::function get_scaled = [this, data = m_trophies_db.at(db_pos).get(), dpr, new_height](const int& i) { - QTableWidgetItem* item = m_trophy_table->item(i, TrophyColumns::Id); - QTableWidgetItem* icon_item = m_trophy_table->item(i, TrophyColumns::Icon); - if (!item || !icon_item) + if (QTableWidgetItem* item = m_trophy_table->item(i, TrophyColumns::Id)) { - return QPixmap(); + trophy_ids.append(item->text().toInt()); } + } - const int trophy_id = item->text().toInt(); + const std::function get_scaled = [this, data = m_trophies_db.at(db_pos).get(), dpr, new_height](const int& trophy_id) + { QPixmap icon; { - std::scoped_lock lock(data->mtx); + std::scoped_lock lock(m_trophies_db_mtx); if (data->trophy_images.contains(trophy_id)) { icon = data->trophy_images[trophy_id]; @@ -577,13 +577,18 @@ void trophy_manager_dialog::ResizeTrophyIcons() return new_icon.scaledToHeight(new_height, Qt::SmoothTransformation); }; - QList scaled = QtConcurrent::blockingMapped>(indices, get_scaled); - for (int i = 0; i < m_trophy_table->rowCount() && i < scaled.count(); ++i) + // NOTE: Due to a Qt bug, QtConcurrent::blockingMapped has a high risk of deadlocking + // during the QPixmap operations in get_scaled. So let's just use QtConcurrent::mapped. + QFutureWatcher watcher; + watcher.setFuture(QtConcurrent::mapped(trophy_ids, get_scaled)); + watcher.waitForFinished(); + + for (int i = 0; i < m_trophy_table->rowCount() && i < watcher.future().resultCount(); ++i) { QTableWidgetItem* icon_item = m_trophy_table->item(i, TrophyColumns::Icon); if (icon_item) - icon_item->setData(Qt::DecorationRole, scaled[i]); + icon_item->setData(Qt::DecorationRole, watcher.future().resultAt(i)); } ReadjustTrophyTable(); @@ -657,7 +662,7 @@ void trophy_manager_dialog::ShowContextMenu(const QPoint& pos) const int db_ind = m_game_combo->currentData().toInt(); - connect(show_trophy_dir, &QAction::triggered, [=, this]() + connect(show_trophy_dir, &QAction::triggered, this, [this, db_ind]() { const QString path = qstr(m_trophies_db[db_ind]->path); QDesktopServices::openUrl(QUrl::fromLocalFile(path)); @@ -763,6 +768,8 @@ void trophy_manager_dialog::PopulateGameTable() m_game_combo->addItem(name, i); } + m_game_combo->model()->sort(0, Qt::AscendingOrder); + ResizeGameIcons(); m_game_table->setSortingEnabled(true); // Enable sorting only after using setItem calls diff --git a/rpcs3/rpcs3qt/trophy_manager_dialog.h b/rpcs3/rpcs3qt/trophy_manager_dialog.h index 7c96a39c27cd..4fc018965607 100644 --- a/rpcs3/rpcs3qt/trophy_manager_dialog.h +++ b/rpcs3/rpcs3qt/trophy_manager_dialog.h @@ -11,8 +11,8 @@ #include #include -#include #include +#include class game_list; class gui_settings; @@ -22,11 +22,10 @@ struct GameTrophiesData { std::unique_ptr trop_usr; rXmlDocument trop_config; // I'd like to use unique but the protocol inside of the function passes around shared pointers.. - std::map trophy_images; // Cache trophy images to avoid loading from disk as much as possible. - QStringList trophy_image_paths; + std::unordered_map trophy_images; // Cache trophy images to avoid loading from disk as much as possible. + std::unordered_map trophy_image_paths; std::string game_name; std::string path; - std::mutex mtx; }; enum TrophyColumns @@ -98,6 +97,7 @@ private Q_SLOTS: std::shared_ptr m_gui_settings; std::vector> m_trophies_db; //! Holds all the trophy information. + std::mutex m_trophies_db_mtx; QComboBox* m_game_combo; //! Lets you choose a game QLabel* m_game_progress; //! Shows you the current game's progress QSplitter* m_splitter; //! Contains the game and trophy tables