From 964d9056ac693f1a94375239e0552fb80dc96acf Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Thu, 15 Jul 2021 22:57:12 +0200 Subject: [PATCH 1/4] main: Move some generic initialization out of MixxxMainWindow The `MixxxMainWindow` class should not be responsible for initializing `CoreServices`. Mixxx should still be able to work when there is no `MixxxMainWindow` (e.g. when we switch to a `QQmlApplication` for QML). --- src/main.cpp | 16 ++++++++++++++-- src/mixxxmainwindow.cpp | 19 ++++--------------- src/mixxxmainwindow.h | 7 ++++--- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 65db21103f4..5e41d3b6c82 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -23,8 +23,20 @@ constexpr int kFatalErrorOnStartupExitCode = 1; constexpr int kParseCmdlineArgsErrorExitCode = 2; int runMixxx(MixxxApplication* app, const CmdlineArgs& args) { - auto coreServices = std::make_shared(args); - MixxxMainWindow mainWindow(app, coreServices); + const auto pCoreServices = std::make_shared(args); + pCoreServices->initializeSettings(); + pCoreServices->initializeKeyboard(); + + MixxxMainWindow mainWindow(app, pCoreServices); + app->installEventFilter(&mainWindow); + + QObject::connect(pCoreServices.get(), + &mixxx::CoreServices::initializationProgressUpdate, + &mainWindow, + &MixxxMainWindow::initializationProgressUpdate); + pCoreServices->initialize(app); + mainWindow.initialize(); + // If startup produced a fatal error, then don't even start the // Qt event loop. if (ErrorDialogHandler::instance()->checkError()) { diff --git a/src/mixxxmainwindow.cpp b/src/mixxxmainwindow.cpp index bf1b751a496..09c378e4f42 100644 --- a/src/mixxxmainwindow.cpp +++ b/src/mixxxmainwindow.cpp @@ -105,8 +105,6 @@ MixxxMainWindow::MixxxMainWindow( m_toolTipsCfg(mixxx::TooltipsPreference::TOOLTIPS_ON) { DEBUG_ASSERT(pApp); DEBUG_ASSERT(pCoreServices); - m_pCoreServices->initializeSettings(); - m_pCoreServices->initializeKeyboard(); // These depend on the settings createMenuBar(); m_pMenuBar->hide(); @@ -124,15 +122,9 @@ MixxxMainWindow::MixxxMainWindow( m_pGuiTick = new GuiTick(); m_pVisualsManager = new VisualsManager(); +} - connect( - m_pCoreServices.get(), - &mixxx::CoreServices::initializationProgressUpdate, - this, - &MixxxMainWindow::initializationProgressUpdate); - - m_pCoreServices->initialize(pApp); - +void MixxxMainWindow::initialize() { m_pCoreServices->getControlIndicatorTimer()->setLegacyVsyncEnabled(true); UserSettingsPointer pConfig = m_pCoreServices->getSettings(); @@ -168,6 +160,8 @@ MixxxMainWindow::MixxxMainWindow( initializationProgressUpdate(65, tr("skin")); + // Install an event filter to catch certain QT events, such as tooltips. + // This allows us to turn off tooltips. installEventFilter(m_pCoreServices->getKeyboardEventFilter().get()); DEBUG_ASSERT(m_pCoreServices->getPlayerManager()); @@ -274,11 +268,6 @@ MixxxMainWindow::MixxxMainWindow( checkDirectRendering(); } - // Install an event filter to catch certain QT events, such as tooltips. - // This allows us to turn off tooltips. - pApp->installEventFilter(this); // The eventfilter is located in this - // Mixxx class as a callback. - // Try open player device If that fails, the preference panel is opened. bool retryClicked; do { diff --git a/src/mixxxmainwindow.h b/src/mixxxmainwindow.h index d2adb8623f7..d3a1d8f9f84 100644 --- a/src/mixxxmainwindow.h +++ b/src/mixxxmainwindow.h @@ -50,6 +50,8 @@ class MixxxMainWindow : public QMainWindow { MixxxMainWindow(QApplication* app, std::shared_ptr pCoreServices); ~MixxxMainWindow() override; + /// Initialize main window after creation. Should only be called once. + void initialize(); /// creates the menu_bar and inserts the file Menu void createMenuBar(); void connectMenuBar(); @@ -82,6 +84,8 @@ class MixxxMainWindow : public QMainWindow { void slotNoDeckPassthroughInputConfigured(); void slotNoVinylControlInputConfigured(); + void initializationProgressUpdate(int progress, const QString& serviceName); + private slots: void slotTooltipModeChanged(mixxx::TooltipsPreference tt); @@ -97,9 +101,6 @@ class MixxxMainWindow : public QMainWindow { bool eventFilter(QObject *obj, QEvent *event) override; void closeEvent(QCloseEvent *event) override; - private slots: - void initializationProgressUpdate(int progress, const QString& serviceName); - private: void initializeWindow(); void checkDirectRendering(); From 52709c6208e5aa28c375ea57b9ce7d476c7c01be Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 17 Jul 2021 17:05:11 +0200 Subject: [PATCH 2/4] test: Add CoreServices "smoke test" to check if initialization works Before, CoreServices would cause a debug assertion when initializing it without other stuff from `MixxxMainWindow`. This is just a smoke test to verify that it's possible to initialize the `CoreServices` class on its own, without `MixxxMainWindow. --- CMakeLists.txt | 1 + src/test/coreservicestest.cpp | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 src/test/coreservicestest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 1fbe4b8f1b9..826044ee326 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1536,6 +1536,7 @@ add_executable(mixxx-test src/test/controller_mapping_validation_test.cpp src/test/controllerscriptenginelegacy_test.cpp src/test/controlobjecttest.cpp + src/test/coreservicestest.cpp src/test/coverartcache_test.cpp src/test/coverartutils_test.cpp src/test/cratestorage_test.cpp diff --git a/src/test/coreservicestest.cpp b/src/test/coreservicestest.cpp new file mode 100644 index 00000000000..82b7d0f4874 --- /dev/null +++ b/src/test/coreservicestest.cpp @@ -0,0 +1,34 @@ +#include + +#include "coreservices.h" +#include "test/mixxxtest.h" +#include "util/cmdlineargs.h" + +class CoreServicesTest : public MixxxTest { +}; + +// This test is disabled because it fails on CI for some unknown reason. +TEST_F(CoreServicesTest, DISABLED_TestInitialization) { + // Initialize the app + const int argc = 1; + CmdlineArgs cmdlineArgs; + char progName[] = "mixxxtest"; + char safeMode[] = "--safe-mode"; + char* argv[] = {progName, safeMode}; + cmdlineArgs.parse(argc, argv); + + const auto pCoreServices = std::make_unique(cmdlineArgs); + pCoreServices->initializeSettings(); + pCoreServices->initializeKeyboard(); + pCoreServices->initialize(application()); + + EXPECT_NE(pCoreServices->getControllerManager(), nullptr); + EXPECT_NE(pCoreServices->getEffectsManager(), nullptr); + EXPECT_NE(pCoreServices->getLV2Backend(), nullptr); + EXPECT_NE(pCoreServices->getLibrary(), nullptr); + EXPECT_NE(pCoreServices->getPlayerManager(), nullptr); + EXPECT_NE(pCoreServices->getScreensaverManager(), nullptr); + EXPECT_NE(pCoreServices->getSettingsManager(), nullptr); + EXPECT_NE(pCoreServices->getSoundManager(), nullptr); + EXPECT_NE(pCoreServices->getVinylControlManager(), nullptr); +} From 5635101fd701e033c85f1c9a85707f4bb5197991 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 26 Jul 2021 17:47:37 +0200 Subject: [PATCH 3/4] CoreServices: Move settings/keyboard initialization to constructor --- src/coreservices.cpp | 2 ++ src/coreservices.h | 5 ++--- src/main.cpp | 2 -- src/test/coreservicestest.cpp | 2 -- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/coreservices.cpp b/src/coreservices.cpp index fd8737b4fde..36d1c0725c7 100644 --- a/src/coreservices.cpp +++ b/src/coreservices.cpp @@ -106,6 +106,8 @@ namespace mixxx { CoreServices::CoreServices(const CmdlineArgs& args) : m_runtime_timer(QLatin1String("CoreServices::runtime")), m_cmdlineArgs(args) { + initializeSettings(); + initializeKeyboard(); } CoreServices::~CoreServices() = default; diff --git a/src/coreservices.h b/src/coreservices.h index 48e3ebc00b9..2e79a551f52 100644 --- a/src/coreservices.h +++ b/src/coreservices.h @@ -40,9 +40,6 @@ class CoreServices : public QObject { CoreServices(const CmdlineArgs& args); ~CoreServices(); - void initializeSettings(); - // FIXME: should be private, but WMainMenuBar needs it initialized early - void initializeKeyboard(); void initializeScreensaverManager(); void initialize(QApplication* pApp); void shutdown(); @@ -121,6 +118,8 @@ class CoreServices : public QObject { private: bool initializeDatabase(); + void initializeKeyboard(); + void initializeSettings(); std::shared_ptr m_pSettingsManager; std::shared_ptr m_pControlIndicatorTimer; diff --git a/src/main.cpp b/src/main.cpp index 5e41d3b6c82..c29eb564d20 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -24,8 +24,6 @@ constexpr int kParseCmdlineArgsErrorExitCode = 2; int runMixxx(MixxxApplication* app, const CmdlineArgs& args) { const auto pCoreServices = std::make_shared(args); - pCoreServices->initializeSettings(); - pCoreServices->initializeKeyboard(); MixxxMainWindow mainWindow(app, pCoreServices); app->installEventFilter(&mainWindow); diff --git a/src/test/coreservicestest.cpp b/src/test/coreservicestest.cpp index 82b7d0f4874..cf4971056e1 100644 --- a/src/test/coreservicestest.cpp +++ b/src/test/coreservicestest.cpp @@ -18,8 +18,6 @@ TEST_F(CoreServicesTest, DISABLED_TestInitialization) { cmdlineArgs.parse(argc, argv); const auto pCoreServices = std::make_unique(cmdlineArgs); - pCoreServices->initializeSettings(); - pCoreServices->initializeKeyboard(); pCoreServices->initialize(application()); EXPECT_NE(pCoreServices->getControllerManager(), nullptr); From b1017e03593b86d9660e61d42665cf0ea96d6a04 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 26 Jul 2021 17:49:26 +0200 Subject: [PATCH 4/4] CoreServices: Remove unused method --- src/coreservices.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreservices.h b/src/coreservices.h index 2e79a551f52..d1447e3d407 100644 --- a/src/coreservices.h +++ b/src/coreservices.h @@ -40,7 +40,6 @@ class CoreServices : public QObject { CoreServices(const CmdlineArgs& args); ~CoreServices(); - void initializeScreensaverManager(); void initialize(QApplication* pApp); void shutdown();