Make GLTexture dirty flags atomic instead of using a mutex

The mutex right now is used for controlling access to the m_dirty flags,
and for guarding access to m_gl and other data.

We don't need to use it for accessing the flags, as an atomic int will
do just fine and relieve us of potential deadlocks. The mutex must only
guard changes to the actual data.

Task-number: QTBUG-61130
Change-Id: Ia1f25af2233387f375c077965e901c67f972f1ec
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

2 files changed, 33 insertions, 21 deletions

diff --git a/src/render/texture/gltexture.cpp b/src/render/texture/gltexture.cpp
index 0df596ec2..01159b78c 100644
--- a/src/render/texture/gltexture.cpp
+++ b/src/render/texture/gltexture.cpp
@@ -97,15 +97,14 @@ void GLTexture::destroyGLTexture()
 {
     delete m_gl;
     m_gl = nullptr;
-    QMutexLocker locker(&m_dirtyFlagMutex);
-    m_dirty = 0;
+    m_dirtyFlags.store(0);
 
     destroyResources();
 }
 
 QOpenGLTexture* GLTexture::getOrCreateGLTexture()
 {
-    QMutexLocker locker(&m_dirtyFlagMutex);
+    QMutexLocker locker(&m_textureMutex);
     bool needUpload = false;
     bool texturedDataInvalid = false;
 
@@ -135,7 +134,7 @@ QOpenGLTexture* GLTexture::getOrCreateGLTexture()
                     m_properties.mipLevels = imageData.first()->mipLevels();
             }
 
-            m_dirty |= Properties;
+            setDirtyFlag(Properties, true);
             needUpload = true;
         } else {
             qWarning() << "[Qt3DRender::GLTexture] No QTextureData generated from Texture Generator yet. Texture will be invalid for this frame";
@@ -144,7 +143,7 @@ QOpenGLTexture* GLTexture::getOrCreateGLTexture()
     }
 
     // additional texture images may be defined through image data generators
-    if (m_dirty.testFlag(TextureData)) {
+    if (testDirtyFlag(TextureData)) {
         m_imageData.clear();
         needUpload = true;
 
@@ -168,7 +167,7 @@ QOpenGLTexture* GLTexture::getOrCreateGLTexture()
                         m_properties.format = static_cast<QAbstractTexture::TextureFormat>(imgData->format());
                     }
 
-                    m_dirty |= Properties;
+                    setDirtyFlag(Properties, true);
                 }
             } else {
                 qWarning() << "[Qt3DRender::GLTexture] No QTextureImageData generated from functor yet, texture will be invalid for this frame";
@@ -185,7 +184,7 @@ QOpenGLTexture* GLTexture::getOrCreateGLTexture()
         return nullptr;
 
     // if the properties changed, we need to re-allocate the texture
-    if (m_dirty.testFlag(Properties)) {
+    if (testDirtyFlag(Properties)) {
         delete m_gl;
         m_gl = nullptr;
     }
@@ -207,31 +206,34 @@ QOpenGLTexture* GLTexture::getOrCreateGLTexture()
     }
 
     // need to set texture parameters?
-    if (m_dirty.testFlag(Properties) || m_dirty.testFlag(Parameters)) {
+    if (testDirtyFlag(Properties) || testDirtyFlag(Parameters)) {
         updateGLTextureParameters();
     }
 
-    m_dirty = 0;
+    // un-set properties and parameters. The TextureData flag might have been set by another thread
+    // in the meantime, so don't clear that.
+    setDirtyFlag(Properties, false);
+    setDirtyFlag(Parameters, false);
 
     return m_gl;
 }
 
 void GLTexture::setParameters(const TextureParameters &params)
 {
+    QMutexLocker locker(&m_textureMutex);
     if (m_parameters != params) {
         m_parameters = params;
-        QMutexLocker locker(&m_dirtyFlagMutex);
-        m_dirty |= Parameters;
+        setDirtyFlag(Parameters);
     }
 }
 
 void GLTexture::setProperties(const TextureProperties &props)
 {
+    QMutexLocker locker(&m_textureMutex);
     if (m_properties != props) {
         m_properties = props;
-        QMutexLocker locker(&m_dirtyFlagMutex);
         m_actualTarget = props.target;
-        m_dirty |= Properties;
+        setDirtyFlag(Properties);
     }
 }
 
diff --git a/src/render/texture/gltexture_p.h b/src/render/texture/gltexture_p.h
index 4c0957f76..294732b2a 100644
--- a/src/render/texture/gltexture_p.h
+++ b/src/render/texture/gltexture_p.h
@@ -144,19 +144,17 @@ public:
     // a generator that needs to be uploaded.
     void requestUpload()
     {
-        QMutexLocker locker(&m_dirtyFlagMutex);
-        m_dirty |= TextureData;
+        setDirtyFlag(TextureData, true);
     }
 
     bool isDirty()
     {
-        QMutexLocker locker(&m_dirtyFlagMutex);
-        return m_dirty == 0 ? false : true;
+        return m_dirtyFlags.load() != 0;
     }
 
     QMutex *textureLock()
     {
-        return &m_dirtyFlagMutex;
+        return &m_textureMutex;
     }
 protected:
 
@@ -184,7 +182,19 @@ private:
         Parameters   = 0x04      // texture parameters need to be (re-)set
 
     };
-    Q_DECLARE_FLAGS(DirtyFlags, DirtyFlag)
+
+    bool testDirtyFlag(DirtyFlag flag)
+    {
+        return m_dirtyFlags.load() & flag;
+    }
+
+    void setDirtyFlag(DirtyFlag flag, bool value = true)
+    {
+        if (value)
+            m_dirtyFlags |= flag;
+        else
+            m_dirtyFlags &= ~static_cast<int>(flag);
+    }
 
     QOpenGLTexture *buildGLTexture();
     void uploadGLTextureData();
@@ -192,8 +202,8 @@ private:
     void destroyResources();
 
     bool m_unique;
-    DirtyFlags m_dirty;
-    QMutex m_dirtyFlagMutex;
+    QAtomicInt m_dirtyFlags;
+    QMutex m_textureMutex;
     QOpenGLTexture *m_gl;
 
     TextureDataManager *m_textureDataManager;