qt: fix compositor_x11 freeze when window is restored from minimized state

changed milestone to %4.0

this looks like a bad idea, especially since we have cases where the window is sized to match the video

Yeah, I also did not like it so much, but I can not resize to the same size since it does not go through when new size is equal to the old size.

I'm open to suggestions. This did not cause any noticeable impact to me, and the size should be fixed with the expose() call anyway.

I don't know yet, I should have a look, but we really can't do this

changed this line in version 2 of the diff

I changed this, maybe it is acceptable now at least. (Still not the perfect solution, but we really need to fix this).

So I investigate this issue, what really needs to be do is resetting the pixmaps in the CompositorX11RenderClient, your solution works because there is a connection on the widthChanged signal.

I tried to improve your MR

I wanted to listen to the statechange signal from the UI window, but the signal is never sent
I opted to add an explicit signal, instead of repurposing an existing one as this feels hackish
connections have been move outside the CompositorX11RenderClient as it doesn't really need to deal with Qt window, only xcb window, this also allowed to lower the expectation regarding which signal the object will emit
that's probably the same matter in CompositorX11UISurface::handleScreenChange but I don't have a second screen to test my theory right now

diff --git a/modules/gui/qt/maininterface/compositor_x11_renderclient.cpp b/modules/gui/qt/maininterface/compositor_x11_renderclient.cpp
index 075e5814e5..9eab61998f 100644
--- a/modules/gui/qt/maininterface/compositor_x11_renderclient.cpp
+++ b/modules/gui/qt/maininterface/compositor_x11_renderclient.cpp
@@ -22,12 +22,11 @@
 
 using namespace vlc;
 
-CompositorX11RenderClient::CompositorX11RenderClient(qt_intf_t* p_intf, xcb_connection_t* conn,  QWindow* window, QObject *parent)
+CompositorX11RenderClient::CompositorX11RenderClient(qt_intf_t* p_intf, xcb_connection_t* conn,  xcb_window_t wid, QObject *parent)
     : QObject(parent)
     , m_intf(p_intf)
-    , m_window(window)
     , m_conn(conn)
-    , m_wid(window->winId())
+    , m_wid(wid)
     , m_pixmap(m_conn)
     , m_picture(m_conn)
 {
@@ -65,9 +64,6 @@ CompositorX11RenderClient::CompositorX11RenderClient(qt_intf_t* p_intf, xcb_conn
         xcb_change_property(m_conn, XCB_PROP_MODE_REPLACE, m_wid,
                             _NET_WM_BYPASS_COMPOSITOR, XCB_ATOM_CARDINAL, 32, 1, &val);
     }
-
-    connect(window, &QWindow::widthChanged, this, &CompositorX11RenderClient::resetPixmap);
-    connect(window, &QWindow::heightChanged, this, &CompositorX11RenderClient::resetPixmap);
 }
 
 CompositorX11RenderClient::~CompositorX11RenderClient()
@@ -78,7 +74,7 @@ CompositorX11RenderClient::~CompositorX11RenderClient()
 
 xcb_drawable_t CompositorX11RenderClient::getWindowXid() const
 {
-    return m_window->winId();
+    return m_wid;
 }
 
 void CompositorX11RenderClient::createPicture()
diff --git a/modules/gui/qt/maininterface/compositor_x11_renderclient.hpp b/modules/gui/qt/maininterface/compositor_x11_renderclient.hpp
index 19e9258182..de81ac0eb8 100644
--- a/modules/gui/qt/maininterface/compositor_x11_renderclient.hpp
+++ b/modules/gui/qt/maininterface/compositor_x11_renderclient.hpp
@@ -43,7 +43,7 @@ class CompositorX11RenderClient : public QObject
 public:
     CompositorX11RenderClient(
             qt_intf_t* p_intf, xcb_connection_t* conn,
-            QWindow* window,
+            xcb_window_t wid,
             QObject* parent = nullptr);
 
     ~CompositorX11RenderClient();
@@ -58,7 +58,6 @@ public slots:
 
 private:
     qt_intf_t* m_intf;
-    QWindow* m_window = nullptr;
 
     xcb_connection_t* m_conn = 0;
     xcb_window_t m_wid = 0;
diff --git a/modules/gui/qt/maininterface/compositor_x11_renderwindow.cpp b/modules/gui/qt/maininterface/compositor_x11_renderwindow.cpp
index 5051b2a848..635bf09292 100644
--- a/modules/gui/qt/maininterface/compositor_x11_renderwindow.cpp
+++ b/modules/gui/qt/maininterface/compositor_x11_renderwindow.cpp
@@ -538,7 +538,9 @@ void CompositorX11RenderWindow::setVideoWindow( QWindow* window)
 {
     //ensure Qt x11 pending operation have been forwarded to the server
     xcb_flush(qGuiApp->nativeInterface<QNativeInterface::QX11Application>()->connection());
-    m_videoClient = std::make_unique<CompositorX11RenderClient>(m_intf, m_conn, window);
+    m_videoClient = std::make_unique<CompositorX11RenderClient>(m_intf, m_conn, window->winId());
+    connect(window, &QWindow::widthChanged, m_videoClient.get(), &CompositorX11RenderClient::resetPixmap);
+    connect(window, &QWindow::heightChanged, m_videoClient.get(), &CompositorX11RenderClient::resetPixmap);
     m_videoPosition = QRect(0,0,0,0);
     m_videoWindow = window;
     emit videoSurfaceChanged(m_videoClient.get());
@@ -565,7 +567,10 @@ void CompositorX11RenderWindow::setInterfaceWindow(CompositorX11UISurface* windo
 {
     //ensure Qt x11 pending operation have been forwarded to the server
     xcb_flush(qGuiApp->nativeInterface<QNativeInterface::QX11Application>()->connection());
-    m_interfaceClient = std::make_unique<CompositorX11RenderClient>(m_intf, m_conn, window);
+    m_interfaceClient = std::make_unique<CompositorX11RenderClient>(m_intf, m_conn, window->winId());
+    connect(window, &QWindow::widthChanged, m_interfaceClient.get(), &CompositorX11RenderClient::resetPixmap);
+    connect(window, &QWindow::heightChanged, m_interfaceClient.get(), &CompositorX11RenderClient::resetPixmap);
+    connect(window, &CompositorX11UISurface::requestPixmapReset, m_interfaceClient.get(), &CompositorX11RenderClient::resetPixmap);
     m_interfaceWindow = window;
 }
 
diff --git a/modules/gui/qt/maininterface/compositor_x11_uisurface.cpp b/modules/gui/qt/maininterface/compositor_x11_uisurface.cpp
index ef279bc027..4460ec0c89 100644
--- a/modules/gui/qt/maininterface/compositor_x11_uisurface.cpp
+++ b/modules/gui/qt/maininterface/compositor_x11_uisurface.cpp
@@ -367,11 +367,7 @@ void CompositorX11UISurface::resizeFbo()
     {
         const bool current = m_context->makeCurrent(this);
         assert(current);
-
-        m_context->functions()->glDeleteTextures(1, &m_textureId);
-        m_textureId = 0;
-        m_context->functions()->glDeleteFramebuffers(1, &m_fboId);
-        m_fboId = 0;
+        destroyFbo();
         createFbo();
         m_context->doneCurrent();
         updateSizes();
@@ -420,14 +416,16 @@ void CompositorX11UISurface::exposeEvent(QExposeEvent *)
         {
             m_uiRenderControl->initialize();
         }
+        emit requestPixmapReset();
         requestUpdate();
     }
 }
 
 void CompositorX11UISurface::handleScreenChange()
 {
-   m_uiWindow->setGeometry(0, 0, width(), height());
-   requestUpdate();
+    //m_uiWindow->setGeometry(0, 0, width(), height());
+    emit requestPixmapReset();
+    requestUpdate();
 }
 
 void CompositorX11UISurface::forwardFocusObjectChanged(QObject* object)
diff --git a/modules/gui/qt/maininterface/compositor_x11_uisurface.hpp b/modules/gui/qt/maininterface/compositor_x11_uisurface.hpp
index be620d4fe4..7637dac01a 100644
--- a/modules/gui/qt/maininterface/compositor_x11_uisurface.hpp
+++ b/modules/gui/qt/maininterface/compositor_x11_uisurface.hpp
@@ -75,6 +75,8 @@ signals:
     void sizeChanged(const QSize& size);
     void updated();
 
+    void requestPixmapReset();
+
 protected:
     bool eventFilter(QObject* object, QEvent *event) override;

Can you create a new merge request for that?

see !5483 (merged)

added MRStatus::InReview label and removed MRStatus::Reviewable label

added 64 commits

26c4d532...2e086ae8 - 61 commits from branch videolan:master
a26fd315 - qt: remove unused member in CompositorX11UISurface
d90a2ff4 - qt: use destroyFbo() in CompositorX11UISurface::resizeFbo()
ec0e1796 - qt: fix compositor_x11 freeze when window is restored from minimized state

Compare with previous version

added 1 commit

16d2a9f1 - qt: fix compositor_x11 freeze when window is restored from minimized state

Compare with previous version

added 1 commit

ceba43a8 - qt: fix compositor_x11 freeze when window is restored from minimized state

Compare with previous version

resolved all threads

It only works like this. I added a comment now to indicate why it is there.

Ping @chub.

mentioned in merge request !5483 (merged)

Succeeded by !5483 (merged).

closed

         m_uiWindow->setScreen(screen());
         break;
     case QEvent::WindowStateChange:
         if (m_renderWindow->windowState() != Qt::WindowMinimized)
             resize(size() - QSize(1, 1));

         m_uiWindow->setScreen(screen());
         break;
     case QEvent::WindowStateChange:
         // Workaround: without this, quick view freezes after going back from minimized state
         if (m_renderWindow->windowState() != Qt::WindowMinimized)
             widthChanged(width());

qt: fix compositor_x11 freeze when window is restored from minimized state

Merge request reports

Activity