From 7f0bdc21a8f1a5fbe2afdba61f037a71f00c1d61 Mon Sep 17 00:00:00 2001
From: Paul Lemire <paul.lemire@kdab.com>
Date: Thu, 3 May 2018 12:17:04 +0200
Subject: Remove race in texture data upload

In the non locked submission phase of the renderer, the GLTextures must have
had their content/properties already updated as this is too late to actually
create or upload texture based on references which may be invalid or outdated.

This wasn't properly handled. We now perfom the GLTexture creation and data
upload when both the RenderThread and AspectThread are locked.

Task-number: QTBUG-67989
Change-Id: I07ac23120e8d37a86e60d2b892d1437b8cb0b3dc
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>
---
 .../opengl/graphicshelpers/submissioncontext.cpp   |  3 +-
 src/render/renderers/opengl/renderer/renderer.cpp  | 33 ++++++++++++++--------
 2 files changed, 24 insertions(+), 12 deletions(-)

(limited to 'src/render/renderers/opengl')

diff --git a/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp b/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp
index 7072ed03e..3ebc965df 100644
--- a/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp
+++ b/src/render/renderers/opengl/graphicshelpers/submissioncontext.cpp
@@ -885,7 +885,8 @@ int SubmissionContext::activateTexture(TextureScope scope, GLTexture *tex, int o
     // actually re-bind if required, the tex->dna on the unit not being the same
     // Note: tex->dna() could be 0 if the texture has not been created yet
     if (m_activeTextures[onUnit].texture != tex) {
-        QOpenGLTexture *glTex = tex->getOrCreateGLTexture();
+        // Texture must have been created and updated at this point
+        QOpenGLTexture *glTex = tex->getGLTexture();
         if (glTex == nullptr)
             return -1;
         glTex->bind(onUnit);
diff --git a/src/render/renderers/opengl/renderer/renderer.cpp b/src/render/renderers/opengl/renderer/renderer.cpp
index bcc5fba80..a95bf230b 100644
--- a/src/render/renderers/opengl/renderer/renderer.cpp
+++ b/src/render/renderers/opengl/renderer/renderer.cpp
@@ -1152,9 +1152,19 @@ void Renderer::updateGLResources()
         const QVector<HTexture> activeTextureHandles = std::move(m_dirtyTextures);
         for (const HTexture &handle: activeTextureHandles) {
             Texture *texture = m_nodesManager->textureManager()->data(handle);
-            // Upload/Update texture
+            // Update texture properties
             updateTexture(texture);
         }
+        // We want to upload textures data at this point as the SubmissionThread and
+        // AspectThread are locked ensuring no races between Texture/TextureImage and
+        // GLTexture
+        if (m_submissionContext != nullptr) {
+            GLTextureManager *glTextureManager = m_nodesManager->glTextureManager();
+            const QVector<GLTexture *> glTextures = glTextureManager->activeResources();
+            // Upload texture data
+            for (GLTexture *glTexture : glTextures)
+                glTexture->getOrCreateGLTexture();
+        }
     }
     // When Textures are cleaned up, their id is saved
     // so that they can be cleaned up in the render thread
@@ -1196,13 +1206,18 @@ void Renderer::updateTexture(Texture *texture)
     GLTextureManager *glTextureManager = m_nodesManager->glTextureManager();
     GLTexture *glTexture = glTextureManager->lookupResource(texture->peerId());
 
-    // No GLTexture associated yet -> create it
-    if (glTexture == nullptr) {
+    auto createOrUpdateGLTexture = [=] () {
+        GLTexture *newGLTexture = nullptr;
         if (isUnique)
-            glTextureManager->createUnique(texture);
+            newGLTexture = glTextureManager->createUnique(texture);
         else
-            glTextureManager->getOrCreateShared(texture);
+            newGLTexture = glTextureManager->getOrCreateShared(texture);
         texture->unsetDirty();
+    };
+
+    // No GLTexture associated yet -> create it
+    if (glTexture == nullptr) {
+        createOrUpdateGLTexture();
         return;
     }
 
@@ -1210,12 +1225,8 @@ void Renderer::updateTexture(Texture *texture)
     // and abandon the old one
     if (glTextureManager->isShared(glTexture)) {
         glTextureManager->abandon(glTexture, texture);
-        // Check if a shared texture should become unique
-        if (isUnique)
-            glTextureManager->createUnique(texture);
-        else
-            glTextureManager->getOrCreateShared(texture);
-        texture->unsetDirty();
+        // Note: if isUnique is true, a once shared texture will become unique
+        createOrUpdateGLTexture();
         return;
     }
 
-- 
cgit v1.2.3