Fix texture data upload timing

Following the texture refactoring, textures are correctly uploaded at
application startup. However if the content of texture data is later
changed, e.g. by changing source property of a TextureImage, this is
never made available to the GPU.

Tracing this through, the texture image data generator is correctly
executed but it seems the OpenGL submission thread tries to upload the
data before the new data is made available to the TextureDataManager.
The subsequent clearing of the dirty flag means it never gets uploaded.

To fix this the TextureData dirty flag in GLTexture has been re-purposed
to mean that the generators have been executed and the data is ready and
available for upload. This flag was actually redundant for the purpose
of scheduling the generators to be executed as that is handled directly
by the texture data manager and GLTexture.

With this commit, the upload is never attempted until after the texture
image data is ready and we see textures behaving correctly once again.

Task-number: QTBUG-57509
Change-Id: I662c8d17e17283a2a3be5afbf61289ec2405d4fc
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

4 files changed, 26 insertions, 11 deletions

diff --git a/src/render/texture/gltexture.cpp b/src/render/texture/gltexture.cpp
index 7916e390d..a3b1f24a9 100644
--- a/src/render/texture/gltexture.cpp
+++ b/src/render/texture/gltexture.cpp
@@ -249,7 +249,10 @@ void GLTexture::setImages(const QVector<Image> &images)
         }
 
         m_images = images;
-        m_dirty |= TextureData;
+
+        // Don't mark the texture data as dirty yet. We defer this until the
+        // generators have been executed and the data is made available to the
+        // TextureDataManager.
 
         // make sure the generators are executed
         for (const Image& img : qAsConst(images)) {
diff --git a/src/render/texture/gltexture_p.h b/src/render/texture/gltexture_p.h
index f911262be..ca73c1131 100644
--- a/src/render/texture/gltexture_p.h
+++ b/src/render/texture/gltexture_p.h
@@ -140,6 +140,10 @@ public:
      */
     void destroyGLTexture();
 
+    // Called by TextureDataManager when it has new texture data from
+    // a generator that needs to be uploaded.
+    void requestUpload() { m_dirty |= TextureData; }
+
 protected:
 
     template<class APITexture, class APITextureImage>
@@ -160,9 +164,10 @@ protected:
 private:
 
     enum DirtyFlag {
-        TextureData = 0x1,     // one or more generators need to be executed
-        Properties   = 0x2,     // texture needs to be (re-)created
-        Parameters   = 0x4      // texture parameters need to be (re-)set
+        TextureData  = 0x01,     // one or more generators have been executed, data needs uploading to GPU
+        Properties   = 0x02,     // texture needs to be (re-)created
+        Parameters   = 0x04      // texture parameters need to be (re-)set
+
     };
     Q_DECLARE_FLAGS(DirtyFlags, DirtyFlag)
 
diff --git a/src/render/texture/texturedatamanager_p.h b/src/render/texture/texturedatamanager_p.h
index a93bace52..d58676929 100644
--- a/src/render/texture/texturedatamanager_p.h
+++ b/src/render/texture/texturedatamanager_p.h
@@ -57,6 +57,7 @@
 #include <Qt3DRender/qtextureimagedata.h>
 #include <Qt3DRender/qtexturegenerator.h>
 #include <Qt3DRender/qtextureimagedatagenerator.h>
+#include <Qt3DRender/private/gltexture_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -91,7 +92,7 @@ public:
      * generators are executed the next frame. Reference generator by
      * given texture
      */
-    void requestData(const GeneratorPtr &generator, const APITexture *tex)
+    void requestData(const GeneratorPtr &generator, APITexture *tex)
     {
         QMutexLocker lock(&m_mutex);
 
@@ -107,7 +108,7 @@ public:
      * Dereference given generator from texture. If no other textures still reference
      * the generator, the associated data will be deleted
      */
-    void releaseData(const GeneratorPtr &generator, const APITexture *tex)
+    void releaseData(const GeneratorPtr &generator, APITexture *tex)
     {
         QMutexLocker lock(&m_mutex);
 
@@ -159,17 +160,24 @@ public:
         QMutexLocker lock(&m_mutex);
 
         Entry *entry = findEntry(generator);
-        if (!entry)
+        if (!entry) {
             qWarning() << "[TextureDataManager] assignData() called with non-existent generator";
-        else
+        } else {
             entry->data = data;
+
+            // Mark each texture that references this data as being dirty
+            // so that the submission thread knows to upload
+            for (auto texture : qAsConst(entry->referencingTextures)) {
+                texture->requestUpload();
+            }
+        }
     }
 
 private:
 
     struct Entry {
         GeneratorPtr generator;
-        QVector<const APITexture*> referencingTextures;
+        QVector<APITexture*> referencingTextures;
         DataPtr data;
     };
 
@@ -198,8 +206,6 @@ private:
     QVector<Entry> m_data;
 };
 
-class GLTexture;
-
 class Q_AUTOTEST_EXPORT TextureDataManager
         : public GeneratorDataManager<QTextureGeneratorPtr, QTextureDataPtr, GLTexture>
 {
diff --git a/tests/auto/render/texturedatamanager/tst_texturedatamanager.cpp b/tests/auto/render/texturedatamanager/tst_texturedatamanager.cpp
index 2b4c9cd91..b76495d93 100644
--- a/tests/auto/render/texturedatamanager/tst_texturedatamanager.cpp
+++ b/tests/auto/render/texturedatamanager/tst_texturedatamanager.cpp
@@ -68,6 +68,7 @@ typedef QSharedPointer<FakeData> FakeDataPtr;
 
 struct FakeAPITexture
 {
+    void requestUpload() {}
 };
 
 using Manager = Qt3DRender::Render::GeneratorDataManager<FakeGeneratorPtr, FakeDataPtr, FakeAPITexture>;