[vlc-commits] [Git][videolan/vlc][master] 5 commits: qt: disable synchronization when video surface is invisible

[Steve Lhomme (@robUx4)]

Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
e3f1fd7f by Fatih Uzunoglu at 2025-09-14T08:03:25+00:00
qt: disable synchronization when video surface is invisible

- - - - -
43233f6a by Fatih Uzunoglu at 2025-09-14T08:03:25+00:00
qt: disable synchronization when video surface has no provider

- - - - -
46243e92 by Fatih Uzunoglu at 2025-09-14T08:03:25+00:00
qt: fix window connection remains when new window is null in video surface

- - - - -
ceb5e973 by Fatih Uzunoglu at 2025-09-14T08:03:25+00:00
qt: disconnect the window connection solely in `VideoSurface::itemChange()`

- - - - -
17bebed8 by Fatih Uzunoglu at 2025-09-14T08:03:25+00:00
qt: schedule update after setting `ItemHasContents` in video surface

This should not be necessary at all, because when the item starts
having content, Qt should automatically schedule an update anyway
as without the initial update, the item can not paint its content.

However, we don't lose anything by this, so it should be okay as
a precautionary measure in case Qt does not do that.

- - - - -


2 changed files:

- modules/gui/qt/maininterface/videosurface.cpp
- modules/gui/qt/maininterface/videosurface.hpp


Changes:

=====================================
modules/gui/qt/maininterface/videosurface.cpp
=====================================
@@ -279,11 +279,42 @@ void VideoSurface::synchronize()
 
 void VideoSurface::itemChange(ItemChange change, const ItemChangeData &value)
 {
-    if (change == ItemDevicePixelRatioHasChanged || change == ItemSceneChange)
+    switch (change)
     {
-        m_dprChanged = true;
-        // Request update, so that `updatePaintNode()` gets called which updates the DPR for `::synchronize()`:
-        update();
+        case ItemSceneChange:
+        {
+            // It is intentional that window connection is made in `::updatePaintNode()`, and not here, because we don't
+            // want to explicitly connect whenever `ItemHasContents`, `isVisible()`, `window()` are all satisfied, which
+            // is implicitly the case with `::updatePaintNode()` (it is only called when all these are satisfied). This
+            // is strictly for maintenance reasons.
+
+            disconnect(m_synchConnection);
+
+            // if window changed but is valid, we can signal dpr change just to be sure, It is not clear if Qt signals
+            // ItemDevicePixelRatioHasChanged when item's window/scene changes to a new window that has different DPR:
+            if (value.window)
+                [[fallthrough]];
+            else
+                break;
+        }
+        case ItemDevicePixelRatioHasChanged:
+        {
+            m_dprChanged = true;
+            // Request update, so that `updatePaintNode()` gets called which updates the DPR for `::synchronize()`:
+            if (flags().testFlag(ItemHasContents)) // "Only items which specify QQuickItem::ItemHasContents are allowed to call QQuickItem::update()."
+                update();
+            break;
+        }
+        case ItemVisibleHasChanged:
+        {
+            if (!value.boolValue)
+            {
+                // Connection is made in `::updatePaintNode()` (which is called when both the item is visible and `ItemHasContents` is set).
+                disconnect(m_synchConnection);
+            }
+            break;
+        }
+        default: break;
     }
 
     QQuickItem::itemChange(change, value);
@@ -328,10 +359,13 @@ void VideoSurface::setVideoSurfaceProvider(VideoSurfaceProvider *newVideoSurface
         connect(&m_wheelEventConverter, &WheelToVLCConverter::vlcWheelKey, m_provider, &VideoSurfaceProvider::onMouseWheeled);
 
         setFlag(ItemHasContents, true);
+        update(); // this should not be necessary right after setting `ItemHasContents`, but just in case
     }
     else
     {
         setFlag(ItemHasContents, false);
+        // Connection is made in `::updatePaintNode()` (which is called when both the item is visible and `ItemHasContents` is set).
+        disconnect(m_synchConnection);
     }
 
     emit videoSurfaceProviderChanged();
@@ -350,25 +384,19 @@ QSGNode *VideoSurface::updatePaintNode(QSGNode *node, UpdatePaintNodeData *data)
     if (Q_UNLIKELY(!m_provider))
         return node;
 
-    if (w != m_oldWindow)
+    if (!m_synchConnection)
     {
-        if (m_oldWindow)
-            disconnect(m_synchConnection);
-
-        m_oldWindow = w;
+        // Disconnection is made in `::itemChange()`'s `ItemSceneChange` handler.
 
-        if (w)
+        // This is constant:
+        if (m_provider->supportsThreadedSurfaceUpdates())
         {
-            // This is constant:
-            if (m_provider->supportsThreadedSurfaceUpdates())
-            {
-                // Synchronize just before swapping the frame for better synchronization:
-                m_synchConnection = connect(w, &QQuickWindow::afterRendering, this, &VideoSurface::synchronize, Qt::DirectConnection);
-            }
-            else
-            {
-                m_synchConnection = connect(w, &QQuickWindow::afterAnimating, this, &VideoSurface::synchronize);
-            }
+            // Synchronize just before swapping the frame for better synchronization:
+            m_synchConnection = connect(w, &QQuickWindow::afterRendering, this, &VideoSurface::synchronize, Qt::DirectConnection);
+        }
+        else
+        {
+            m_synchConnection = connect(w, &QQuickWindow::afterAnimating, this, &VideoSurface::synchronize);
         }
     }
 


=====================================
modules/gui/qt/maininterface/videosurface.hpp
=====================================
@@ -130,7 +130,6 @@ private:
 
     QPointer<VideoSurfaceProvider> m_provider;
 
-    QPointer<QQuickWindow> m_oldWindow;
     QMetaObject::Connection m_synchConnection;
 
     // These are updated and read from different threads, but during synchronization stage so explicit synchronization



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/90659dd5a95a50ca7e145bd775dd18c58b3b433e...17bebed823514eeb1fe69b66fcde21af5f0a8be0

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/90659dd5a95a50ca7e145bd775dd18c58b3b433e...17bebed823514eeb1fe69b66fcde21af5f0a8be0
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance