diff --git a/src/controllers/controller.h b/src/controllers/controller.h index 566872778f65..4ae4296ce495 100644 --- a/src/controllers/controller.h +++ b/src/controllers/controller.h @@ -141,7 +141,8 @@ class Controller : public QObject { public: // This must be reimplemented by sub-classes desiring to send raw bytes to a - // controller. Return true in case of successful completion, false in case of failure. + // controller. Return true in case of successful completion, false in case + // of partial completion or failure. virtual bool sendBytes(const QByteArray& data) = 0; private: // but used by ControllerManager diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp index 6ad97197432a..1c9cef3aa26d 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp @@ -114,11 +114,11 @@ bool ControllerScriptEngineLegacy::callShutdownFunction() { return callFunctionOnObjects(m_scriptFunctionPrefixes, "shutdown"); #ifdef MIXXX_USE_QML } else { - QHashIterator i(m_rootItems); + QHashIterator i(m_rootItems); bool success = true; while (i.hasNext()) { i.next(); - auto& screen = i.value(); + const auto& screen = i.value(); const QString& screenIdentifier = i.key(); if (!screen->getShutdown().isCallable()) { @@ -148,9 +148,8 @@ bool ControllerScriptEngineLegacy::callShutdownFunction() { } bool ControllerScriptEngineLegacy::callInitFunction() { // m_pController is nullptr in tests. - const auto controllerName = m_pController ? m_pController->getName() : QString{}; const auto args = QJSValueList{ - controllerName, + m_pController ? m_pController->getName() : QString{}, m_logger().isDebugEnabled(), }; @@ -160,10 +159,10 @@ bool ControllerScriptEngineLegacy::callInitFunction() { return callFunctionOnObjects(m_scriptFunctionPrefixes, "init", args, true); #ifdef MIXXX_USE_QML } else { - QHashIterator i(m_rootItems); + QHashIterator i(m_rootItems); while (i.hasNext()) { i.next(); - auto& screen = i.value(); + const auto& screen = i.value(); const QString& screenIdentifier = i.key(); if (!screen->getInit().isCallable()) { @@ -481,7 +480,7 @@ bool ControllerScriptEngineLegacy::bindSceneToScreen( // evaluating it. watchFilePath(qmlFile.file.absoluteFilePath()); - auto pScene = loadQMLFile(qmlFile, pScreen); + auto* pScene = loadQMLFile(qmlFile, pScreen); if (!pScene) { VERIFY_OR_DEBUG_ASSERT(!pScreen->isValid() || !pScreen->isRunning() || pScreen->stop()) { @@ -520,7 +519,7 @@ void ControllerScriptEngineLegacy::handleScreenFrame( return; }; - auto screen = m_rootItems.value(screenInfo.identifier); + auto* pScreen = m_rootItems.value(screenInfo.identifier); if (CmdlineArgs::Instance().getControllerPreviewScreens()) { QImage screenDebug(frame); @@ -550,12 +549,12 @@ void ControllerScriptEngineLegacy::handleScreenFrame( // compilers that don't support it (e.g. older than Xcode 14.3/macOS 13) QByteArray input(reinterpret_cast(frame.constBits()), frame.sizeInBytes()); - if (!screen->getTransform().isCallable() && screenInfo.rawData) { + if (!pScreen->getTransform().isCallable() && screenInfo.rawData) { m_renderingScreens[screenInfo.identifier]->requestSendingFrameData(m_pController, input); return; } - if (!screen->getTransform().isCallable()) { + if (!pScreen->getTransform().isCallable()) { qCWarning(m_logger) << "Could not find a valid transform function but the screen " "doesn't accept raw data. Aborting screen rendering."; @@ -569,7 +568,7 @@ void ControllerScriptEngineLegacy::handleScreenFrame( } // During the frame transformation, any QML errors are considered fatal. setErrorsAreFatal(true); - auto result = screen->getTransform().call( + auto result = pScreen->getTransform().call( QJSValueList{m_pJSEngine->toScriptValue(input), m_pJSEngine->toScriptValue(timestamp)}); if (result.isError()) { @@ -743,7 +742,7 @@ bool ControllerScriptEngineLegacy::evaluateScriptFile(const QFileInfo& scriptFil } #ifdef MIXXX_USE_QML -mixxx::qml::QmlMixxxController* ControllerScriptEngineLegacy::loadQMLFile( +mixxx::qml::QmlMixxxControllerScreen* ControllerScriptEngineLegacy::loadQMLFile( const LegacyControllerMapping::ScriptFileInfo& qmlScript, std::shared_ptr pScreen) { VERIFY_OR_DEBUG_ASSERT(m_pJSEngine || @@ -804,8 +803,8 @@ mixxx::qml::QmlMixxxController* ControllerScriptEngineLegacy::loadQMLFile( return nullptr; } - mixxx::qml::QmlMixxxController* rootItem = - qobject_cast(pRootObject); + mixxx::qml::QmlMixxxControllerScreen* rootItem = + qobject_cast(pRootObject); if (!rootItem) { qWarning("run: Not a QQuickItem"); delete pRootObject; diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h index 6e26be93d630..19d93c7826ce 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h @@ -16,7 +16,7 @@ class QQuickItem; class ControllerRenderingEngine; namespace mixxx { namespace qml { -class QmlMixxxController; +class QmlMixxxControllerScreen; } } // namespace mixxx #endif @@ -90,7 +90,7 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase { void extractTransformFunction(const QMetaObject* metaObject, const QString& screenIdentifier); // The returned QmlMixxxController will be owned and managed by the pScreen - mixxx::qml::QmlMixxxController* loadQMLFile( + mixxx::qml::QmlMixxxControllerScreen* loadQMLFile( const LegacyControllerMapping::ScriptFileInfo& qmlScript, std::shared_ptr pScreen); @@ -120,8 +120,9 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase { QHash> m_renderingScreens; // Contains all the scenes loaded for this mapping. Key is the scene // identifier (LegacyControllerMapping::ScreenInfo::identifier), value in - // the QML root item - QHash m_rootItems; + // the QML root item. Note that the pointer is owned by the QML scene which + // will free them on shutdown (ControllerScriptEngineLegacy::shutdown) + QHash m_rootItems; QList m_modules; QList m_infoScreens; QString m_resourcePath{QStringLiteral(".")}; diff --git a/src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp b/src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp index 67e56886ffa3..2093cb7ffcaa 100644 --- a/src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp +++ b/src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp @@ -524,7 +524,9 @@ int ControllerScriptInterfaceLegacy::beginTimer( m_timers[timerId] = info; if (timerId == 0) { m_pScriptEngineLegacy->logOrThrowError(QStringLiteral("Script timer could not be created")); - } else if (oneShot && CmdlineArgs::Instance().getControllerDebug()) { + } else if (oneShot && + // FIXME workaround log spam (Github issue to be created) + CmdlineArgs::Instance().getControllerDebug()) { qCDebug(m_logger) << "Starting one-shot timer:" << timerId; } else { qCDebug(m_logger) << "Starting timer:" << timerId; diff --git a/src/qml/qmlmixxxcontroller.cpp b/src/qml/qmlmixxxcontroller.cpp index b034068b8132..80a65ab41936 100644 --- a/src/qml/qmlmixxxcontroller.cpp +++ b/src/qml/qmlmixxxcontroller.cpp @@ -8,11 +8,11 @@ namespace mixxx { namespace qml { -QmlMixxxController::QmlMixxxController(QQuickItem* parent) +QmlMixxxControllerScreen::QmlMixxxControllerScreen(QQuickItem* parent) : QQuickItem(parent) { } -void QmlMixxxController::setTransform(const QJSValue& value) { +void QmlMixxxControllerScreen::setTransform(const QJSValue& value) { if (!value.isCallable()) { return; } @@ -20,7 +20,7 @@ void QmlMixxxController::setTransform(const QJSValue& value) { emit transformChanged(); } -void QmlMixxxController::setInit(const QJSValue& value) { +void QmlMixxxControllerScreen::setInit(const QJSValue& value) { if (!value.isCallable()) { return; } @@ -28,7 +28,7 @@ void QmlMixxxController::setInit(const QJSValue& value) { emit initChanged(); } -void QmlMixxxController::setShutdown(const QJSValue& value) { +void QmlMixxxControllerScreen::setShutdown(const QJSValue& value) { if (!value.isCallable()) { return; } diff --git a/src/qml/qmlmixxxcontroller.h b/src/qml/qmlmixxxcontroller.h index 5cdcb2c9b73a..b96c7386afcc 100644 --- a/src/qml/qmlmixxxcontroller.h +++ b/src/qml/qmlmixxxcontroller.h @@ -7,7 +7,7 @@ namespace mixxx { namespace qml { -class QmlMixxxController : public QQuickItem { +class QmlMixxxControllerScreen : public QQuickItem { Q_OBJECT QML_NAMED_ELEMENT(Controller) Q_PROPERTY(QJSValue init READ getInit WRITE setInit @@ -18,7 +18,7 @@ class QmlMixxxController : public QQuickItem { NOTIFY transformChanged REQUIRED); public: - explicit QmlMixxxController(QQuickItem* parent = nullptr); + explicit QmlMixxxControllerScreen(QQuickItem* parent = nullptr); void setInit(const QJSValue& value); void setShutdown(const QJSValue& value); diff --git a/src/test/controllerscriptenginelegacy_test.cpp b/src/test/controllerscriptenginelegacy_test.cpp index 126a709ca7f8..48f3b074ed4e 100644 --- a/src/test/controllerscriptenginelegacy_test.cpp +++ b/src/test/controllerscriptenginelegacy_test.cpp @@ -56,6 +56,12 @@ class ControllerScriptEngineLegacyTest : public ControllerScriptEngineLegacy, pu void TearDown() override { mixxx::Time::setTestMode(false); +#ifdef MIXXX_USE_QML + for (auto& item : m_rootItems) { + delete item; + } + m_rootItems.clear(); +#endif } bool evaluateScriptFile(const QFileInfo& scriptFile) { @@ -86,7 +92,7 @@ class ControllerScriptEngineLegacyTest : public ControllerScriptEngineLegacy, pu return m_renderingScreens; } - QHash& rootItems() { + QHash& rootItems() { return m_rootItems; } @@ -686,7 +692,7 @@ TEST_F(ControllerScriptEngineLegacyTest, screenWontSentRawDataIfNotConfigured) { "accept raw data. Aborting screen rendering."); renderingScreens().insert(dummyScreen.identifier, pDummyRender); - rootItems().insert(dummyScreen.identifier, new mixxx::qml::QmlMixxxController()); + rootItems().insert(dummyScreen.identifier, new mixxx::qml::QmlMixxxControllerScreen()); testHandleScreen( dummyScreen, @@ -717,7 +723,7 @@ TEST_F(ControllerScriptEngineLegacyTest, screenWillSentRawDataIfConfigured) { EXPECT_CALL(*pDummyRender, requestSendingFrameData(_, QByteArray())); renderingScreens().insert(dummyScreen.identifier, pDummyRender); - rootItems().insert(dummyScreen.identifier, new mixxx::qml::QmlMixxxController()); + rootItems().insert(dummyScreen.identifier, new mixxx::qml::QmlMixxxControllerScreen()); testHandleScreen( dummyScreen,