From fa17168ba21dd216717cbac8a93f561171b519b3 Mon Sep 17 00:00:00 2001 From: Norihiro Kamae Date: Tue, 15 Aug 2023 13:11:32 +0900 Subject: [PATCH 1/3] UI: Removes the workaround of not receiving expose event The commit e67fdbca79 is a workaround for QtWayland bug that the `QEvent::Expose` was never received on a certain condition. The bugfix was introduced to QtWayland 6.1.0 so that the workaround is not necessary for now. --- UI/qt-display.cpp | 74 ++--------------------------------------------- UI/qt-display.hpp | 2 +- 2 files changed, 3 insertions(+), 73 deletions(-) diff --git a/UI/qt-display.cpp b/UI/qt-display.cpp index e74e162e0..4b24b4581 100644 --- a/UI/qt-display.cpp +++ b/UI/qt-display.cpp @@ -13,70 +13,6 @@ #include #endif -#ifdef ENABLE_WAYLAND -#include - -class SurfaceEventFilter : public QObject { - OBSQTDisplay *display; - int mTimerId; - -public: - SurfaceEventFilter(OBSQTDisplay *src) : display(src), mTimerId(0) {} - -protected: - bool eventFilter(QObject *obj, QEvent *event) override - { - bool result = QObject::eventFilter(obj, event); - QPlatformSurfaceEvent *surfaceEvent; - - switch (event->type()) { - case QEvent::PlatformSurface: - surfaceEvent = - static_cast(event); - - switch (surfaceEvent->surfaceEventType()) { - case QPlatformSurfaceEvent::SurfaceCreated: - if (display->windowHandle()->isExposed()) - createOBSDisplay(); - else - mTimerId = startTimer(67); // Arbitrary - break; - case QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed: - display->DestroyDisplay(); - break; - default: - break; - } - - break; - case QEvent::Expose: - createOBSDisplay(); - break; - default: - break; - } - - return result; - } - - void timerEvent(QTimerEvent *) override - { - createOBSDisplay(display->isVisible()); - } - -private: - void createOBSDisplay(bool force = false) - { - display->CreateDisplay(force); - if (mTimerId > 0) { - killTimer(mTimerId); - mTimerId = 0; - } - } -}; - -#endif - static inline long long color_to_int(const QColor &color) { auto shift = [&](unsigned val, int shift) { @@ -129,12 +65,6 @@ OBSQTDisplay::OBSQTDisplay(QWidget *parent, Qt::WindowFlags flags) connect(windowHandle(), &QWindow::visibleChanged, windowVisible); connect(windowHandle(), &QWindow::screenChanged, screenChanged); - -#ifdef ENABLE_WAYLAND - if (obs_get_nix_platform() == OBS_NIX_PLATFORM_WAYLAND) - windowHandle()->installEventFilter( - new SurfaceEventFilter(this)); -#endif } QColor OBSQTDisplay::GetDisplayBackgroundColor() const @@ -157,12 +87,12 @@ void OBSQTDisplay::UpdateDisplayBackgroundColor() obs_display_set_background_color(display, backgroundColor); } -void OBSQTDisplay::CreateDisplay(bool force) +void OBSQTDisplay::CreateDisplay() { if (display) return; - if (!windowHandle()->isExposed() && !force) + if (!windowHandle()->isExposed()) return; QSize size = GetPixelSize(this); diff --git a/UI/qt-display.hpp b/UI/qt-display.hpp index 10fe735ed..44946da3a 100644 --- a/UI/qt-display.hpp +++ b/UI/qt-display.hpp @@ -37,7 +37,7 @@ public: QColor GetDisplayBackgroundColor() const; void SetDisplayBackgroundColor(const QColor &color); void UpdateDisplayBackgroundColor(); - void CreateDisplay(bool force = false); + void CreateDisplay(); void DestroyDisplay() { display = nullptr; }; void OnMove(); From fd502cbd4d38fc574fcbb83d2d56de7bd69ce47a Mon Sep 17 00:00:00 2001 From: Norihiro Kamae Date: Tue, 15 Aug 2023 13:22:40 +0900 Subject: [PATCH 2/3] UI: Fix crash at render_display while shutdown on macOS The display context was sometimes created again by a resize event after destruction from `OBSBasic::closeEvent`. That resulted in the display context alive until reaching the destructor of the `OBSQTDisplay` class and caused a crash in `render_display` on macOS. To avoid this, destroy the display context at the event `SurfaceAboutToBeDestroyed` and let not create the display context again. --- UI/qt-display.cpp | 38 ++++++++++++++++++++++++++++++++++++++ UI/qt-display.hpp | 7 ++++++- UI/window-basic-main.cpp | 4 ---- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/UI/qt-display.cpp b/UI/qt-display.cpp index 4b24b4581..3da4c7bc7 100644 --- a/UI/qt-display.cpp +++ b/UI/qt-display.cpp @@ -13,6 +13,39 @@ #include #endif +class SurfaceEventFilter : public QObject { + OBSQTDisplay *display; + +public: + SurfaceEventFilter(OBSQTDisplay *src) : display(src) {} + +protected: + bool eventFilter(QObject *obj, QEvent *event) override + { + bool result = QObject::eventFilter(obj, event); + QPlatformSurfaceEvent *surfaceEvent; + + switch (event->type()) { + case QEvent::PlatformSurface: + surfaceEvent = + static_cast(event); + + switch (surfaceEvent->surfaceEventType()) { + case QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed: + display->DestroyDisplay(); + break; + default: + break; + } + break; + default: + break; + } + + return result; + } +}; + static inline long long color_to_int(const QColor &color) { auto shift = [&](unsigned val, int shift) { @@ -65,6 +98,8 @@ OBSQTDisplay::OBSQTDisplay(QWidget *parent, Qt::WindowFlags flags) connect(windowHandle(), &QWindow::visibleChanged, windowVisible); connect(windowHandle(), &QWindow::screenChanged, screenChanged); + + windowHandle()->installEventFilter(new SurfaceEventFilter(this)); } QColor OBSQTDisplay::GetDisplayBackgroundColor() const @@ -92,6 +127,9 @@ void OBSQTDisplay::CreateDisplay() if (display) return; + if (destroying) + return; + if (!windowHandle()->isExposed()) return; diff --git a/UI/qt-display.hpp b/UI/qt-display.hpp index 44946da3a..7a90f10fb 100644 --- a/UI/qt-display.hpp +++ b/UI/qt-display.hpp @@ -12,6 +12,7 @@ class OBSQTDisplay : public QWidget { SetDisplayBackgroundColor) OBSDisplay display; + bool destroying = false; virtual void paintEvent(QPaintEvent *event) override; virtual void moveEvent(QMoveEvent *event) override; @@ -38,7 +39,11 @@ public: void SetDisplayBackgroundColor(const QColor &color); void UpdateDisplayBackgroundColor(); void CreateDisplay(); - void DestroyDisplay() { display = nullptr; }; + void DestroyDisplay() + { + display = nullptr; + destroying = true; + }; void OnMove(); void OnDisplayChange(); diff --git a/UI/window-basic-main.cpp b/UI/window-basic-main.cpp index 9b4334314..02b1dafed 100644 --- a/UI/window-basic-main.cpp +++ b/UI/window-basic-main.cpp @@ -5154,10 +5154,6 @@ void OBSBasic::closeEvent(QCloseEvent *event) delete extraBrowsers; - ui->preview->DestroyDisplay(); - if (program) - program->DestroyDisplay(); - config_set_string(App()->GlobalConfig(), "BasicWindow", "DockState", saveState().toBase64().constData()); From dfce9b929aa2cd6bc63e855375daf94c2942ad4e Mon Sep 17 00:00:00 2001 From: Norihiro Kamae Date: Tue, 15 Aug 2023 13:43:22 +0900 Subject: [PATCH 3/3] UI: Fix crash at resizing display followed by destruction on macOS The resize event was triggered during the shtudown of OBS. A commit 9f330050ef moves the actual resize code in `gl_update` to the main thread without synchronizing between the main thread and the graphics thread. When the display context is destroyed just after the resizing, the delayed resize code can read the destroyed display context. As a workaround, this commit will destroy the display context at the earlier timing of the shutdown process. --- UI/window-basic-main.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/UI/window-basic-main.cpp b/UI/window-basic-main.cpp index 02b1dafed..50f0fa53f 100644 --- a/UI/window-basic-main.cpp +++ b/UI/window-basic-main.cpp @@ -5128,6 +5128,16 @@ void OBSBasic::closeEvent(QCloseEvent *event) closing = true; + /* While closing, a resize event to OBSQTDisplay could be triggered. + * The graphics thread on macOS dispatches a lambda function to be + * executed asynchronously in the main thread. However, the display is + * sometimes deleted before the lambda function is actually executed. + * To avoid such a case, destroy displays earlier than others such as + * deleting browser docks. */ + ui->preview->DestroyDisplay(); + if (program) + program->DestroyDisplay(); + if (outputHandler->VirtualCamActive()) outputHandler->StopVirtualCam();