Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start to untangle CoreServices/MixxxMainWindow initialization #4110

Merged
merged 4 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/coreservices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 2 additions & 4 deletions src/coreservices.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ class CoreServices : public QObject {
CoreServices(const CmdlineArgs& args);
~CoreServices();

void initializeSettings();
// FIXME: should be private, but WMainMenuBar needs it initialized early
Be-ing marked this conversation as resolved.
Show resolved Hide resolved
void initializeKeyboard();
void initializeScreensaverManager();
void initialize(QApplication* pApp);
void shutdown();

Expand Down Expand Up @@ -121,6 +117,8 @@ class CoreServices : public QObject {

private:
bool initializeDatabase();
void initializeKeyboard();
void initializeSettings();

std::shared_ptr<SettingsManager> m_pSettingsManager;
std::shared_ptr<mixxx::ControlIndicatorTimer> m_pControlIndicatorTimer;
Expand Down
14 changes: 12 additions & 2 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,18 @@ constexpr int kFatalErrorOnStartupExitCode = 1;
constexpr int kParseCmdlineArgsErrorExitCode = 2;

int runMixxx(MixxxApplication* app, const CmdlineArgs& args) {
auto coreServices = std::make_shared<mixxx::CoreServices>(args);
MixxxMainWindow mainWindow(app, coreServices);
const auto pCoreServices = std::make_shared<mixxx::CoreServices>(args);

MixxxMainWindow mainWindow(app, pCoreServices);
app->installEventFilter(&mainWindow);

QObject::connect(pCoreServices.get(),
&mixxx::CoreServices::initializationProgressUpdate,
&mainWindow,
&MixxxMainWindow::initializationProgressUpdate);
pCoreServices->initialize(app);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be moved to the CoreServices constructor by passing the QApplication* as another argument?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we need the main window for monitoring progress during the initialization?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we need the main window for monitoring progress during the initialization?

Indeed. CoreServices are MixxxMainWindow are classes that communicate with each other via signals, but neither does MixxxMainWindow own CoreServices (as it did before) nor vice versa.

Hence, we need to construct both first, then connect the signals, then initialize them.

mainWindow.initialize();

// If startup produced a fatal error, then don't even start the
// Qt event loop.
if (ErrorDialogHandler::instance()->checkError()) {
Expand Down
19 changes: 4 additions & 15 deletions src/mixxxmainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions src/mixxxmainwindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class MixxxMainWindow : public QMainWindow {
MixxxMainWindow(QApplication* app, std::shared_ptr<mixxx::CoreServices> 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();
Expand Down Expand Up @@ -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);

Expand All @@ -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();
Expand Down
32 changes: 32 additions & 0 deletions src/test/coreservicestest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#include <gtest/gtest.h>

#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<mixxx::CoreServices>(cmdlineArgs);
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);
}