From f28799864710bb82c8dfe58f9c51e89c90c8b4e5 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 22 May 2021 19:46:39 +0200 Subject: [PATCH 01/13] skin/legacy: Add Skin abstraction --- CMakeLists.txt | 1 + src/preferences/dialog/dlgprefinterface.cpp | 74 +++++---------------- src/skin/legacy/skin.cpp | 69 +++++++++++++++++++ src/skin/legacy/skin.h | 36 ++++++++++ src/skin/skinloader.cpp | 17 +++++ src/skin/skinloader.h | 2 + 6 files changed, 141 insertions(+), 58 deletions(-) create mode 100644 src/skin/legacy/skin.cpp create mode 100644 src/skin/legacy/skin.h diff --git a/CMakeLists.txt b/CMakeLists.txt index d6f16085184c..f39af56a4df6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -732,6 +732,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/skin/legacy/launchimage.cpp src/skin/legacy/legacyskinparser.cpp src/skin/legacy/pixmapsource.cpp + src/skin/legacy/skin.cpp src/skin/legacy/skincontext.cpp src/skin/legacy/tooltips.cpp src/skin/skinloader.cpp diff --git a/src/preferences/dialog/dlgprefinterface.cpp b/src/preferences/dialog/dlgprefinterface.cpp index 27917233d9d5..a4683f8ad057 100644 --- a/src/preferences/dialog/dlgprefinterface.cpp +++ b/src/preferences/dialog/dlgprefinterface.cpp @@ -15,49 +15,13 @@ #include "moc_dlgprefinterface.cpp" #include "preferences/usersettings.h" #include "skin/legacy/legacyskinparser.h" +#include "skin/legacy/skin.h" #include "skin/skinloader.h" #include "util/screensaver.h" #include "util/widgethelper.h" using mixxx::skin::SkinManifest; - -namespace { - -const QRegExp kMinSizeRegExp("(\\d+), *(\\d+)<"); - -bool skinFitsScreenSize( - const QScreen& screen, - const QString& skin) { - // Use the full resolution of the entire screen that is - // available in full-screen mode. - const auto screenSize = screen.size(); - QFile skinfile(skin + QStringLiteral("/skin.xml")); - if (skinfile.open(QFile::ReadOnly | QFile::Text)) { - QTextStream in(&skinfile); - bool found_size = false; - while (!in.atEnd()) { - if (kMinSizeRegExp.indexIn(in.readLine()) != -1) { - found_size = true; - break; - } - } - if (found_size) { - return !(kMinSizeRegExp.cap(1).toInt() > screenSize.width() || - kMinSizeRegExp.cap(2).toInt() > screenSize.height()); - } - } - - // If regex failed, fall back to skin name parsing. - QString skinName = skin.left(skin.indexOf(QRegExp("\\d"))); - QString resName = skin.right(skin.count() - skinName.count()); - QString res = resName.left(resName.lastIndexOf(QRegExp("\\d")) + 1); - QString skinWidth = res.left(res.indexOf("x")); - QString skinHeight = res.right(res.count() - skinWidth.count() - 1); - return skinWidth.toInt() <= screenSize.width() && - skinHeight.toInt() <= screenSize.height(); -} - -} // namespace +using mixxx::skin::legacy::Skin; DlgPrefInterface::DlgPrefInterface( QWidget* parent, @@ -145,26 +109,21 @@ DlgPrefInterface::DlgPrefInterface( skinDescriptionText->setText(""); skinDescriptionText->hide(); - QList skinSearchPaths = m_pSkinLoader->getSkinSearchPaths(); - QList skins; - for (QDir& dir : skinSearchPaths) { - dir.setFilter(QDir::Dirs | QDir::NoDotAndDotDot); - skins.append(dir.entryInfoList()); - } + const QList skins = m_pSkinLoader->getSkins(); QString configuredSkinPath = m_pSkinLoader->getConfiguredSkinPath(); int index = 0; const auto* const pScreen = getScreen(); - for (const QFileInfo& skinInfo : skins) { - ComboBoxSkinconf->insertItem(index, skinInfo.fileName()); + for (const Skin& skin : skins) { + ComboBoxSkinconf->insertItem(index, skin.name()); - if (skinInfo.absoluteFilePath() == configuredSkinPath) { - m_skin = skinInfo.fileName(); + if (skin.path() == configuredSkinPath) { + m_skin = skin.name(); ComboBoxSkinconf->setCurrentIndex(index); // schemes must be updated here to populate the drop-down box and set m_colorScheme slotUpdateSchemes(); skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(m_skin, m_colorScheme)); - if (skinFitsScreenSize(*pScreen, configuredSkinPath)) { + if (skin.fitsScreenSize(*pScreen)) { warningLabel->hide(); } else { warningLabel->show(); @@ -234,8 +193,8 @@ QScreen* DlgPrefInterface::getScreen() const { void DlgPrefInterface::slotUpdateSchemes() { // Re-populates the scheme combobox and attempts to pick the color scheme from config file. // Since this involves opening a file we won't do this as part of regular slotUpdate - QList schlist = LegacySkinParser::getSchemeList( - m_pSkinLoader->getSkinPath(m_skin)); + const Skin skin(QFileInfo(m_pSkinLoader->getSkinPath(m_skin))); + QList schlist = skin.colorschemes(); ComboBoxSchemeconf->clear(); @@ -378,11 +337,10 @@ void DlgPrefInterface::slotSetScheme(int) { skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(m_skin, m_colorScheme)); } -void DlgPrefInterface::slotSetSkinDescription(const QString& skin) { - SkinManifest manifest = LegacySkinParser::getSkinManifest( - LegacySkinParser::openSkin(m_pSkinLoader->getSkinPath(skin))); - QString description = QString::fromStdString(manifest.description()); - if (manifest.has_description() && !description.isEmpty()) { +void DlgPrefInterface::slotSetSkinDescription(const QString& skinName) { + const Skin skin(QFileInfo(m_pSkinLoader->getSkinPath(skinName))); + const QString description = skin.description(); + if (!description.isEmpty()) { skinDescriptionText->show(); skinDescriptionText->setText(description); } else { @@ -394,10 +352,10 @@ void DlgPrefInterface::slotSetSkin(int) { QString newSkin = ComboBoxSkinconf->currentText(); if (newSkin != m_skin) { m_skin = newSkin; + const Skin skin(QFileInfo(m_pSkinLoader->getSkinPath(m_skin))); m_bRebootMixxxView = newSkin != m_skinOnUpdate; const auto* const pScreen = getScreen(); - if (pScreen && - skinFitsScreenSize(*pScreen, m_pSkinLoader->getSkinPath(m_skin))) { + if (pScreen && skin.fitsScreenSize(*pScreen)) { warningLabel->hide(); } else { warningLabel->show(); diff --git a/src/skin/legacy/skin.cpp b/src/skin/legacy/skin.cpp new file mode 100644 index 000000000000..bcaa97c9d331 --- /dev/null +++ b/src/skin/legacy/skin.cpp @@ -0,0 +1,69 @@ +#include "skin/legacy/skin.h" + +#include "skin/legacy/legacyskinparser.h" + +namespace { + +const QRegExp kMinSizeRegExp("(\\d+), *(\\d+)<"); + +} // namespace + +namespace mixxx { +namespace skin { +namespace legacy { + +QFileInfo Skin::skinFile() const { + return QFileInfo(path().absoluteFilePath() + QStringLiteral("/skin.xml")); +} + +QString Skin::name() const { + return m_path.fileName(); +} + +QList Skin::colorschemes() const { + return LegacySkinParser::getSchemeList(path().absoluteFilePath()); +} + +QString Skin::description() const { + SkinManifest manifest = LegacySkinParser::getSkinManifest( + LegacySkinParser::openSkin(path().absoluteFilePath())); + QString description = QString::fromStdString(manifest.description()); + if (!manifest.has_description() || description.isEmpty()) { + return QString(); + } + return description; +} + +bool Skin::fitsScreenSize(const QScreen& screen) const { + // Use the full resolution of the entire screen that is + // available in full-screen mode. + const auto screenSize = screen.size(); + QFile file(skinFile().absoluteFilePath()); + if (file.open(QFile::ReadOnly | QFile::Text)) { + QTextStream in(&file); + bool found_size = false; + while (!in.atEnd()) { + if (kMinSizeRegExp.indexIn(in.readLine()) != -1) { + found_size = true; + break; + } + } + if (found_size) { + return !(kMinSizeRegExp.cap(1).toInt() > screenSize.width() || + kMinSizeRegExp.cap(2).toInt() > screenSize.height()); + } + } + + // If regex failed, fall back to skin name parsing. + QString skinName = name().left(name().indexOf(QRegExp("\\d"))); + QString resName = name().right(name().count() - skinName.count()); + QString res = resName.left(resName.lastIndexOf(QRegExp("\\d")) + 1); + QString skinWidth = res.left(res.indexOf("x")); + QString skinHeight = res.right(res.count() - skinWidth.count() - 1); + return skinWidth.toInt() <= screenSize.width() && + skinHeight.toInt() <= screenSize.height(); +} + +} // namespace legacy +} // namespace skin +} // namespace mixxx diff --git a/src/skin/legacy/skin.h b/src/skin/legacy/skin.h new file mode 100644 index 000000000000..5b036b9c9a5c --- /dev/null +++ b/src/skin/legacy/skin.h @@ -0,0 +1,36 @@ +#pragma once + +#include +#include +#include +#include + +namespace mixxx { +namespace skin { +namespace legacy { + +class Skin { + public: + explicit Skin(const QFileInfo& path) + : m_path(path) { + } + + QFileInfo path() const { + return m_path; + } + + QString name() const; + QString description() const; + QList colorschemes() const; + + bool fitsScreenSize(const QScreen& screen) const; + + private: + QFileInfo skinFile() const; + + QFileInfo m_path; +}; + +} // namespace legacy +} // namespace skin +} // namespace mixxx diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index d54fbab610ed..3c1eae58fd07 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -16,6 +16,8 @@ #include "util/timer.h" #include "recording/recordingmanager.h" +using mixxx::skin::legacy::Skin; + SkinLoader::SkinLoader(UserSettingsPointer pConfig) : m_pConfig(pConfig) { } @@ -24,6 +26,21 @@ SkinLoader::~SkinLoader() { LegacySkinParser::clearSharedGroupStrings(); } +QList SkinLoader::getSkins() const { + const QList skinSearchPaths = getSkinSearchPaths(); + QList skins; + for (const QDir& dir : skinSearchPaths) { + for (const QFileInfo& fileInfo : dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot)) { + QDir skinDir(fileInfo.absoluteFilePath()); + if (skinDir.exists(QStringLiteral("skin.xml"))) { + skins.append(Skin(fileInfo)); + } + } + } + + return skins; +} + QList SkinLoader::getSkinSearchPaths() const { QList searchPaths; diff --git a/src/skin/skinloader.h b/src/skin/skinloader.h index 9b3796508ffb..287ea9ae7faf 100644 --- a/src/skin/skinloader.h +++ b/src/skin/skinloader.h @@ -6,6 +6,7 @@ #include #include "preferences/usersettings.h" +#include "skin/legacy/skin.h" class KeyboardEventFilter; class PlayerManager; @@ -39,6 +40,7 @@ class SkinLoader { QString getConfiguredSkinPath() const; QString getDefaultSkinName() const; QList getSkinSearchPaths() const; + QList getSkins() const; private: QString pickResizableSkin(const QString& oldSkin) const; From e596e51739d02ea61c0ad37aee8b2c61c84d773f Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 22 May 2021 20:51:08 +0200 Subject: [PATCH 02/13] DlgPrefInterface: Use QMap of skins --- src/preferences/dialog/dlgprefinterface.cpp | 73 +++++++++++---------- src/preferences/dialog/dlgprefinterface.h | 8 ++- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/preferences/dialog/dlgprefinterface.cpp b/src/preferences/dialog/dlgprefinterface.cpp index a4683f8ad057..b47cd9121e4a 100644 --- a/src/preferences/dialog/dlgprefinterface.cpp +++ b/src/preferences/dialog/dlgprefinterface.cpp @@ -32,6 +32,7 @@ DlgPrefInterface::DlgPrefInterface( m_pConfig(pConfig), m_mixxx(mixxx), m_pSkinLoader(pSkinLoader), + m_skin(QFileInfo(pSkinLoader->getConfiguredSkinPath())), m_dScaleFactorAuto(1.0), m_bUseAutoScaleFactor(false), m_dScaleFactor(1.0), @@ -110,28 +111,26 @@ DlgPrefInterface::DlgPrefInterface( skinDescriptionText->hide(); const QList skins = m_pSkinLoader->getSkins(); - - QString configuredSkinPath = m_pSkinLoader->getConfiguredSkinPath(); int index = 0; - const auto* const pScreen = getScreen(); for (const Skin& skin : skins) { ComboBoxSkinconf->insertItem(index, skin.name()); - - if (skin.path() == configuredSkinPath) { - m_skin = skin.name(); - ComboBoxSkinconf->setCurrentIndex(index); - // schemes must be updated here to populate the drop-down box and set m_colorScheme - slotUpdateSchemes(); - skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(m_skin, m_colorScheme)); - if (skin.fitsScreenSize(*pScreen)) { - warningLabel->hide(); - } else { - warningLabel->show(); - } - slotSetSkinDescription(m_skin); - } + m_skins.insert(skin.name(), std::optional(skin)); index++; } + qWarning() << m_skins.keys(); + + ComboBoxSkinconf->setCurrentIndex(index); + // schemes must be updated here to populate the drop-down box and set m_colorScheme + slotUpdateSchemes(); + skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview( + m_skin.name(), m_colorScheme)); + const auto* const pScreen = getScreen(); + if (m_skin.fitsScreenSize(*pScreen)) { + warningLabel->hide(); + } else { + warningLabel->show(); + } + slotSetSkinDescription(m_skin.name()); connect(ComboBoxSkinconf, QOverload::of(&QComboBox::currentIndexChanged), @@ -193,8 +192,7 @@ QScreen* DlgPrefInterface::getScreen() const { void DlgPrefInterface::slotUpdateSchemes() { // Re-populates the scheme combobox and attempts to pick the color scheme from config file. // Since this involves opening a file we won't do this as part of regular slotUpdate - const Skin skin(QFileInfo(m_pSkinLoader->getSkinPath(m_skin))); - QList schlist = skin.colorschemes(); + QList schlist = m_skin.colorschemes(); ComboBoxSchemeconf->clear(); @@ -228,11 +226,15 @@ void DlgPrefInterface::slotUpdateSchemes() { } void DlgPrefInterface::slotUpdate() { - m_skinOnUpdate = m_pConfig->getValueString(ConfigKey("[Config]", "ResizableSkin")); - if (m_skinOnUpdate.isEmpty()) { - m_skinOnUpdate = m_pSkinLoader->getDefaultSkinName(); + const QString skinNameOnUpdate = + m_pConfig->getValueString(ConfigKey("[Config]", "ResizableSkin")); + const std::optional skinOnUpdate = m_skins[skinNameOnUpdate]; + if (skinOnUpdate) { + m_skinNameOnUpdate = (*skinOnUpdate).name(); + } else { + m_skinNameOnUpdate = m_pSkinLoader->getDefaultSkinName(); } - ComboBoxSkinconf->setCurrentIndex(ComboBoxSkinconf->findText(m_skinOnUpdate)); + ComboBoxSkinconf->setCurrentIndex(ComboBoxSkinconf->findText(m_skinNameOnUpdate)); slotUpdateSchemes(); m_bRebootMixxxView = false; @@ -334,7 +336,7 @@ void DlgPrefInterface::slotSetScheme(int) { m_colorScheme = newScheme; m_bRebootMixxxView = true; } - skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(m_skin, m_colorScheme)); + skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(m_skin.name(), m_colorScheme)); } void DlgPrefInterface::slotSetSkinDescription(const QString& skinName) { @@ -349,26 +351,29 @@ void DlgPrefInterface::slotSetSkinDescription(const QString& skinName) { } void DlgPrefInterface::slotSetSkin(int) { - QString newSkin = ComboBoxSkinconf->currentText(); - if (newSkin != m_skin) { - m_skin = newSkin; - const Skin skin(QFileInfo(m_pSkinLoader->getSkinPath(m_skin))); - m_bRebootMixxxView = newSkin != m_skinOnUpdate; + QString newSkinName = ComboBoxSkinconf->currentText(); + if (newSkinName != m_skin.name()) { + const std::optional newSkin = m_skins[newSkinName]; + VERIFY_OR_DEBUG_ASSERT(newSkin) { + return; + } + m_skin = *newSkin; + m_bRebootMixxxView = newSkinName != m_skinNameOnUpdate; const auto* const pScreen = getScreen(); - if (pScreen && skin.fitsScreenSize(*pScreen)) { + if (pScreen && m_skin.fitsScreenSize(*pScreen)) { warningLabel->hide(); } else { warningLabel->show(); } slotUpdateSchemes(); - slotSetSkinDescription(m_skin); + slotSetSkinDescription(m_skin.name()); } - skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(newSkin, m_colorScheme)); + skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(newSkinName, m_colorScheme)); } void DlgPrefInterface::slotApply() { - m_pConfig->set(ConfigKey("[Config]", "ResizableSkin"), m_skin); + m_pConfig->set(ConfigKey("[Config]", "ResizableSkin"), m_skin.name()); m_pConfig->set(ConfigKey("[Config]", "Scheme"), m_colorScheme); QString locale = ComboBoxLocale->itemData( @@ -407,7 +412,7 @@ void DlgPrefInterface::slotApply() { if (m_bRebootMixxxView) { m_mixxx->rebootMixxxView(); // Allow switching skins multiple times without closing the dialog - m_skinOnUpdate = m_skin; + m_skinNameOnUpdate = m_skin.name(); } m_bRebootMixxxView = false; } diff --git a/src/preferences/dialog/dlgprefinterface.h b/src/preferences/dialog/dlgprefinterface.h index 694abc2dc1cf..face751977f0 100644 --- a/src/preferences/dialog/dlgprefinterface.h +++ b/src/preferences/dialog/dlgprefinterface.h @@ -1,12 +1,15 @@ #pragma once +#include #include #include +#include #include "preferences/constants.h" #include "preferences/dialog/dlgpreferencepage.h" #include "preferences/dialog/ui_dlgprefinterfacedlg.h" #include "preferences/usersettings.h" +#include "skin/legacy/skin.h" class ControlProxy; class ControlPotmeter; @@ -54,8 +57,9 @@ class DlgPrefInterface : public DlgPreferencePage, public Ui::DlgPrefControlsDlg MixxxMainWindow *m_mixxx; std::shared_ptr m_pSkinLoader; - QString m_skin; - QString m_skinOnUpdate; + QMap> m_skins; + mixxx::skin::legacy::Skin m_skin; + QString m_skinNameOnUpdate; QString m_colorScheme; QString m_localeOnUpdate; mixxx::TooltipsPreference m_tooltipMode; From 4675b640b5fa7b1d2442b2e731ce8fff8b35ac8e Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 22 May 2021 20:53:29 +0200 Subject: [PATCH 03/13] DlgPrefInterface: Remove unused parameter from slotSetSkinDescription --- src/preferences/dialog/dlgprefinterface.cpp | 9 ++++----- src/preferences/dialog/dlgprefinterface.h | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/preferences/dialog/dlgprefinterface.cpp b/src/preferences/dialog/dlgprefinterface.cpp index b47cd9121e4a..fb2b1158b0d1 100644 --- a/src/preferences/dialog/dlgprefinterface.cpp +++ b/src/preferences/dialog/dlgprefinterface.cpp @@ -130,7 +130,7 @@ DlgPrefInterface::DlgPrefInterface( } else { warningLabel->show(); } - slotSetSkinDescription(m_skin.name()); + slotSetSkinDescription(); connect(ComboBoxSkinconf, QOverload::of(&QComboBox::currentIndexChanged), @@ -339,9 +339,8 @@ void DlgPrefInterface::slotSetScheme(int) { skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(m_skin.name(), m_colorScheme)); } -void DlgPrefInterface::slotSetSkinDescription(const QString& skinName) { - const Skin skin(QFileInfo(m_pSkinLoader->getSkinPath(skinName))); - const QString description = skin.description(); +void DlgPrefInterface::slotSetSkinDescription() { + const QString description = m_skin.description(); if (!description.isEmpty()) { skinDescriptionText->show(); skinDescriptionText->setText(description); @@ -366,7 +365,7 @@ void DlgPrefInterface::slotSetSkin(int) { warningLabel->show(); } slotUpdateSchemes(); - slotSetSkinDescription(m_skin.name()); + slotSetSkinDescription(); } skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(newSkinName, m_colorScheme)); diff --git a/src/preferences/dialog/dlgprefinterface.h b/src/preferences/dialog/dlgprefinterface.h index face751977f0..e378b95fc4c2 100644 --- a/src/preferences/dialog/dlgprefinterface.h +++ b/src/preferences/dialog/dlgprefinterface.h @@ -34,7 +34,7 @@ class DlgPrefInterface : public DlgPreferencePage, public Ui::DlgPrefControlsDlg void slotResetToDefaults() override; void slotSetTooltips(); - void slotSetSkinDescription(const QString& skin); + void slotSetSkinDescription(); void slotSetSkin(int); void slotSetScheme(int); void slotUpdateSchemes(); From c4357ca733c11fac4a1418a0cefd5215333e5b99 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 22 May 2021 20:55:10 +0200 Subject: [PATCH 04/13] DlgPrefInterface: Make some slots private --- src/preferences/dialog/dlgprefinterface.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/preferences/dialog/dlgprefinterface.h b/src/preferences/dialog/dlgprefinterface.h index e378b95fc4c2..25c0176b4baf 100644 --- a/src/preferences/dialog/dlgprefinterface.h +++ b/src/preferences/dialog/dlgprefinterface.h @@ -33,6 +33,7 @@ class DlgPrefInterface : public DlgPreferencePage, public Ui::DlgPrefControlsDlg void slotApply() override; void slotResetToDefaults() override; + private slots: void slotSetTooltips(); void slotSetSkinDescription(); void slotSetSkin(int); From b03a4196f2f154060c7796d7cf049507c6ac96a9 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 22 May 2021 22:44:41 +0200 Subject: [PATCH 05/13] SkinLoader: Avoid detach of Qt container in range loop --- src/skin/skinloader.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index 3c1eae58fd07..3e14bd27e4da 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -30,7 +30,8 @@ QList SkinLoader::getSkins() const { const QList skinSearchPaths = getSkinSearchPaths(); QList skins; for (const QDir& dir : skinSearchPaths) { - for (const QFileInfo& fileInfo : dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot)) { + const QList fileInfos = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot); + for (const QFileInfo& fileInfo : fileInfos) { QDir skinDir(fileInfo.absoluteFilePath()); if (skinDir.exists(QStringLiteral("skin.xml"))) { skins.append(Skin(fileInfo)); From 6f56f2e31b600b48bdf809d8032e55974d9384e2 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 22 May 2021 23:07:45 +0200 Subject: [PATCH 06/13] Skin: Add default constructor and add validity checks --- src/preferences/dialog/dlgprefinterface.cpp | 14 +++++++------- src/preferences/dialog/dlgprefinterface.h | 2 +- src/skin/legacy/skin.cpp | 19 +++++++++++++++++++ src/skin/legacy/skin.h | 10 ++++------ 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/preferences/dialog/dlgprefinterface.cpp b/src/preferences/dialog/dlgprefinterface.cpp index fb2b1158b0d1..6867dcbf8bc5 100644 --- a/src/preferences/dialog/dlgprefinterface.cpp +++ b/src/preferences/dialog/dlgprefinterface.cpp @@ -114,7 +114,7 @@ DlgPrefInterface::DlgPrefInterface( int index = 0; for (const Skin& skin : skins) { ComboBoxSkinconf->insertItem(index, skin.name()); - m_skins.insert(skin.name(), std::optional(skin)); + m_skins.insert(skin.name(), skin); index++; } qWarning() << m_skins.keys(); @@ -228,9 +228,9 @@ void DlgPrefInterface::slotUpdateSchemes() { void DlgPrefInterface::slotUpdate() { const QString skinNameOnUpdate = m_pConfig->getValueString(ConfigKey("[Config]", "ResizableSkin")); - const std::optional skinOnUpdate = m_skins[skinNameOnUpdate]; - if (skinOnUpdate) { - m_skinNameOnUpdate = (*skinOnUpdate).name(); + const Skin skinOnUpdate = m_skins[skinNameOnUpdate]; + if (skinOnUpdate.isValid()) { + m_skinNameOnUpdate = skinOnUpdate.name(); } else { m_skinNameOnUpdate = m_pSkinLoader->getDefaultSkinName(); } @@ -352,11 +352,11 @@ void DlgPrefInterface::slotSetSkinDescription() { void DlgPrefInterface::slotSetSkin(int) { QString newSkinName = ComboBoxSkinconf->currentText(); if (newSkinName != m_skin.name()) { - const std::optional newSkin = m_skins[newSkinName]; - VERIFY_OR_DEBUG_ASSERT(newSkin) { + const Skin newSkin = m_skins[newSkinName]; + VERIFY_OR_DEBUG_ASSERT(newSkin.isValid()) { return; } - m_skin = *newSkin; + m_skin = newSkin; m_bRebootMixxxView = newSkinName != m_skinNameOnUpdate; const auto* const pScreen = getScreen(); if (pScreen && m_skin.fitsScreenSize(*pScreen)) { diff --git a/src/preferences/dialog/dlgprefinterface.h b/src/preferences/dialog/dlgprefinterface.h index 25c0176b4baf..12096af9e6c0 100644 --- a/src/preferences/dialog/dlgprefinterface.h +++ b/src/preferences/dialog/dlgprefinterface.h @@ -58,7 +58,7 @@ class DlgPrefInterface : public DlgPreferencePage, public Ui::DlgPrefControlsDlg MixxxMainWindow *m_mixxx; std::shared_ptr m_pSkinLoader; - QMap> m_skins; + QMap m_skins; mixxx::skin::legacy::Skin m_skin; QString m_skinNameOnUpdate; QString m_colorScheme; diff --git a/src/skin/legacy/skin.cpp b/src/skin/legacy/skin.cpp index bcaa97c9d331..48465501beb4 100644 --- a/src/skin/legacy/skin.cpp +++ b/src/skin/legacy/skin.cpp @@ -12,19 +12,37 @@ namespace mixxx { namespace skin { namespace legacy { +Skin::Skin(const QFileInfo& path) + : m_path(path) { + DEBUG_ASSERT(isValid()); +} + +bool Skin::isValid() const { + return !m_path.filePath().isEmpty() && m_path.exists(); +} + +QFileInfo Skin::path() const { + DEBUG_ASSERT(isValid()); + return m_path; +} + QFileInfo Skin::skinFile() const { + DEBUG_ASSERT(isValid()); return QFileInfo(path().absoluteFilePath() + QStringLiteral("/skin.xml")); } QString Skin::name() const { + DEBUG_ASSERT(isValid()); return m_path.fileName(); } QList Skin::colorschemes() const { + DEBUG_ASSERT(isValid()); return LegacySkinParser::getSchemeList(path().absoluteFilePath()); } QString Skin::description() const { + DEBUG_ASSERT(isValid()); SkinManifest manifest = LegacySkinParser::getSkinManifest( LegacySkinParser::openSkin(path().absoluteFilePath())); QString description = QString::fromStdString(manifest.description()); @@ -35,6 +53,7 @@ QString Skin::description() const { } bool Skin::fitsScreenSize(const QScreen& screen) const { + DEBUG_ASSERT(isValid()); // Use the full resolution of the entire screen that is // available in full-screen mode. const auto screenSize = screen.size(); diff --git a/src/skin/legacy/skin.h b/src/skin/legacy/skin.h index 5b036b9c9a5c..0effc819b203 100644 --- a/src/skin/legacy/skin.h +++ b/src/skin/legacy/skin.h @@ -11,13 +11,11 @@ namespace legacy { class Skin { public: - explicit Skin(const QFileInfo& path) - : m_path(path) { - } + Skin() = default; + Skin(const QFileInfo& path); - QFileInfo path() const { - return m_path; - } + bool isValid() const; + QFileInfo path() const; QString name() const; QString description() const; From ab8347d453fcceb5079ea1c6f1f6fa2ad53e8e39 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 22 May 2021 23:26:31 +0200 Subject: [PATCH 07/13] Skin: Add preview() method to retrieve preview image --- src/preferences/dialog/dlgprefinterface.cpp | 42 ++++++++++----------- src/skin/legacy/skin.cpp | 18 +++++++++ src/skin/legacy/skin.h | 2 + src/skin/skinloader.cpp | 16 -------- src/skin/skinloader.h | 1 - 5 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/preferences/dialog/dlgprefinterface.cpp b/src/preferences/dialog/dlgprefinterface.cpp index 6867dcbf8bc5..e4ebb6ee94b0 100644 --- a/src/preferences/dialog/dlgprefinterface.cpp +++ b/src/preferences/dialog/dlgprefinterface.cpp @@ -122,8 +122,7 @@ DlgPrefInterface::DlgPrefInterface( ComboBoxSkinconf->setCurrentIndex(index); // schemes must be updated here to populate the drop-down box and set m_colorScheme slotUpdateSchemes(); - skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview( - m_skin.name(), m_colorScheme)); + skinPreviewLabel->setPixmap(m_skin.preview(m_colorScheme)); const auto* const pScreen = getScreen(); if (m_skin.fitsScreenSize(*pScreen)) { warningLabel->hide(); @@ -192,7 +191,7 @@ QScreen* DlgPrefInterface::getScreen() const { void DlgPrefInterface::slotUpdateSchemes() { // Re-populates the scheme combobox and attempts to pick the color scheme from config file. // Since this involves opening a file we won't do this as part of regular slotUpdate - QList schlist = m_skin.colorschemes(); + const QList schlist = m_skin.colorschemes(); ComboBoxSchemeconf->clear(); @@ -200,7 +199,7 @@ void DlgPrefInterface::slotUpdateSchemes() { ComboBoxSchemeconf->setEnabled(false); ComboBoxSchemeconf->addItem(tr("This skin does not support color schemes", nullptr)); ComboBoxSchemeconf->setCurrentIndex(0); - // clear m_colorScheme so that SkinLoader::getSkinPreview returns the correct preview + // clear m_colorScheme so that the correct skin preview is loaded m_colorScheme = QString(); } else { ComboBoxSchemeconf->setEnabled(true); @@ -336,7 +335,7 @@ void DlgPrefInterface::slotSetScheme(int) { m_colorScheme = newScheme; m_bRebootMixxxView = true; } - skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(m_skin.name(), m_colorScheme)); + skinPreviewLabel->setPixmap(m_skin.preview(m_colorScheme)); } void DlgPrefInterface::slotSetSkinDescription() { @@ -351,24 +350,25 @@ void DlgPrefInterface::slotSetSkinDescription() { void DlgPrefInterface::slotSetSkin(int) { QString newSkinName = ComboBoxSkinconf->currentText(); - if (newSkinName != m_skin.name()) { - const Skin newSkin = m_skins[newSkinName]; - VERIFY_OR_DEBUG_ASSERT(newSkin.isValid()) { - return; - } - m_skin = newSkin; - m_bRebootMixxxView = newSkinName != m_skinNameOnUpdate; - const auto* const pScreen = getScreen(); - if (pScreen && m_skin.fitsScreenSize(*pScreen)) { - warningLabel->hide(); - } else { - warningLabel->show(); - } - slotUpdateSchemes(); - slotSetSkinDescription(); + if (newSkinName == m_skin.name()) { + return; } - skinPreviewLabel->setPixmap(m_pSkinLoader->getSkinPreview(newSkinName, m_colorScheme)); + const Skin newSkin = m_skins[newSkinName]; + VERIFY_OR_DEBUG_ASSERT(newSkin.isValid()) { + return; + } + m_skin = newSkin; + m_bRebootMixxxView = newSkinName != m_skinNameOnUpdate; + const auto* const pScreen = getScreen(); + if (pScreen && m_skin.fitsScreenSize(*pScreen)) { + warningLabel->hide(); + } else { + warningLabel->show(); + } + slotUpdateSchemes(); + slotSetSkinDescription(); + skinPreviewLabel->setPixmap(m_skin.preview(m_colorScheme)); } void DlgPrefInterface::slotApply() { diff --git a/src/skin/legacy/skin.cpp b/src/skin/legacy/skin.cpp index 48465501beb4..07a18036362e 100644 --- a/src/skin/legacy/skin.cpp +++ b/src/skin/legacy/skin.cpp @@ -26,6 +26,24 @@ QFileInfo Skin::path() const { return m_path; } +QPixmap Skin::preview(const QString& schemeName = QString()) const { + QPixmap preview; + if (!schemeName.isEmpty()) { + QString schemeNameUnformatted = schemeName; + QString schemeNameFormatted = schemeNameUnformatted.replace(" ", ""); + preview.load(m_path.absoluteFilePath() + + QStringLiteral("/skin_preview_") + schemeNameFormatted + + QStringLiteral(".png")); + } else { + preview.load(m_path.absoluteFilePath() + QStringLiteral("/skin_preview.png")); + } + if (!preview.isNull()) { + return preview; + } + preview.load(":/images/skin_preview_placeholder.png"); + return preview; +} + QFileInfo Skin::skinFile() const { DEBUG_ASSERT(isValid()); return QFileInfo(path().absoluteFilePath() + QStringLiteral("/skin.xml")); diff --git a/src/skin/legacy/skin.h b/src/skin/legacy/skin.h index 0effc819b203..78ff1349f06f 100644 --- a/src/skin/legacy/skin.h +++ b/src/skin/legacy/skin.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -16,6 +17,7 @@ class Skin { bool isValid() const; QFileInfo path() const; + QPixmap preview(const QString& schemeName) const; QString name() const; QString description() const; diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index 3e14bd27e4da..045fd5db0998 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -73,22 +73,6 @@ QString SkinLoader::getSkinPath(const QString& skinName) const { return QString(); } -QPixmap SkinLoader::getSkinPreview(const QString& skinName, const QString& schemeName) const { - QPixmap preview; - if (!schemeName.isEmpty()) { - QString schemeNameUnformatted = schemeName; - QString schemeNameFormatted = schemeNameUnformatted.replace(" ",""); - preview.load(getSkinPath(skinName) + "/skin_preview_" + schemeNameFormatted + ".png"); - } else { - preview.load(getSkinPath(skinName) + "/skin_preview.png"); - } - if (!preview.isNull()){ - return preview; - } - preview.load(":/images/skin_preview_placeholder.png"); - return preview; -} - QString SkinLoader::getConfiguredSkinPath() const { QString configSkin = m_pConfig->getValueString(ConfigKey("[Config]", "ResizableSkin")); diff --git a/src/skin/skinloader.h b/src/skin/skinloader.h index 287ea9ae7faf..f284a091806d 100644 --- a/src/skin/skinloader.h +++ b/src/skin/skinloader.h @@ -36,7 +36,6 @@ class SkinLoader { LaunchImage* loadLaunchImage(QWidget* pParent); QString getSkinPath(const QString& skinName) const; - QPixmap getSkinPreview(const QString& skinName, const QString& schemeName) const; QString getConfiguredSkinPath() const; QString getDefaultSkinName() const; QList getSkinSearchPaths() const; From 3a8effa33c51450e84e7cd627770d8a68b19910b Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 23 May 2021 01:11:44 +0200 Subject: [PATCH 08/13] Skin: Add abstract base class for LegacySkin --- CMakeLists.txt | 2 +- src/preferences/dialog/dlgprefinterface.cpp | 47 ++++++++-------- src/preferences/dialog/dlgprefinterface.h | 6 +- src/skin/legacy/{skin.cpp => legacyskin.cpp} | 20 +++---- src/skin/legacy/legacyskin.h | 41 ++++++++++++++ src/skin/legacy/skin.h | 36 ------------ src/skin/skin.h | 37 ++++++++++++ src/skin/skinloader.cpp | 59 +++++++++++--------- src/skin/skinloader.h | 8 +-- 9 files changed, 153 insertions(+), 103 deletions(-) rename src/skin/legacy/{skin.cpp => legacyskin.cpp} (86%) create mode 100644 src/skin/legacy/legacyskin.h delete mode 100644 src/skin/legacy/skin.h create mode 100644 src/skin/skin.h diff --git a/CMakeLists.txt b/CMakeLists.txt index f39af56a4df6..5883132da4e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -732,7 +732,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/skin/legacy/launchimage.cpp src/skin/legacy/legacyskinparser.cpp src/skin/legacy/pixmapsource.cpp - src/skin/legacy/skin.cpp + src/skin/legacy/legacyskin.cpp src/skin/legacy/skincontext.cpp src/skin/legacy/tooltips.cpp src/skin/skinloader.cpp diff --git a/src/preferences/dialog/dlgprefinterface.cpp b/src/preferences/dialog/dlgprefinterface.cpp index e4ebb6ee94b0..12938374e8fd 100644 --- a/src/preferences/dialog/dlgprefinterface.cpp +++ b/src/preferences/dialog/dlgprefinterface.cpp @@ -15,13 +15,13 @@ #include "moc_dlgprefinterface.cpp" #include "preferences/usersettings.h" #include "skin/legacy/legacyskinparser.h" -#include "skin/legacy/skin.h" +#include "skin/skin.h" #include "skin/skinloader.h" #include "util/screensaver.h" #include "util/widgethelper.h" using mixxx::skin::SkinManifest; -using mixxx::skin::legacy::Skin; +using mixxx::skin::SkinPointer; DlgPrefInterface::DlgPrefInterface( QWidget* parent, @@ -32,7 +32,7 @@ DlgPrefInterface::DlgPrefInterface( m_pConfig(pConfig), m_mixxx(mixxx), m_pSkinLoader(pSkinLoader), - m_skin(QFileInfo(pSkinLoader->getConfiguredSkinPath())), + m_pSkin(pSkinLoader->getConfiguredSkin()), m_dScaleFactorAuto(1.0), m_bUseAutoScaleFactor(false), m_dScaleFactor(1.0), @@ -110,21 +110,20 @@ DlgPrefInterface::DlgPrefInterface( skinDescriptionText->setText(""); skinDescriptionText->hide(); - const QList skins = m_pSkinLoader->getSkins(); + const QList skins = m_pSkinLoader->getSkins(); int index = 0; - for (const Skin& skin : skins) { - ComboBoxSkinconf->insertItem(index, skin.name()); - m_skins.insert(skin.name(), skin); + for (const SkinPointer& pSkin : skins) { + ComboBoxSkinconf->insertItem(index, pSkin->name()); + m_skins.insert(pSkin->name(), pSkin); index++; } - qWarning() << m_skins.keys(); ComboBoxSkinconf->setCurrentIndex(index); // schemes must be updated here to populate the drop-down box and set m_colorScheme slotUpdateSchemes(); - skinPreviewLabel->setPixmap(m_skin.preview(m_colorScheme)); + skinPreviewLabel->setPixmap(m_pSkin->preview(m_colorScheme)); const auto* const pScreen = getScreen(); - if (m_skin.fitsScreenSize(*pScreen)) { + if (m_pSkin->fitsScreenSize(*pScreen)) { warningLabel->hide(); } else { warningLabel->show(); @@ -191,7 +190,7 @@ QScreen* DlgPrefInterface::getScreen() const { void DlgPrefInterface::slotUpdateSchemes() { // Re-populates the scheme combobox and attempts to pick the color scheme from config file. // Since this involves opening a file we won't do this as part of regular slotUpdate - const QList schlist = m_skin.colorschemes(); + const QList schlist = m_pSkin->colorschemes(); ComboBoxSchemeconf->clear(); @@ -227,9 +226,9 @@ void DlgPrefInterface::slotUpdateSchemes() { void DlgPrefInterface::slotUpdate() { const QString skinNameOnUpdate = m_pConfig->getValueString(ConfigKey("[Config]", "ResizableSkin")); - const Skin skinOnUpdate = m_skins[skinNameOnUpdate]; - if (skinOnUpdate.isValid()) { - m_skinNameOnUpdate = skinOnUpdate.name(); + const SkinPointer pSkinOnUpdate = m_skins[skinNameOnUpdate]; + if (pSkinOnUpdate != nullptr && pSkinOnUpdate->isValid()) { + m_skinNameOnUpdate = pSkinOnUpdate->name(); } else { m_skinNameOnUpdate = m_pSkinLoader->getDefaultSkinName(); } @@ -335,11 +334,11 @@ void DlgPrefInterface::slotSetScheme(int) { m_colorScheme = newScheme; m_bRebootMixxxView = true; } - skinPreviewLabel->setPixmap(m_skin.preview(m_colorScheme)); + skinPreviewLabel->setPixmap(m_pSkin->preview(m_colorScheme)); } void DlgPrefInterface::slotSetSkinDescription() { - const QString description = m_skin.description(); + const QString description = m_pSkin->description(); if (!description.isEmpty()) { skinDescriptionText->show(); skinDescriptionText->setText(description); @@ -350,29 +349,29 @@ void DlgPrefInterface::slotSetSkinDescription() { void DlgPrefInterface::slotSetSkin(int) { QString newSkinName = ComboBoxSkinconf->currentText(); - if (newSkinName == m_skin.name()) { + if (newSkinName == m_pSkin->name()) { return; } - const Skin newSkin = m_skins[newSkinName]; - VERIFY_OR_DEBUG_ASSERT(newSkin.isValid()) { + const SkinPointer pNewSkin = m_skins[newSkinName]; + VERIFY_OR_DEBUG_ASSERT(pNewSkin != nullptr && pNewSkin->isValid()) { return; } - m_skin = newSkin; + m_pSkin = pNewSkin; m_bRebootMixxxView = newSkinName != m_skinNameOnUpdate; const auto* const pScreen = getScreen(); - if (pScreen && m_skin.fitsScreenSize(*pScreen)) { + if (pScreen && m_pSkin->fitsScreenSize(*pScreen)) { warningLabel->hide(); } else { warningLabel->show(); } slotUpdateSchemes(); slotSetSkinDescription(); - skinPreviewLabel->setPixmap(m_skin.preview(m_colorScheme)); + skinPreviewLabel->setPixmap(m_pSkin->preview(m_colorScheme)); } void DlgPrefInterface::slotApply() { - m_pConfig->set(ConfigKey("[Config]", "ResizableSkin"), m_skin.name()); + m_pConfig->set(ConfigKey("[Config]", "ResizableSkin"), m_pSkin->name()); m_pConfig->set(ConfigKey("[Config]", "Scheme"), m_colorScheme); QString locale = ComboBoxLocale->itemData( @@ -411,7 +410,7 @@ void DlgPrefInterface::slotApply() { if (m_bRebootMixxxView) { m_mixxx->rebootMixxxView(); // Allow switching skins multiple times without closing the dialog - m_skinNameOnUpdate = m_skin.name(); + m_skinNameOnUpdate = m_pSkin->name(); } m_bRebootMixxxView = false; } diff --git a/src/preferences/dialog/dlgprefinterface.h b/src/preferences/dialog/dlgprefinterface.h index 12096af9e6c0..af1b55971e71 100644 --- a/src/preferences/dialog/dlgprefinterface.h +++ b/src/preferences/dialog/dlgprefinterface.h @@ -9,7 +9,7 @@ #include "preferences/dialog/dlgpreferencepage.h" #include "preferences/dialog/ui_dlgprefinterfacedlg.h" #include "preferences/usersettings.h" -#include "skin/legacy/skin.h" +#include "skin/skin.h" class ControlProxy; class ControlPotmeter; @@ -58,8 +58,8 @@ class DlgPrefInterface : public DlgPreferencePage, public Ui::DlgPrefControlsDlg MixxxMainWindow *m_mixxx; std::shared_ptr m_pSkinLoader; - QMap m_skins; - mixxx::skin::legacy::Skin m_skin; + QMap m_skins; + mixxx::skin::SkinPointer m_pSkin; QString m_skinNameOnUpdate; QString m_colorScheme; QString m_localeOnUpdate; diff --git a/src/skin/legacy/skin.cpp b/src/skin/legacy/legacyskin.cpp similarity index 86% rename from src/skin/legacy/skin.cpp rename to src/skin/legacy/legacyskin.cpp index 07a18036362e..4362b3a38da8 100644 --- a/src/skin/legacy/skin.cpp +++ b/src/skin/legacy/legacyskin.cpp @@ -1,4 +1,4 @@ -#include "skin/legacy/skin.h" +#include "skin/legacy/legacyskin.h" #include "skin/legacy/legacyskinparser.h" @@ -12,21 +12,21 @@ namespace mixxx { namespace skin { namespace legacy { -Skin::Skin(const QFileInfo& path) +LegacySkin::LegacySkin(const QFileInfo& path) : m_path(path) { DEBUG_ASSERT(isValid()); } -bool Skin::isValid() const { +bool LegacySkin::isValid() const { return !m_path.filePath().isEmpty() && m_path.exists(); } -QFileInfo Skin::path() const { +QFileInfo LegacySkin::path() const { DEBUG_ASSERT(isValid()); return m_path; } -QPixmap Skin::preview(const QString& schemeName = QString()) const { +QPixmap LegacySkin::preview(const QString& schemeName = QString()) const { QPixmap preview; if (!schemeName.isEmpty()) { QString schemeNameUnformatted = schemeName; @@ -44,22 +44,22 @@ QPixmap Skin::preview(const QString& schemeName = QString()) const { return preview; } -QFileInfo Skin::skinFile() const { +QFileInfo LegacySkin::skinFile() const { DEBUG_ASSERT(isValid()); return QFileInfo(path().absoluteFilePath() + QStringLiteral("/skin.xml")); } -QString Skin::name() const { +QString LegacySkin::name() const { DEBUG_ASSERT(isValid()); return m_path.fileName(); } -QList Skin::colorschemes() const { +QList LegacySkin::colorschemes() const { DEBUG_ASSERT(isValid()); return LegacySkinParser::getSchemeList(path().absoluteFilePath()); } -QString Skin::description() const { +QString LegacySkin::description() const { DEBUG_ASSERT(isValid()); SkinManifest manifest = LegacySkinParser::getSkinManifest( LegacySkinParser::openSkin(path().absoluteFilePath())); @@ -70,7 +70,7 @@ QString Skin::description() const { return description; } -bool Skin::fitsScreenSize(const QScreen& screen) const { +bool LegacySkin::fitsScreenSize(const QScreen& screen) const { DEBUG_ASSERT(isValid()); // Use the full resolution of the entire screen that is // available in full-screen mode. diff --git a/src/skin/legacy/legacyskin.h b/src/skin/legacy/legacyskin.h new file mode 100644 index 000000000000..4576a98de8b2 --- /dev/null +++ b/src/skin/legacy/legacyskin.h @@ -0,0 +1,41 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include "skin/skin.h" + +namespace mixxx { +namespace skin { +namespace legacy { + +class LegacySkin : public mixxx::skin::Skin { + public: + LegacySkin() = default; + LegacySkin(const QFileInfo& path); + + mixxx::skin::SkinType type() const override { + return mixxx::skin::SkinType::Legacy; + }; + bool isValid() const override; + QFileInfo path() const override; + QPixmap preview(const QString& schemeName) const override; + + QString name() const override; + QString description() const override; + QList colorschemes() const override; + + bool fitsScreenSize(const QScreen& screen) const override; + + private: + QFileInfo skinFile() const; + + QFileInfo m_path; +}; + +} // namespace legacy +} // namespace skin +} // namespace mixxx diff --git a/src/skin/legacy/skin.h b/src/skin/legacy/skin.h deleted file mode 100644 index 78ff1349f06f..000000000000 --- a/src/skin/legacy/skin.h +++ /dev/null @@ -1,36 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include - -namespace mixxx { -namespace skin { -namespace legacy { - -class Skin { - public: - Skin() = default; - Skin(const QFileInfo& path); - - bool isValid() const; - QFileInfo path() const; - QPixmap preview(const QString& schemeName) const; - - QString name() const; - QString description() const; - QList colorschemes() const; - - bool fitsScreenSize(const QScreen& screen) const; - - private: - QFileInfo skinFile() const; - - QFileInfo m_path; -}; - -} // namespace legacy -} // namespace skin -} // namespace mixxx diff --git a/src/skin/skin.h b/src/skin/skin.h new file mode 100644 index 000000000000..91d2c8b65a0b --- /dev/null +++ b/src/skin/skin.h @@ -0,0 +1,37 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +namespace mixxx { +namespace skin { + +enum class SkinType { + Legacy, +}; + +class Skin { + public: + virtual ~Skin() = default; + + virtual SkinType type() const = 0; + + virtual bool isValid() const = 0; + virtual QFileInfo path() const = 0; + virtual QPixmap preview(const QString& schemeName) const = 0; + + virtual QString name() const = 0; + virtual QString description() const = 0; + virtual QList colorschemes() const = 0; + + virtual bool fitsScreenSize(const QScreen& screen) const = 0; +}; + +typedef std::shared_ptr SkinPointer; + +} // namespace skin +} // namespace mixxx diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index 045fd5db0998..e4539f2df871 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -5,18 +5,20 @@ #include #include -#include "vinylcontrol/vinylcontrolmanager.h" -#include "skin/legacy/legacyskinparser.h" #include "controllers/controllermanager.h" -#include "library/library.h" #include "effects/effectsmanager.h" +#include "library/library.h" #include "mixer/playermanager.h" -#include "util/debug.h" +#include "recording/recordingmanager.h" #include "skin/legacy/launchimage.h" +#include "skin/legacy/legacyskin.h" +#include "skin/legacy/legacyskinparser.h" +#include "util/debug.h" #include "util/timer.h" -#include "recording/recordingmanager.h" +#include "vinylcontrol/vinylcontrolmanager.h" -using mixxx::skin::legacy::Skin; +using mixxx::skin::SkinPointer; +using mixxx::skin::legacy::LegacySkin; SkinLoader::SkinLoader(UserSettingsPointer pConfig) : m_pConfig(pConfig) { @@ -26,15 +28,15 @@ SkinLoader::~SkinLoader() { LegacySkinParser::clearSharedGroupStrings(); } -QList SkinLoader::getSkins() const { +QList SkinLoader::getSkins() const { const QList skinSearchPaths = getSkinSearchPaths(); - QList skins; + QList skins; for (const QDir& dir : skinSearchPaths) { const QList fileInfos = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot); for (const QFileInfo& fileInfo : fileInfos) { QDir skinDir(fileInfo.absoluteFilePath()); if (skinDir.exists(QStringLiteral("skin.xml"))) { - skins.append(Skin(fileInfo)); + skins.append(std::make_shared(fileInfo)); } } } @@ -63,17 +65,19 @@ QList SkinLoader::getSkinSearchPaths() const { return searchPaths; } -QString SkinLoader::getSkinPath(const QString& skinName) const { +SkinPointer SkinLoader::getSkin(const QString& skinName) const { const QList skinSearchPaths = getSkinSearchPaths(); for (QDir dir : skinSearchPaths) { if (dir.cd(skinName)) { - return dir.absolutePath(); + if (dir.exists("skin.xml")) { + return std::make_shared(QFileInfo(dir.absolutePath())); + } } } - return QString(); + return nullptr; } -QString SkinLoader::getConfiguredSkinPath() const { +SkinPointer SkinLoader::getConfiguredSkin() const { QString configSkin = m_pConfig->getValueString(ConfigKey("[Config]", "ResizableSkin")); // If we don't have a skin defined, we might be migrating from 1.11 and @@ -90,14 +94,15 @@ QString SkinLoader::getConfiguredSkinPath() const { } } - QString skinPath = getSkinPath(configSkin); + SkinPointer pSkin = getSkin(configSkin); - if (skinPath.isEmpty()) { - skinPath = getSkinPath(getDefaultSkinName()); - qDebug() << "Could not find the user's configured skin." - << "Falling back on the default skin:" << skinPath; + if (pSkin == nullptr || !pSkin->isValid()) { + const QString defaultSkinName = getDefaultSkinName(); + pSkin = getSkin(defaultSkinName); + qWarning() << "Could not find the user's configured skin." + << "Falling back on the default skin:" << defaultSkinName; } - return skinPath; + return pSkin; } QString SkinLoader::getDefaultSkinName() const { @@ -114,10 +119,10 @@ QWidget* SkinLoader::loadConfiguredSkin(QWidget* pParent, EffectsManager* pEffectsManager, RecordingManager* pRecordingManager) { ScopedTimer timer("SkinLoader::loadConfiguredSkin"); - QString skinPath = getConfiguredSkinPath(); + SkinPointer pSkin = getConfiguredSkin(); - // If we don't have a skin path then fail. - if (skinPath.isEmpty()) { + // If we don't have a skin then fail. + VERIFY_OR_DEBUG_ASSERT(pSkin != nullptr && pSkin->isValid()) { return nullptr; } @@ -130,13 +135,17 @@ QWidget* SkinLoader::loadConfiguredSkin(QWidget* pParent, pVCMan, pEffectsManager, pRecordingManager); - return legacy.parseSkin(skinPath, pParent); + return legacy.parseSkin(pSkin->path().absoluteFilePath(), pParent); } LaunchImage* SkinLoader::loadLaunchImage(QWidget* pParent) { - QString skinPath = getConfiguredSkinPath(); + SkinPointer pSkin = getConfiguredSkin(); + VERIFY_OR_DEBUG_ASSERT(pSkin != nullptr && pSkin->isValid()) { + return nullptr; + } + LegacySkinParser parser(m_pConfig); - LaunchImage* pLaunchImage = parser.parseLaunchImage(skinPath, pParent); + LaunchImage* pLaunchImage = parser.parseLaunchImage(pSkin->path().absoluteFilePath(), pParent); if (pLaunchImage == nullptr) { // Construct default LaunchImage pLaunchImage = new LaunchImage(pParent, QString()); diff --git a/src/skin/skinloader.h b/src/skin/skinloader.h index f284a091806d..910b318b4e1c 100644 --- a/src/skin/skinloader.h +++ b/src/skin/skinloader.h @@ -6,7 +6,7 @@ #include #include "preferences/usersettings.h" -#include "skin/legacy/skin.h" +#include "skin/skin.h" class KeyboardEventFilter; class PlayerManager; @@ -35,11 +35,11 @@ class SkinLoader { LaunchImage* loadLaunchImage(QWidget* pParent); - QString getSkinPath(const QString& skinName) const; - QString getConfiguredSkinPath() const; + mixxx::skin::SkinPointer getSkin(const QString& skinName) const; + mixxx::skin::SkinPointer getConfiguredSkin() const; QString getDefaultSkinName() const; QList getSkinSearchPaths() const; - QList getSkins() const; + QList getSkins() const; private: QString pickResizableSkin(const QString& oldSkin) const; From d50070a4df3e71d13a287bbef30b5780b503e853 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 24 May 2021 12:46:32 +0200 Subject: [PATCH 09/13] SkinLoader: Pass CoreServices to loadConfiguredSkin --- src/mixxx.cpp | 8 +------- src/skin/skinloader.cpp | 23 +++++++++-------------- src/skin/skinloader.h | 18 ++++-------------- 3 files changed, 14 insertions(+), 35 deletions(-) diff --git a/src/mixxx.cpp b/src/mixxx.cpp index 711cd8454bd5..573f3ca076a9 100644 --- a/src/mixxx.cpp +++ b/src/mixxx.cpp @@ -1090,13 +1090,7 @@ bool MixxxMainWindow::loadConfiguredSkin() { // TODO: use std::shared_ptr throughout skin widgets instead of these hacky get() calls m_pCentralWidget = m_pSkinLoader->loadConfiguredSkin(this, &m_skinCreatedControls, - m_pCoreServices->getKeyboardEventFilter().get(), - m_pCoreServices->getPlayerManager().get(), - m_pCoreServices->getControllerManager().get(), - m_pCoreServices->getLibrary().get(), - m_pCoreServices->getVinylControlManager().get(), - m_pCoreServices->getEffectsManager().get(), - m_pCoreServices->getRecordingManager().get()); + m_pCoreServices.get()); if (centralWidget() == m_pLaunchImage) { initializationProgressUpdate(100, ""); } diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index e4539f2df871..cb61dc68a6c0 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -6,6 +6,7 @@ #include #include "controllers/controllermanager.h" +#include "coreservices.h" #include "effects/effectsmanager.h" #include "library/library.h" #include "mixer/playermanager.h" @@ -111,13 +112,7 @@ QString SkinLoader::getDefaultSkinName() const { QWidget* SkinLoader::loadConfiguredSkin(QWidget* pParent, QSet* pSkinCreatedControls, - KeyboardEventFilter* pKeyboard, - PlayerManager* pPlayerManager, - ControllerManager* pControllerManager, - Library* pLibrary, - VinylControlManager* pVCMan, - EffectsManager* pEffectsManager, - RecordingManager* pRecordingManager) { + mixxx::CoreServices* pCoreServices) { ScopedTimer timer("SkinLoader::loadConfiguredSkin"); SkinPointer pSkin = getConfiguredSkin(); @@ -128,13 +123,13 @@ QWidget* SkinLoader::loadConfiguredSkin(QWidget* pParent, LegacySkinParser legacy(m_pConfig, pSkinCreatedControls, - pKeyboard, - pPlayerManager, - pControllerManager, - pLibrary, - pVCMan, - pEffectsManager, - pRecordingManager); + pCoreServices->getKeyboardEventFilter().get(), + pCoreServices->getPlayerManager().get(), + pCoreServices->getControllerManager().get(), + pCoreServices->getLibrary().get(), + pCoreServices->getVinylControlManager().get(), + pCoreServices->getEffectsManager().get(), + pCoreServices->getRecordingManager().get()); return legacy.parseSkin(pSkin->path().absoluteFilePath(), pParent); } diff --git a/src/skin/skinloader.h b/src/skin/skinloader.h index 910b318b4e1c..a15f0d020a25 100644 --- a/src/skin/skinloader.h +++ b/src/skin/skinloader.h @@ -8,15 +8,11 @@ #include "preferences/usersettings.h" #include "skin/skin.h" -class KeyboardEventFilter; -class PlayerManager; -class ControllerManager; class ControlObject; -class Library; -class VinylControlManager; -class EffectsManager; -class RecordingManager; class LaunchImage; +namespace mixxx { +class CoreServices; +} class SkinLoader { public: @@ -25,13 +21,7 @@ class SkinLoader { QWidget* loadConfiguredSkin(QWidget* pParent, QSet* skinCreatedControls, - KeyboardEventFilter* pKeyboard, - PlayerManager* pPlayerManager, - ControllerManager* pControllerManager, - Library* pLibrary, - VinylControlManager* pVCMan, - EffectsManager* pEffectsManager, - RecordingManager* pRecordingManager); + mixxx::CoreServices* pCoreServices); LaunchImage* loadLaunchImage(QWidget* pParent); From 728fcbd948b11e5fe567147356766623155a96dc Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 24 May 2021 13:01:53 +0200 Subject: [PATCH 10/13] SkinLoader: Move actual loader implementation into LegacySkin class --- src/skin/legacy/legacyskin.cpp | 29 +++++++++++++++++++++++++++++ src/skin/legacy/legacyskin.h | 5 +++++ src/skin/skin.h | 16 ++++++++++++++++ src/skin/skinloader.cpp | 18 ++++-------------- src/skin/skinloader.h | 8 +------- 5 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/skin/legacy/legacyskin.cpp b/src/skin/legacy/legacyskin.cpp index 4362b3a38da8..d59e9861a183 100644 --- a/src/skin/legacy/legacyskin.cpp +++ b/src/skin/legacy/legacyskin.cpp @@ -1,5 +1,6 @@ #include "skin/legacy/legacyskin.h" +#include "coreservices.h" #include "skin/legacy/legacyskinparser.h" namespace { @@ -101,6 +102,34 @@ bool LegacySkin::fitsScreenSize(const QScreen& screen) const { skinHeight.toInt() <= screenSize.height(); } +LaunchImage* LegacySkin::loadLaunchImage(QWidget* pParent, UserSettingsPointer pConfig) const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return nullptr; + } + LegacySkinParser parser(pConfig); + LaunchImage* pLaunchImage = parser.parseLaunchImage(m_path.absoluteFilePath(), pParent); + return pLaunchImage; +} + +QWidget* LegacySkin::loadSkin(QWidget* pParent, + UserSettingsPointer pConfig, + QSet* pSkinCreatedControls, + mixxx::CoreServices* pCoreServices) const { + VERIFY_OR_DEBUG_ASSERT(isValid()) { + return nullptr; + } + LegacySkinParser legacy(pConfig, + pSkinCreatedControls, + pCoreServices->getKeyboardEventFilter().get(), + pCoreServices->getPlayerManager().get(), + pCoreServices->getControllerManager().get(), + pCoreServices->getLibrary().get(), + pCoreServices->getVinylControlManager().get(), + pCoreServices->getEffectsManager().get(), + pCoreServices->getRecordingManager().get()); + return legacy.parseSkin(m_path.absoluteFilePath(), pParent); +} + } // namespace legacy } // namespace skin } // namespace mixxx diff --git a/src/skin/legacy/legacyskin.h b/src/skin/legacy/legacyskin.h index 4576a98de8b2..0bfb760689f2 100644 --- a/src/skin/legacy/legacyskin.h +++ b/src/skin/legacy/legacyskin.h @@ -29,6 +29,11 @@ class LegacySkin : public mixxx::skin::Skin { QList colorschemes() const override; bool fitsScreenSize(const QScreen& screen) const override; + LaunchImage* loadLaunchImage(QWidget* pParent, UserSettingsPointer pConfig) const override; + QWidget* loadSkin(QWidget* pParent, + UserSettingsPointer pConfig, + QSet* pSkinCreatedControls, + mixxx::CoreServices* pCoreServices) const override; private: QFileInfo skinFile() const; diff --git a/src/skin/skin.h b/src/skin/skin.h index 91d2c8b65a0b..386be45d95c5 100644 --- a/src/skin/skin.h +++ b/src/skin/skin.h @@ -4,10 +4,20 @@ #include #include #include +#include #include +#include #include +#include "preferences/usersettings.h" + +class ControlObject; +class LaunchImage; + namespace mixxx { + +class CoreServices; + namespace skin { enum class SkinType { @@ -29,6 +39,12 @@ class Skin { virtual QList colorschemes() const = 0; virtual bool fitsScreenSize(const QScreen& screen) const = 0; + + virtual LaunchImage* loadLaunchImage(QWidget* pParent, UserSettingsPointer pConfig) const = 0; + virtual QWidget* loadSkin(QWidget* pParent, + UserSettingsPointer pConfig, + QSet* pSkinCreatedControls, + mixxx::CoreServices* pCoreServices) const = 0; }; typedef std::shared_ptr SkinPointer; diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index cb61dc68a6c0..493928324cee 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -6,7 +6,6 @@ #include #include "controllers/controllermanager.h" -#include "coreservices.h" #include "effects/effectsmanager.h" #include "library/library.h" #include "mixer/playermanager.h" @@ -121,30 +120,21 @@ QWidget* SkinLoader::loadConfiguredSkin(QWidget* pParent, return nullptr; } - LegacySkinParser legacy(m_pConfig, - pSkinCreatedControls, - pCoreServices->getKeyboardEventFilter().get(), - pCoreServices->getPlayerManager().get(), - pCoreServices->getControllerManager().get(), - pCoreServices->getLibrary().get(), - pCoreServices->getVinylControlManager().get(), - pCoreServices->getEffectsManager().get(), - pCoreServices->getRecordingManager().get()); - return legacy.parseSkin(pSkin->path().absoluteFilePath(), pParent); + return pSkin->loadSkin(pParent, m_pConfig, pSkinCreatedControls, pCoreServices); } -LaunchImage* SkinLoader::loadLaunchImage(QWidget* pParent) { +LaunchImage* SkinLoader::loadLaunchImage(QWidget* pParent) const { SkinPointer pSkin = getConfiguredSkin(); VERIFY_OR_DEBUG_ASSERT(pSkin != nullptr && pSkin->isValid()) { return nullptr; } - LegacySkinParser parser(m_pConfig); - LaunchImage* pLaunchImage = parser.parseLaunchImage(pSkin->path().absoluteFilePath(), pParent); + LaunchImage* pLaunchImage = pSkin->loadLaunchImage(pParent, m_pConfig); if (pLaunchImage == nullptr) { // Construct default LaunchImage pLaunchImage = new LaunchImage(pParent, QString()); } + return pLaunchImage; } diff --git a/src/skin/skinloader.h b/src/skin/skinloader.h index a15f0d020a25..51ef701fd2eb 100644 --- a/src/skin/skinloader.h +++ b/src/skin/skinloader.h @@ -8,12 +8,6 @@ #include "preferences/usersettings.h" #include "skin/skin.h" -class ControlObject; -class LaunchImage; -namespace mixxx { -class CoreServices; -} - class SkinLoader { public: SkinLoader(UserSettingsPointer pConfig); @@ -23,7 +17,7 @@ class SkinLoader { QSet* skinCreatedControls, mixxx::CoreServices* pCoreServices); - LaunchImage* loadLaunchImage(QWidget* pParent); + LaunchImage* loadLaunchImage(QWidget* pParent) const; mixxx::skin::SkinPointer getSkin(const QString& skinName) const; mixxx::skin::SkinPointer getConfiguredSkin() const; From 649feaf5c1d47f566f40776d6f859b44fb500c9f Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 24 May 2021 13:21:45 +0200 Subject: [PATCH 11/13] SkinLoader: Move more legacy skin internals out of SkinLoader class --- src/skin/legacy/legacyskin.cpp | 11 ++++++++++- src/skin/legacy/legacyskin.h | 3 +++ src/skin/skinloader.cpp | 25 +++++++++++++++++++++---- src/skin/skinloader.h | 1 + 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/skin/legacy/legacyskin.cpp b/src/skin/legacy/legacyskin.cpp index d59e9861a183..8880b9faf5cb 100644 --- a/src/skin/legacy/legacyskin.cpp +++ b/src/skin/legacy/legacyskin.cpp @@ -6,6 +6,7 @@ namespace { const QRegExp kMinSizeRegExp("(\\d+), *(\\d+)<"); +const QString kSkinManifestFileName(QStringLiteral("skin.xml")); } // namespace @@ -13,6 +14,14 @@ namespace mixxx { namespace skin { namespace legacy { +// static +SkinPointer LegacySkin::fromDirectory(const QDir& dir) { + if (dir.exists(kSkinManifestFileName)) { + return std::make_shared(QFileInfo(dir.absolutePath())); + } + return nullptr; +} + LegacySkin::LegacySkin(const QFileInfo& path) : m_path(path) { DEBUG_ASSERT(isValid()); @@ -47,7 +56,7 @@ QPixmap LegacySkin::preview(const QString& schemeName = QString()) const { QFileInfo LegacySkin::skinFile() const { DEBUG_ASSERT(isValid()); - return QFileInfo(path().absoluteFilePath() + QStringLiteral("/skin.xml")); + return QFileInfo(path().absoluteFilePath() + QStringLiteral("/") + kSkinManifestFileName); } QString LegacySkin::name() const { diff --git a/src/skin/legacy/legacyskin.h b/src/skin/legacy/legacyskin.h index 0bfb760689f2..bd011813cd99 100644 --- a/src/skin/legacy/legacyskin.h +++ b/src/skin/legacy/legacyskin.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -17,6 +18,8 @@ class LegacySkin : public mixxx::skin::Skin { LegacySkin() = default; LegacySkin(const QFileInfo& path); + static SkinPointer fromDirectory(const QDir& dir); + mixxx::skin::SkinType type() const override { return mixxx::skin::SkinType::Legacy; }; diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index 493928324cee..8b96aafdfe58 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -35,8 +35,12 @@ QList SkinLoader::getSkins() const { const QList fileInfos = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot); for (const QFileInfo& fileInfo : fileInfos) { QDir skinDir(fileInfo.absoluteFilePath()); - if (skinDir.exists(QStringLiteral("skin.xml"))) { - skins.append(std::make_shared(fileInfo)); + SkinPointer pSkin = skinFromDirectory(skinDir); + if (pSkin) { + VERIFY_OR_DEBUG_ASSERT(pSkin->isValid()) { + continue; + } + skins.append(pSkin); } } } @@ -69,8 +73,12 @@ SkinPointer SkinLoader::getSkin(const QString& skinName) const { const QList skinSearchPaths = getSkinSearchPaths(); for (QDir dir : skinSearchPaths) { if (dir.cd(skinName)) { - if (dir.exists("skin.xml")) { - return std::make_shared(QFileInfo(dir.absolutePath())); + SkinPointer pSkin = skinFromDirectory(dir); + if (pSkin) { + VERIFY_OR_DEBUG_ASSERT(pSkin->isValid()) { + continue; + } + return pSkin; } } } @@ -150,3 +158,12 @@ QString SkinLoader::pickResizableSkin(const QString& oldSkin) const { } return QString(); } + +SkinPointer SkinLoader::skinFromDirectory(const QDir& dir) const { + SkinPointer pSkin = LegacySkin::fromDirectory(dir); + if (pSkin && pSkin->isValid()) { + return pSkin; + } + + return nullptr; +} diff --git a/src/skin/skinloader.h b/src/skin/skinloader.h index 51ef701fd2eb..46130d3d888e 100644 --- a/src/skin/skinloader.h +++ b/src/skin/skinloader.h @@ -27,6 +27,7 @@ class SkinLoader { private: QString pickResizableSkin(const QString& oldSkin) const; + mixxx::skin::SkinPointer skinFromDirectory(const QDir& dir) const; UserSettingsPointer m_pConfig; }; From 512ca676f8bf2d76fe2d64cf45e74088c7ed66bf Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 24 May 2021 13:37:12 +0200 Subject: [PATCH 12/13] SkinLoader: Move class into mixxx::skin:: namespace --- src/mixxx.cpp | 2 +- src/mixxx.h | 9 +++++++-- src/preferences/dialog/dlgpreferences.cpp | 2 +- src/preferences/dialog/dlgpreferences.h | 9 +++++++-- src/preferences/dialog/dlgprefinterface.cpp | 2 +- src/preferences/dialog/dlgprefinterface.h | 11 ++++++++--- src/skin/skinloader.cpp | 9 +++++++-- src/skin/skinloader.h | 14 ++++++++++---- 8 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/mixxx.cpp b/src/mixxx.cpp index 573f3ca076a9..c6790aa29b47 100644 --- a/src/mixxx.cpp +++ b/src/mixxx.cpp @@ -113,7 +113,7 @@ MixxxMainWindow::MixxxMainWindow( initializeWindow(); // Show launch image immediately so the user knows Mixxx is starting - m_pSkinLoader = std::make_unique(m_pCoreServices->getSettings()); + m_pSkinLoader = std::make_unique(m_pCoreServices->getSettings()); m_pLaunchImage = m_pSkinLoader->loadLaunchImage(this); m_pCentralWidget = (QWidget*)m_pLaunchImage; setCentralWidget(m_pCentralWidget); diff --git a/src/mixxx.h b/src/mixxx.h index 6016430bf306..a593abb5abe2 100644 --- a/src/mixxx.h +++ b/src/mixxx.h @@ -25,10 +25,15 @@ class EngineMaster; class GuiTick; class LaunchImage; class Library; -class SkinLoader; class VisualsManager; class WMainMenuBar; +namespace mixxx { +namespace skin { +class SkinLoader; +} +} // namespace mixxx + #ifdef __ENGINEPRIME__ namespace mixxx { class LibraryExporter; @@ -116,7 +121,7 @@ class MixxxMainWindow : public QMainWindow { QWidget* m_pCentralWidget; LaunchImage* m_pLaunchImage; - std::shared_ptr m_pSkinLoader; + std::shared_ptr m_pSkinLoader; GuiTick* m_pGuiTick; VisualsManager* m_pVisualsManager; diff --git a/src/preferences/dialog/dlgpreferences.cpp b/src/preferences/dialog/dlgpreferences.cpp index 0da47b1a76d2..24e802743409 100644 --- a/src/preferences/dialog/dlgpreferences.cpp +++ b/src/preferences/dialog/dlgpreferences.cpp @@ -53,7 +53,7 @@ DlgPreferences::DlgPreferences( MixxxMainWindow* mixxx, - std::shared_ptr pSkinLoader, + std::shared_ptr pSkinLoader, std::shared_ptr pSoundManager, std::shared_ptr pPlayerManager, std::shared_ptr pControllerManager, diff --git a/src/preferences/dialog/dlgpreferences.h b/src/preferences/dialog/dlgpreferences.h index 4362bbe2cdb2..e48a28b38536 100644 --- a/src/preferences/dialog/dlgpreferences.h +++ b/src/preferences/dialog/dlgpreferences.h @@ -43,7 +43,6 @@ class DlgPrefLV2; class LV2Backend; class ControllerManager; class EffectsManager; -class SkinLoader; class PlayerManager; class Library; class VinylControlManager; @@ -51,6 +50,12 @@ class VinylControlManager; class DlgPrefModplug; #endif // __MODPLUG__ +namespace mixxx { +namespace skin { +class SkinLoader; +} +} // namespace mixxx + class DlgPreferences : public QDialog, public Ui::DlgPreferencesDlg { Q_OBJECT public: @@ -66,7 +71,7 @@ class DlgPreferences : public QDialog, public Ui::DlgPreferencesDlg { }; DlgPreferences(MixxxMainWindow* mixxx, - std::shared_ptr pSkinLoader, + std::shared_ptr pSkinLoader, std::shared_ptr pSoundManager, std::shared_ptr pPlayerManager, std::shared_ptr pControllerManager, diff --git a/src/preferences/dialog/dlgprefinterface.cpp b/src/preferences/dialog/dlgprefinterface.cpp index 12938374e8fd..af4f2093b280 100644 --- a/src/preferences/dialog/dlgprefinterface.cpp +++ b/src/preferences/dialog/dlgprefinterface.cpp @@ -26,7 +26,7 @@ using mixxx::skin::SkinPointer; DlgPrefInterface::DlgPrefInterface( QWidget* parent, MixxxMainWindow* mixxx, - std::shared_ptr pSkinLoader, + std::shared_ptr pSkinLoader, UserSettingsPointer pConfig) : DlgPreferencePage(parent), m_pConfig(pConfig), diff --git a/src/preferences/dialog/dlgprefinterface.h b/src/preferences/dialog/dlgprefinterface.h index af1b55971e71..d7004d46ea04 100644 --- a/src/preferences/dialog/dlgprefinterface.h +++ b/src/preferences/dialog/dlgprefinterface.h @@ -13,18 +13,23 @@ class ControlProxy; class ControlPotmeter; -class SkinLoader; class PlayerManager; class MixxxMainWindow; class ControlObject; +namespace mixxx { +namespace skin { +class SkinLoader; +} +} // namespace mixxx + class DlgPrefInterface : public DlgPreferencePage, public Ui::DlgPrefControlsDlg { Q_OBJECT public: DlgPrefInterface( QWidget* parent, MixxxMainWindow* mixxx, - std::shared_ptr pSkinLoader, + std::shared_ptr pSkinLoader, UserSettingsPointer pConfig); ~DlgPrefInterface() override = default; @@ -56,7 +61,7 @@ class DlgPrefInterface : public DlgPreferencePage, public Ui::DlgPrefControlsDlg UserSettingsPointer m_pConfig; ControlObject* m_pControlTrackTimeDisplay; MixxxMainWindow *m_mixxx; - std::shared_ptr m_pSkinLoader; + std::shared_ptr m_pSkinLoader; QMap m_skins; mixxx::skin::SkinPointer m_pSkin; diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index 8b96aafdfe58..86b444e32316 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -17,8 +17,10 @@ #include "util/timer.h" #include "vinylcontrol/vinylcontrolmanager.h" -using mixxx::skin::SkinPointer; -using mixxx::skin::legacy::LegacySkin; +namespace mixxx { +namespace skin { + +using legacy::LegacySkin; SkinLoader::SkinLoader(UserSettingsPointer pConfig) : m_pConfig(pConfig) { @@ -167,3 +169,6 @@ SkinPointer SkinLoader::skinFromDirectory(const QDir& dir) const { return nullptr; } + +} // namespace skin +} // namespace mixxx diff --git a/src/skin/skinloader.h b/src/skin/skinloader.h index 46130d3d888e..e392e5203e80 100644 --- a/src/skin/skinloader.h +++ b/src/skin/skinloader.h @@ -8,6 +8,9 @@ #include "preferences/usersettings.h" #include "skin/skin.h" +namespace mixxx { +namespace skin { + class SkinLoader { public: SkinLoader(UserSettingsPointer pConfig); @@ -19,15 +22,18 @@ class SkinLoader { LaunchImage* loadLaunchImage(QWidget* pParent) const; - mixxx::skin::SkinPointer getSkin(const QString& skinName) const; - mixxx::skin::SkinPointer getConfiguredSkin() const; + SkinPointer getSkin(const QString& skinName) const; + SkinPointer getConfiguredSkin() const; QString getDefaultSkinName() const; QList getSkinSearchPaths() const; - QList getSkins() const; + QList getSkins() const; private: QString pickResizableSkin(const QString& oldSkin) const; - mixxx::skin::SkinPointer skinFromDirectory(const QDir& dir) const; + SkinPointer skinFromDirectory(const QDir& dir) const; UserSettingsPointer m_pConfig; }; + +} // namespace skin +} // namespace mixxx From b94d0eed142b43f5d9002f6d73dfb144ae057988 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 25 May 2021 17:10:10 +0200 Subject: [PATCH 13/13] SkinLoader: Improve error handling (add fallback if skin is broken) --- src/skin/skinloader.cpp | 47 +++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index 86b444e32316..a4754ff699cc 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -97,11 +97,6 @@ SkinPointer SkinLoader::getConfiguredSkin() const { if (!oldSkin.isEmpty()) { configSkin = pickResizableSkin(oldSkin); } - // If the old skin was empty or we couldn't guess a skin, go with the - // default. - if (configSkin.isEmpty()) { - configSkin = getDefaultSkinName(); - } } SkinPointer pSkin = getSkin(configSkin); @@ -109,8 +104,9 @@ SkinPointer SkinLoader::getConfiguredSkin() const { if (pSkin == nullptr || !pSkin->isValid()) { const QString defaultSkinName = getDefaultSkinName(); pSkin = getSkin(defaultSkinName); - qWarning() << "Could not find the user's configured skin." - << "Falling back on the default skin:" << defaultSkinName; + qWarning() << "Configured skin" << configSkin + << "not found, falling back to default skin" + << defaultSkinName; } return pSkin; } @@ -125,12 +121,45 @@ QWidget* SkinLoader::loadConfiguredSkin(QWidget* pParent, ScopedTimer timer("SkinLoader::loadConfiguredSkin"); SkinPointer pSkin = getConfiguredSkin(); - // If we don't have a skin then fail. + // If we don't have a skin then fail. This makes sense here, because the + // method above already tried to fall back to the default skin if the + // configured one is not available. If `pSkin` is nullptr, we both the + // configured and the default skin were not found, so there is nothing we + // can do. VERIFY_OR_DEBUG_ASSERT(pSkin != nullptr && pSkin->isValid()) { return nullptr; } - return pSkin->loadSkin(pParent, m_pConfig, pSkinCreatedControls, pCoreServices); + QWidget* pLoadedSkin = pSkin->loadSkin(pParent, m_pConfig, pSkinCreatedControls, pCoreServices); + + // If the skin exists but failed to load, try to fall back to the default skin. + if (pLoadedSkin == nullptr) { + const QString defaultSkinName = getDefaultSkinName(); + if (defaultSkinName == pSkin->name()) { + qCritical() << "Configured skin " << pSkin->name() + << " failed to load, no fallback available (it already " + "is the default skin)"; + } else { + qWarning() << "Configured skin " << pSkin->name() + << " failed to load, falling back to default skin " + << defaultSkinName; + pSkin = getSkin(defaultSkinName); + // If we don't have a skin then fail. + VERIFY_OR_DEBUG_ASSERT(pSkin != nullptr && pSkin->isValid()) { + qCritical() << "Default skin" << defaultSkinName << "not found"; + return nullptr; + } + + // This might also fail, but + pLoadedSkin = pSkin->loadSkin(pParent, m_pConfig, pSkinCreatedControls, pCoreServices); + } + DEBUG_ASSERT(pLoadedSkin != nullptr); + } + + VERIFY_OR_DEBUG_ASSERT(pLoadedSkin != nullptr) { + qCritical() << "No skin can be loaded, please check your installation."; + } + return pLoadedSkin; } LaunchImage* SkinLoader::loadLaunchImage(QWidget* pParent) const {