[vlc-commits] [Git][videolan/vlc][master] 4 commits: qt: provide the native texture size in `TextureProviderObserver`

Steve Lhomme (@robUx4) gitlab at videolan.org
Tue Sep 9 05:41:59 UTC 2025



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
cc44b805 by Fatih Uzunoglu at 2025-09-09T05:24:39+00:00
qt: provide the native texture size in `TextureProviderObserver`

This is comparable to `textureSize()` in GLSL, it provides the
native texture size as opposed to the scene graph texture size
which returns the sub-texture size, be it an arbitrary sub-
texture or atlas texture.

- - - - -
9eadb33c by Fatih Uzunoglu at 2025-09-09T05:24:39+00:00
qml: properly provide the texture size in `DualKawaseBlur`

The shader is supposed to use the native texture size, not the
scene graph texture size.

- - - - -
8e2f26de by Fatih Uzunoglu at 2025-09-09T05:24:39+00:00
qt: use sampling based approach in `TextureProviderObserver`

Instead of event based approach, it now uses sampling based
approach, like we have done in high precision timer widget
before (389a1999).

I also started using `std::atomic` instead of `QMutex`.

- - - - -
da306eb7 by Fatih Uzunoglu at 2025-09-09T05:24:39+00:00
qt: get rid of the texture provider mutex in `TextureProviderItem`

I was not sure about this, but I have now checked the source code
of `QQuickImage` and `QQuickShaderEffectSource`, and it seems neither
of them uses synchronization, so it should be safe for us too.

- - - - -


5 changed files:

- modules/gui/qt/util/textureproviderobserver.cpp
- modules/gui/qt/util/textureproviderobserver.hpp
- modules/gui/qt/widgets/native/textureprovideritem.cpp
- modules/gui/qt/widgets/native/textureprovideritem.hpp
- modules/gui/qt/widgets/qml/DualKawaseBlur.qml


Changes:

=====================================
modules/gui/qt/util/textureproviderobserver.cpp
=====================================
@@ -19,6 +19,11 @@
 
 #include <QSGTextureProvider>
 
+#if __has_include(<rhi/qrhi.h>) // RHI is semi-public since Qt 6.6
+#define RHI_HEADER_AVAILABLE
+#include <rhi/qrhi.h>
+#endif
+
 TextureProviderObserver::TextureProviderObserver(QObject *parent)
     : QObject{parent}
 {
@@ -31,8 +36,7 @@ void TextureProviderObserver::setSource(const QQuickItem *source)
         return;
 
     {
-        QMutexLocker locker(&m_textureMutex);
-        m_textureSize = {};
+        m_textureSize = QSize{}; // memory order does not matter, `setSource()` is not called frequently.
 
         if (m_source)
         {
@@ -67,11 +71,8 @@ void TextureProviderObserver::setSource(const QQuickItem *source)
                 assert(m_provider);
 
                 connect(m_provider, &QSGTextureProvider::textureChanged, this, &TextureProviderObserver::updateTextureSize, Qt::DirectConnection);
-                connect(m_provider, &QSGTextureProvider::textureChanged, this, &TextureProviderObserver::textureChanged);
 
                 updateTextureSize();
-
-                emit textureChanged(); // This should be safe if QML engine uses auto connection (default), otherwise we need to queue ourselves.
             }, static_cast<Qt::ConnectionType>(Qt::SingleShotConnection | Qt::DirectConnection));
         };
 
@@ -86,25 +87,60 @@ void TextureProviderObserver::setSource(const QQuickItem *source)
 
 QSize TextureProviderObserver::textureSize() const
 {
-    QMutexLocker locker(&m_textureMutex);
-    return m_textureSize;
+    // This is likely called in the QML/GUI thread.
+    // QML/GUI thread can freely block the rendering thread to the extent the time is reasonable and a
+    // fraction of `1/FPS`, because it is already throttled by v-sync (so it would just throttle less).
+    return m_textureSize.load(std::memory_order_acquire);
+}
+
+QSize TextureProviderObserver::nativeTextureSize() const
+{
+    // This is likely called in the QML/GUI thread.
+    // QML/GUI thread can freely block the rendering thread to the extent the time is reasonable and a
+    // fraction of `1/FPS`, because it is already throttled by v-sync (so it would just throttle less).
+    return m_nativeTextureSize.load(std::memory_order_acquire);
 }
 
 void TextureProviderObserver::updateTextureSize()
 {
-    // Better to lock as early as possible, QML can wait until the
-    // update completes to get the up-to-date size (if it requests
-    // read for any reason meanwhile).
-    QMutexLocker locker(&m_textureMutex);
+    // This is likely called in the rendering thread.
+    // Rendering thread should avoid blocking the QML/GUI thread. In this case, unlike the high precision
+    // timer case, it should be fine because the size may be inaccurate in the worst case until the next
+    // frame when the size is sampled again. In high precision timer case, accuracy is favored over
+    // potential stuttering.
+    constexpr auto memoryOrder = std::memory_order_relaxed;
 
     if (m_provider)
     {
         if (const auto texture = m_provider->texture())
         {
-            m_textureSize = texture->textureSize();
+            const auto textureSize = texture->textureSize();
+            m_textureSize.store(textureSize, memoryOrder);
+
+            {
+                // Native texture size
+
+                const auto legacyUpdateNativeTextureSize = [&]() {
+                    const auto ntsr = texture->normalizedTextureSubRect();
+                    m_nativeTextureSize.store({static_cast<int>(textureSize.width() / ntsr.width()),
+                                               static_cast<int>(textureSize.height() / ntsr.height())},
+                                              memoryOrder);
+                };
+
+#ifdef RHI_HEADER_AVAILABLE
+                const QRhiTexture* const rhiTexture = texture->rhiTexture();
+                if (Q_LIKELY(rhiTexture))
+                    m_nativeTextureSize.store(rhiTexture->pixelSize(), memoryOrder);
+                else
+                    legacyUpdateNativeTextureSize();
+#else
+                legacyUpdateNativeTextureSize();
+#endif
+            }
+
             return;
         }
     }
 
-    m_textureSize = {};
+    m_textureSize.store({}, memoryOrder);
 }


=====================================
modules/gui/qt/util/textureproviderobserver.hpp
=====================================
@@ -19,7 +19,7 @@
 #define TEXTUREPROVIDEROBSERVER_HPP
 
 #include <QObject>
-#include <QMutex>
+#include <QReadWriteLock>
 #include <QPointer>
 #include <QSize>
 #include <QQuickItem>
@@ -36,17 +36,34 @@ class TextureProviderObserver : public QObject
     Q_PROPERTY(const QQuickItem* source MEMBER m_source WRITE setSource NOTIFY sourceChanged FINAL)
 
     // WARNING: Texture properties are updated in the rendering thread.
-    Q_PROPERTY(QSize textureSize READ textureSize NOTIFY textureChanged FINAL)
+    // WARNING: Individual properties do not necessarily reflect the same texture at any arbitrary time.
+    //          In other words, properties are not updated atomically as a whole but independently
+    //          once the source texture changes. It depends on when you are reading the properties,
+    //          such that if you read properties before `updateTextureSize()` returns, the properties
+    //          may be inconsistent with each other (some reflecting the old texture, some the new
+    //          texture). This is done to reduce blocking the GUI thread from the rendering thread,
+    //          which is a problem unlike vice versa, and is considered acceptable because sampling
+    //          is supposed to be done periodically anyway (the sampling point must be chosen carefully
+    //          to not conflict with the updates, if the properties must reflect the immediately up-to-date
+    //          texture and the properties change each frame, as otherwise it might end up in a
+    //          "forever chase"), so by the time the sampling is done the properties should be consistent.
+    // NOTE: These properties do not provide notify signal, dynamic textures such as layer may
+    //       change rapidly (even though throttled by v-sync in the rendering thread), and if
+    //       such signal is connected to a receiver that lives in the GUI thread, the queued
+    //       invocations can easily backlog. Similar to the high precision timer, we moved
+    //       away from event based approach in favor of sampling based approach here.
+    Q_PROPERTY(QSize textureSize READ textureSize FINAL) // Scene graph texture size
+    Q_PROPERTY(QSize nativeTextureSize READ nativeTextureSize FINAL) // Native texture size (e.g. for atlas textures, the atlas size)
 
 public:
     explicit TextureProviderObserver(QObject *parent = nullptr);
 
     void setSource(const QQuickItem* source);
     QSize textureSize() const;
+    QSize nativeTextureSize() const;
 
 signals:
     void sourceChanged();
-    void textureChanged();
 
 private slots:
     void updateTextureSize();
@@ -57,14 +74,15 @@ private:
 
     // It is not clear when `QSGTextureProvider::textureChanged()` can be signalled.
     // If it is only signalled during SG synchronization where Qt blocks the GUI thread,
-    // we do not need explicit synchronization here. If it can be signalled at any time,
-    // we can still rely on SG synchronization (instead of explicit synchronization) by
-    // waiting until the next synchronization, but the delay might be more in that case.
-    // At the same time, the source might be living in a different window where the SG
-    // synchronization would not be blocking the (GUI) thread where this observer lives.
-    mutable QMutex m_textureMutex; // Maybe QReadWriteLock would be better.
+    // we do not need explicit synchronization (with atomic, or mutex) here. If it can be
+    // signalled at any time, we can still rely on SG synchronization (instead of explicit
+    // synchronization) by waiting until the next synchronization, but the delay might be
+    // more in that case. At the same time, the source might be living in a different window
+    // where the SG synchronization would not be blocking the (GUI) thread where this
+    // observer lives.
 
-    QSize m_textureSize; // invalid by default
+    std::atomic<QSize> m_textureSize {{}}; // invalid by default
+    std::atomic<QSize> m_nativeTextureSize {{}}; // invalid by default
 };
 
 #endif // TEXTUREPROVIDEROBSERVER_HPP


=====================================
modules/gui/qt/widgets/native/textureprovideritem.cpp
=====================================
@@ -39,7 +39,6 @@ private:
 TextureProviderItem::~TextureProviderItem()
 {
     {
-        QMutexLocker lock(&m_textureProviderMutex);
         if (m_textureProvider)
         {
             // https://doc.qt.io/qt-6/qquickitem.html#graphics-resource-handling
@@ -63,7 +62,6 @@ QSGTextureProvider *TextureProviderItem::textureProvider() const
 {
     // This method is called from the rendering thread.
 
-    QMutexLocker lock(&m_textureProviderMutex);
     if (!m_textureProvider)
     {
         m_textureProvider = new QSGTextureViewProvider;
@@ -134,7 +132,6 @@ void TextureProviderItem::invalidateSceneGraph()
 
     // This slot is called from the rendering thread.
     {
-        QMutexLocker lock(&m_textureProviderMutex);
         if (m_textureProvider)
         {
             delete m_textureProvider;
@@ -151,7 +148,6 @@ void TextureProviderItem::releaseResources()
     // QQuickItem::releaseResources() is guaranteed to have a valid window when it is called:
     assert(window());
     {
-        QMutexLocker lock(&m_textureProviderMutex);
         if (m_textureProvider)
         {
             window()->scheduleRenderJob(new TextureProviderCleaner(m_textureProvider), QQuickWindow::BeforeSynchronizingStage);


=====================================
modules/gui/qt/widgets/native/textureprovideritem.hpp
=====================================
@@ -132,7 +132,6 @@ private:
     QRect m_rect;
 
     mutable QPointer<QSGTextureViewProvider> m_textureProvider;
-    mutable QMutex m_textureProviderMutex; // I'm not sure if this mutex is necessary
 
     std::atomic<QSGTexture::AnisotropyLevel> m_anisotropyLevel = QSGTexture::AnisotropyNone;
     std::atomic<QSGTexture::Filtering> m_filtering = (smooth() ? QSGTexture::Linear : QSGTexture::Nearest);


=====================================
modules/gui/qt/widgets/qml/DualKawaseBlur.qml
=====================================
@@ -68,6 +68,49 @@ Item {
     // `QSGTextureView` can also be used instead of sub-texturing here.
     property rect sourceRect
 
+    // TODO: Get rid of this in favor of GLSL 1.30's `textureSize()`
+    Connections {
+        target: root.Window.window
+        enabled: root.visible
+
+        function onAfterAnimating() {
+            // Sampling point for getting the native texture sizes:
+            // This is emitted from the GUI thread.
+
+            // Unlike high resolution timer widget, we should not
+            // need to explicitly schedule update here, because if
+            // an update is necessary, it should have been scheduled
+            // implicitly (due to source texture provider's signal
+            // `textureChanged()`).
+
+            ds1.sourceTextureSize = ds1SourceObserver.nativeTextureSize
+            ds2.sourceTextureSize = ds2SourceObserver.nativeTextureSize
+            us1.sourceTextureSize = us1SourceObserver.nativeTextureSize
+            us2.sourceTextureSize = us2SourceObserver.nativeTextureSize
+
+            // It is not clear if `ShaderEffect` updates the uniform
+            // buffer after `afterAnimating()` signal but before the
+            // next frame. This is important because if `ShaderEffect`
+            // updates the uniform buffer during item polish, we already
+            // missed it here (`afterAnimating()` is signalled afterward).
+            // However, we can call `ensurePolished()` slot to ask for
+            // re-polish, which in case the `ShaderEffect` should now
+            // consider the new values. If it does not exist (Qt 6.2),
+            // we will rely on the next frame in worst case, which
+            // should be fine as long as the size does not constantly
+            // change in each frame.
+            if (ds1.ensurePolished)
+            {
+                // No need to check for if such slot exists for each,
+                // this is basically Qt version check in disguise.
+                ds1.ensurePolished()
+                ds2.ensurePolished()
+                us1.ensurePolished()
+                us2.ensurePolished()
+            }
+        }
+    }
+
     ShaderEffect {
         id: ds1
 
@@ -97,7 +140,7 @@ Item {
             source: ds1.source
         }
 
-        readonly property size sourceTextureSize: ds1SourceObserver.textureSize
+        property size sourceTextureSize
 
         // cullMode: ShaderEffect.BackFaceCulling // QTBUG-136611 (Layering breaks culling with OpenGL)
 
@@ -140,7 +183,7 @@ Item {
             source: ds2.source
         }
 
-        readonly property size sourceTextureSize: ds2SourceObserver.textureSize
+        property size sourceTextureSize
 
         // cullMode: ShaderEffect.BackFaceCulling // QTBUG-136611 (Layering breaks culling with OpenGL)
 
@@ -184,7 +227,7 @@ Item {
             source: us1.source
         }
 
-        readonly property size sourceTextureSize: us1SourceObserver.textureSize
+        property size sourceTextureSize
 
         // cullMode: ShaderEffect.BackFaceCulling // QTBUG-136611 (Layering breaks culling with OpenGL)
 
@@ -227,7 +270,7 @@ Item {
             source: us2.source
         }
 
-        readonly property size sourceTextureSize: us2SourceObserver.textureSize
+        property size sourceTextureSize
 
         // cullMode: ShaderEffect.BackFaceCulling // QTBUG-136611 (Layering breaks culling with OpenGL)
 



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/48fbe9bcd3e58fec007522571a60d41d972fdb2f...da306eb7c8b08825667e4dbe67ff3d14431af21d

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/48fbe9bcd3e58fec007522571a60d41d972fdb2f...da306eb7c8b08825667e4dbe67ff3d14431af21d
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