Skip to content

Commit

Permalink
Move the engine pause logic in a dedicated object
Browse files Browse the repository at this point in the history
  • Loading branch information
acolombier committed May 16, 2024
1 parent a87fd88 commit 633c84d
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 125 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,7 @@ else()
target_sources(mixxx-lib PRIVATE
# The following source depends of QML being available but aren't part of the new QML UI
src/controllers/rendering/controllerrenderingengine.cpp
src/controllers/controllerenginethreadcontrol.cpp
src/controllers/controllerscreenpreview.cpp
)
endif()
Expand Down
97 changes: 97 additions & 0 deletions src/controllers/controllerenginethreadcontrol.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#include "controllers/controllerenginethreadcontrol.h"

#include <QCoreApplication>

#include "moc_controllerenginethreadcontrol.cpp"
#include "util/assert.h"
#include "util/logger.h"
#include "util/mutex.h"
#include "util/thread_affinity.h"

namespace {
const mixxx::Logger kLogger("ControllerEngineThreadControl");
constexpr int kMaxPauseDurationMilliseconds = 1000;
} // namespace

ControllerEngineThreadControl::ControllerEngineThreadControl(QObject* parent)
: QObject(parent) {
}
bool ControllerEngineThreadControl::pause() {
VERIFY_OR_DEBUG_ASSERT_THIS_QOBJECT_THREAD_ANTI_AFFINITY() {
return false;
}
const auto lock = lockMutex(&m_pauseMutex);
m_pauseCount++;

if (m_canPause && !m_isPaused) {
emit pauseRequested();
}

while (m_canPause && !m_isPaused) {
if (!m_isPausedCondition.wait(&m_pauseMutex, kMaxPauseDurationMilliseconds)) {
kLogger.warning() << "Pause request timed out!";
m_pauseCount--;
return false;
}
}
return !m_canPause || m_isPaused;
}
void ControllerEngineThreadControl::resume() {
VERIFY_OR_DEBUG_ASSERT_THIS_QOBJECT_THREAD_ANTI_AFFINITY() {
return;
}
const auto lock = lockMutex(&m_pauseMutex);
if (m_pauseCount > 0) {
m_pauseCount--;
}
m_isPaused = m_pauseCount > 0;
m_isPausedCondition.wakeOne();
}
void ControllerEngineThreadControl::setCanPause(bool canPause) {
DEBUG_ASSERT_THIS_QOBJECT_THREAD_AFFINITY();
auto lock = lockMutex(&m_pauseMutex);
m_canPause = canPause;

if (m_canPause) {
connect(this,
&ControllerEngineThreadControl::pauseRequested,
this,
&ControllerEngineThreadControl::doPause,
Qt::UniqueConnection);
} else {
// New signals may have been queued emitted requesting for pause, so we
// manually process the event loop now to clear and handle those, before
// disabling pausing. Without this, thread requesting pause will stay
// stuck waiting on the condvar
lock.unlock();
QCoreApplication::processEvents();
lock.relock();

disconnect(this,
&ControllerEngineThreadControl::pauseRequested,
this,
&ControllerEngineThreadControl::doPause);

m_isPaused = false;
m_pauseCount = 0;
m_isPausedCondition.wakeOne();
}
}
void ControllerEngineThreadControl::doPause() {
VERIFY_OR_DEBUG_ASSERT_THIS_QOBJECT_THREAD_AFFINITY() {
return;
}
const auto lock = lockMutex(&m_pauseMutex);
m_isPaused = m_pauseCount > 0;
m_isPausedCondition.wakeOne();

while (m_canPause && m_isPaused) {
VERIFY_OR_DEBUG_ASSERT(m_isPausedCondition.wait(
&m_pauseMutex, kMaxPauseDurationMilliseconds)) {
kLogger.warning() << "Engine pause timed out!";
m_isPaused = false;
m_pauseCount = 0;
};
}
m_isPausedCondition.wakeAll();
}
45 changes: 45 additions & 0 deletions src/controllers/controllerenginethreadcontrol.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#pragma once

#include <QMutex>
#include <QObject>
#include <QWaitCondition>

/// @brief A ControllerEngineThreadControl can be use to orchestrate thread
/// pause on a ControllerScriptEngineBase. It used Pause is required by
/// rendering thread (https://doc.qt.io/qt-6/qquickrendercontrol.html#sync).
/// This offscreen render thread to pause the main "GUI thread" for onboard
/// screens
/// The documentation isn't completely clear about this, but after
/// testing, it appears that the "GUI main thread" is the thread where the QML
/// engine leaves in (also the main thread if we were using a
/// QMLApplication, which isn't the case here)
class ControllerEngineThreadControl : public QObject {
Q_OBJECT
public:
explicit ControllerEngineThreadControl(QObject* parent = nullptr);

public slots:
// The following slots may be used by rendering engine to pause the thread.
// They must be called from different thread than
// ControllerEngineThreadControl's
bool pause();
void resume();

// Change whether or not it is possible to pause the thread. Should be
// called from eh same thread than ControllerEngineThreadControl
void setCanPause(bool canPause);
private slots:
// Used to effectively pause the thread. Must be called from the same thread
// than ControllerEngineThreadControl
void doPause();

signals:
void pauseRequested();

private:
QWaitCondition m_isPausedCondition;
QMutex m_pauseMutex;
int m_pauseCount{0};
bool m_isPaused{false};
bool m_canPause{false};
};
53 changes: 26 additions & 27 deletions src/controllers/rendering/controllerrenderingengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
#include <QThread>

#include "controllers/controller.h"
#include "controllers/controllerenginethreadcontrol.h"
#include "controllers/scripting/legacy/controllerscriptenginelegacy.h"
#include "controllers/scripting/legacy/controllerscriptinterfacelegacy.h"
#include "moc_controllerrenderingengine.cpp"
#include "qml/qmlwaveformoverview.h"
#include "util/cmdlineargs.h"
#include "util/logger.h"
#include "util/thread_affinity.h"
#include "util/time.h"
#include "util/timer.h"

Expand All @@ -37,13 +39,13 @@ using Clock = std::chrono::steady_clock;

ControllerRenderingEngine::ControllerRenderingEngine(
const LegacyControllerMapping::ScreenInfo& info,
ControllerScriptEngineBase* parent)
ControllerEngineThreadControl* engineThreadControl)
: QObject(),
m_screenInfo(info),
m_GLDataFormat(GL_RGBA),
m_GLDataType(GL_UNSIGNED_BYTE),
m_isValid(true),
m_pControllerEngine(parent) {
m_pEngineThreadControl(engineThreadControl) {
switch (m_screenInfo.pixelFormat) {
case QImage::Format_RGB16:
m_GLDataFormat = GL_RGB;
Expand Down Expand Up @@ -83,10 +85,6 @@ void ControllerRenderingEngine::prepare() {
#endif

// these at first sight weird-looking connections facilitate thread-safe communication.
connect(this,
&ControllerRenderingEngine::engineSetupRequested,
this,
&ControllerRenderingEngine::setup);
connect(this,
&ControllerRenderingEngine::sendFrameDataRequested,
this,
Expand All @@ -100,7 +98,7 @@ void ControllerRenderingEngine::prepare() {
}

ControllerRenderingEngine::~ControllerRenderingEngine() {
DEBUG_ASSERT(QThread::currentThread() != thread());
DEBUG_ASSERT_THIS_QOBJECT_THREAD_ANTI_AFFINITY();
m_pThread->wait();
VERIFY_OR_DEBUG_ASSERT(!m_fbo) {
kLogger.critical() << "The ControllerEngine is being deleted but hasn't been "
Expand All @@ -121,23 +119,26 @@ bool ControllerRenderingEngine::isRunning() const {
}

void ControllerRenderingEngine::requestEngineSetup(std::shared_ptr<QQmlEngine> qmlEngine) {
m_isValid = false;
DEBUG_ASSERT(!m_quickWindow);
VERIFY_OR_DEBUG_ASSERT(qmlEngine) {
kLogger.critical() << "No QML engine was passed!";
return;
}
VERIFY_OR_DEBUG_ASSERT(QThread::currentThread() != thread()) {
VERIFY_OR_DEBUG_ASSERT_THIS_QOBJECT_THREAD_ANTI_AFFINITY() {
kLogger.warning() << "Unable to setup OpenGL rendering context from the same "
"thread as the render object";
return;
}
emit engineSetupRequested(qmlEngine);

const auto lock = lockMutex(&m_mutex);
if (!m_quickWindow) {
m_waitCondition.wait(&m_mutex);
}
if (m_isValid) {
QMetaObject::invokeMethod(
this,
[this, qmlEngine]() {
setup(qmlEngine);
},
// This invocation will block the current thread!
Qt::BlockingQueuedConnection);

if (m_quickWindow) {
m_renderControl->prepareThread(m_pThread.get());
}
}
Expand All @@ -148,7 +149,10 @@ void ControllerRenderingEngine::requestSendingFrameData(
}

void ControllerRenderingEngine::setup(std::shared_ptr<QQmlEngine> qmlEngine) {
DEBUG_ASSERT(QThread::currentThread() == thread());
VERIFY_OR_DEBUG_ASSERT_THIS_QOBJECT_THREAD_AFFINITY() {
kLogger.warning() << "The ControllerRenderingEngine setup must be done by its own thread!";
return;
}
QSurfaceFormat format;
format.setSamples(m_screenInfo.msaa);
format.setDepthBufferSize(16);
Expand All @@ -158,7 +162,6 @@ void ControllerRenderingEngine::setup(std::shared_ptr<QQmlEngine> qmlEngine) {
m_context->setFormat(format);
VERIFY_OR_DEBUG_ASSERT(m_context->create()) {
kLogger.warning() << "Unable to initialize controller screen rendering. Giving up";
m_waitCondition.wakeAll();
return;
}
connect(m_context.get(),
Expand All @@ -181,7 +184,6 @@ void ControllerRenderingEngine::setup(std::shared_ptr<QQmlEngine> qmlEngine) {
kLogger.warning() << "Unable to create the OffscreenSurface for controller "
"screen rendering. Giving up";
m_offscreenSurface.reset();
m_waitCondition.wakeAll();
return;
}

Expand All @@ -193,13 +195,10 @@ void ControllerRenderingEngine::setup(std::shared_ptr<QQmlEngine> qmlEngine) {
}

m_quickWindow->setGeometry(0, 0, m_screenInfo.size.width(), m_screenInfo.size.height());

m_isValid = true;
m_waitCondition.wakeAll();
}

void ControllerRenderingEngine::finish() {
DEBUG_ASSERT(QThread::currentThread() == thread());
DEBUG_ASSERT_THIS_QOBJECT_THREAD_AFFINITY();
emit stopping();

m_isValid = false;
Expand Down Expand Up @@ -270,8 +269,8 @@ void ControllerRenderingEngine::renderFrame() {

m_renderControl->beginFrame();

if (m_pControllerEngine) {
m_pControllerEngine->pause();
if (m_pEngineThreadControl) {
m_pEngineThreadControl->pause();
}

m_renderControl->polishItems();
Expand All @@ -283,8 +282,8 @@ void ControllerRenderingEngine::renderFrame() {
};
}

if (m_pControllerEngine) {
m_pControllerEngine->resume();
if (m_pEngineThreadControl) {
m_pEngineThreadControl->resume();
}
QImage fboImage(m_screenInfo.size, m_screenInfo.pixelFormat);

Expand Down Expand Up @@ -341,7 +340,7 @@ bool ControllerRenderingEngine::stop() {
}

void ControllerRenderingEngine::send(Controller* controller, const QByteArray& frame) {
DEBUG_ASSERT(QThread::currentThread() == thread());
DEBUG_ASSERT_THIS_QOBJECT_THREAD_AFFINITY();
ScopedTimer t(u"ControllerRenderingEngine::send");
if (!frame.isEmpty()) {
controller->sendBytes(frame);
Expand Down
18 changes: 7 additions & 11 deletions src/controllers/rendering/controllerrenderingengine.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,15 @@

#include <GL/gl.h>

#include <QMutex>
#include <QObject>
#include <QWaitCondition>
#include <chrono>

#include "controllers/legacycontrollermapping.h"
#include "controllers/scripting/controllerscriptenginebase.h"
#include "preferences/configobject.h"
#include "util/time.h"

class Controller;
class ControllerEngineThreadControl;
class QOffscreenSurface;
class QOpenGLContext;
class QOpenGLFramebufferObject;
Expand All @@ -27,7 +25,7 @@ class ControllerRenderingEngine : public QObject {
Q_OBJECT
public:
ControllerRenderingEngine(const LegacyControllerMapping::ScreenInfo& info,
ControllerScriptEngineBase* parent);
ControllerEngineThreadControl* parent);
~ControllerRenderingEngine();

bool event(QEvent* event) override;
Expand Down Expand Up @@ -74,7 +72,6 @@ class ControllerRenderingEngine : public QObject {
void frameRendered(const LegacyControllerMapping::ScreenInfo& screeninfo,
QImage frame,
const QDateTime& timestamp);
void engineSetupRequested(std::shared_ptr<QQmlEngine> engine);
void stopping();
/// @brief Request the screen thread to send a frame to the device
/// @param controller the controller to send the frame to
Expand All @@ -101,10 +98,9 @@ class ControllerRenderingEngine : public QObject {
GLenum m_GLDataType;

bool m_isValid;

// These mutexes components are used to ensure internal object synchronicity
QWaitCondition m_waitCondition;
QMutex m_mutex;

ControllerScriptEngineBase* m_pControllerEngine;
// Engine control is owned by ControllerScriptEngineBase. The assumption is
// made that ControllerScriptEngineBase always outlive
// ControllerRenderingEngine as it is in charge of stopping and joining the
// thread
ControllerEngineThreadControl* m_pEngineThreadControl;
};
Loading

0 comments on commit 633c84d

Please sign in to comment.