[vlc-commits] [Git][videolan/vlc][master] 2 commits: qt: make `QSGTextureView::setRect()` thread-safe

Steve Lhomme (@robUx4) gitlab at videolan.org
Sat Oct 25 08:22:49 UTC 2025



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
3f658f1c by Fatih Uzunoglu at 2025-10-25T08:06:52+00:00
qt: make `QSGTextureView::setRect()` thread-safe

This is essential to fix view texture updates getting delayed
when the sub-rect changes, as well as event accumulation if
`setRect()` is called from different thread(s) in succession
repetitively, which is a case for example when resizing.

- - - - -
e2b7f26f by Fatih Uzunoglu at 2025-10-25T08:06:52+00:00
qt: do not queue adjusting view rectangle in `TextureProviderItem`

Now that `QSGTextureView::setRect()` is thread-safe, we can do this.

- - - - -


3 changed files:

- modules/gui/qt/util/qsgtextureview.cpp
- modules/gui/qt/util/qsgtextureview.hpp
- modules/gui/qt/widgets/native/textureprovideritem.cpp


Changes:

=====================================
modules/gui/qt/util/qsgtextureview.cpp
=====================================
@@ -17,6 +17,10 @@
  *****************************************************************************/
 #include "qsgtextureview.hpp"
 
+#include <QThread>
+#include <QCoreApplication>
+
+
 QSGTextureView::QSGTextureView(QSGTexture *texture)
 {
     setTexture(texture);
@@ -76,6 +80,14 @@ void QSGTextureView::setTexture(QSGTexture *texture)
 
 bool QSGTextureView::adjustNormalRect() const
 {
+    QMutexLocker lock(&m_rectMutex);
+    return adjustNormalRectWithoutLocking();
+}
+
+bool QSGTextureView::adjustNormalRectWithoutLocking() const
+{
+    assert(QThread::currentThread() == m_texture->thread());
+
     if (!m_rect.isValid())
         return false;
 
@@ -104,32 +116,106 @@ bool QSGTextureView::adjustNormalRect() const
 
 QRect QSGTextureView::rect() const
 {
+    QMutexLocker lock(&m_rectMutex);
     return m_rect;
 }
 
 void QSGTextureView::setRect(const QRect &rect)
 {
+    QMutexLocker<QMutex> lock(&m_rectMutex); // WARNING: `::unlock()` is used below.
+
     if (m_rect == rect)
         return;
 
     m_rect = rect;
 
+    const bool threadIsObjectsThread = (QThread::currentThread() == thread());
+
     // We need the source texture in order to calculate the normal rect.
     // If texture is not available when the rect is set, it will be done
     // later in `normalizedTextureSubRect()`.
-    if (m_texture)
+    if (threadIsObjectsThread && m_texture)
     {
         if (m_rect.isValid())
-            adjustNormalRect();
+        {
+            // If current thread is object's thread, we can check `m_texture`
+            // and call `adjustNormalRect()`, which is not thread safe, which
+            // is also the reason we do not need to protect `m_texture` with
+            // the same mutex:
+            adjustNormalRectWithoutLocking();
+        }
         else
+        {
             m_normalRect.reset();
-        emit updateRequested();
+        }
+
+        lock.unlock();
+        emit updateRequested(); // emit the signal when the mutex is not locked
     }
     else
     {
+        // If current thread is not object's thread, we reset the normal
+        // rect and request a queued update. The repeated queued updates
+        // are compressed as per the thread's (the render thread) event
+        // loop in the worst case, and the render loop (per frame) in the
+        // best/expected case. When an update is requested, the subsequent
+        // `normalizedTextureSubRect()` call should calculate the normal
+        // rectangle. `normalizedTextureSubRect()` may also be called
+        // at any arbitrary time without an update request from the view
+        // side by the scene graph renderer, which makes sure that the
+        // up-to-date value of the rectangle is used without any cost,
+        // because the render loop is already throttled by v-sync, it
+        // would just throttle less if it needs to wait for mutex to be
+        // unlocked. Note that the render thread's event loop can
+        // already be tied to frame presentation, where in that case
+        // the best and worst cases do not differ, though this is an
+        // implementation detail of Qt. Regarding render loop updates
+        // (best case), see the "fast-track" note in `::updateRequest()`.
+        // Regarding calling `setRect()` when the render thread locked
+        // the mutex (due to reading) in `normalizedTextureSubRect()`,
+        // then the main/GUI thread would need to wait. However, that
+        // is considered negligible as the normalization calculation
+        // should be really fast, and usually `setRect()` is expected
+        // to be called upon scene graph synchronization anyway (as
+        // that is when the source texture emits `updateRequested()`).
+
         // Invalidate the normal rect, so that it is calculated via
         // `normalizedTextureSubRect()` when there is a texture:
         m_normalRect.reset();
+
+        // We do not request an update if thread is object's thread
+        // because in that case this branch is only taken when there
+        // is no texture, which means update request would be useless:
+        if (!threadIsObjectsThread)
+        {
+            if (!m_pendingUpdateRequestRectChange)
+            {
+                // The compression is essential to not let queued (due to different thread) `updateRequested()`
+                // signals accumulate and backlog the receiver's event loop when `setRect()` is called repeatedly,
+                // such as during resize. Again, the compression here is aggressive for texture update, but we
+                // also request update during `updateTexture()`, which is called per frame hence more relaxed.
+                m_pendingUpdateRequestRectChange = true;
+
+                lock.unlock();
+
+                QMetaObject::invokeMethod(this, [this]() {
+                    bool requestUpdate = false;
+                    if (m_texture)
+                    {
+                        QMutexLocker<QMutex> lock(&m_rectMutex);
+                        if (m_pendingUpdateRequestRectChange)
+                        {
+                            requestUpdate = true;
+                            m_pendingUpdateRequestRectChange = false;
+                        }
+                    }
+                    if (requestUpdate)
+                        emit updateRequested(); // emit the signal when the mutex is not locked
+                    // If there is no texture at the moment, do not signal `updateRequested()`,
+                    // as it is signalled implicitly when there is a texture.
+                }, Qt::QueuedConnection);
+            }
+        }
     }
 }
 
@@ -204,6 +290,7 @@ QRhiTexture *QSGTextureView::rhiTexture() const
 QSize QSGTextureView::textureSize() const
 {
     assert(m_texture);
+    QMutexLocker lock(&m_rectMutex);
     if (m_rect.isNull())
         return m_texture->textureSize();
     else
@@ -226,8 +313,10 @@ QRectF QSGTextureView::normalizedTextureSubRect() const
 {
     assert(m_texture);
 
+    QMutexLocker lock(&m_rectMutex);
+
     if (!m_normalRect.has_value())
-        adjustNormalRect();
+        adjustNormalRectWithoutLocking();
 
     QRectF subRect = m_texture->normalizedTextureSubRect();
 
@@ -304,16 +393,35 @@ bool QSGTextureView::updateTexture()
 {
     bool ret = false;
 
-    if (const auto dynamicTexture = qobject_cast<QSGDynamicTexture*>(m_texture))
+    if (m_texture)
     {
-        if (dynamicTexture->updateTexture())
-            ret = true;
-    }
+        if (const auto dynamicTexture = qobject_cast<QSGDynamicTexture*>(m_texture))
+        {
+            if (dynamicTexture->updateTexture())
+                ret = true;
+        }
 
-    if (m_normalRectChanged)
-    {
-        ret = true;
-        m_normalRectChanged = false;
+        {
+            // We can "fast-track" the request here, essentially relaxing the aggressive render thread's
+            // event loop compression to render loop (per frame) compression:
+            bool requestUpdate = false;
+            {
+                QMutexLocker lock(&m_rectMutex);
+                if (m_pendingUpdateRequestRectChange)
+                {
+                    requestUpdate = true;
+                    m_pendingUpdateRequestRectChange = false;
+                }
+            }
+            if (requestUpdate)
+                emit updateRequested(); // emit the signal when the mutex is not locked
+        }
+
+        if (m_normalRectChanged)
+        {
+            ret = true;
+            m_normalRectChanged = false;
+        }
     }
 
     return ret;


=====================================
modules/gui/qt/util/qsgtextureview.hpp
=====================================
@@ -19,6 +19,7 @@
 #define QSGTEXTUREVIEW_HPP
 
 #include <QSGTexture>
+#include <QMutex>
 
 #include <optional>
 
@@ -28,12 +29,15 @@ class QSGTextureView : public QSGDynamicTexture
 
     QPointer<QSGTexture> m_texture;
     QRect m_rect;
+    mutable QMutex m_rectMutex; // protects `m_rect`, `m_normalRect`, and `m_pendingUpdateRequestRectChange`
+    bool m_pendingUpdateRequestRectChange = false;
     mutable std::optional<QRectF> m_normalRect;
     mutable bool m_normalRectChanged = false;
     bool m_detachFromAtlasPending = false;
 
 private slots:
     bool adjustNormalRect() const;
+    bool adjustNormalRectWithoutLocking() const;
 
 public slots:
     // Reset the view texture state to the target texture state.
@@ -57,8 +61,8 @@ public:
     void setTexture(QSGTexture* texture);
 
     // Subtexturing:
-    QRect rect() const;
-    void setRect(const QRect& rect);
+    QRect rect() const; // this method is thread-safe
+    void setRect(const QRect& rect); // this method is thread-safe
 
     qint64 comparisonKey() const override;
     QRhiTexture *rhiTexture() const override;


=====================================
modules/gui/qt/widgets/native/textureprovideritem.cpp
=====================================
@@ -99,7 +99,7 @@ QSGTextureProvider *TextureProviderItem::textureProvider() const
 
         // These are going to be queued when necessary:
         connect(this, &TextureProviderItem::sourceChanged, m_textureProvider, adjustSource);
-        connect(this, &TextureProviderItem::rectChanged, m_textureProvider, &QSGTextureViewProvider::setRect);
+        connect(this, &TextureProviderItem::rectChanged, m_textureProvider, &QSGTextureViewProvider::setRect, Qt::DirectConnection);
 
         connect(this, &TextureProviderItem::filteringChanged, m_textureProvider, &QSGTextureViewProvider::setFiltering);
         connect(this, &TextureProviderItem::mipmapFilteringChanged, m_textureProvider, &QSGTextureViewProvider::setMipmapFiltering);



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/e824b12ab741a1791929ac3a541198ea19ffe25b...e2b7f26fa97af9e6c9aee42f10fdc1aa49a0b439

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


VideoLAN code repository instance


More information about the vlc-commits mailing list