Texture: properly handle texture sharing with remote URLs

Change-Id: I6f5ed6b04023578d389c41c4616b8ddb364bdc37
Task-number: QTBUG-69543
Reviewed-by: Mike Krus <mike.krus@kdab.com>

6 files changed, 106 insertions, 37 deletions

diff --git a/src/render/texture/apitexturemanager_p.h b/src/render/texture/apitexturemanager_p.h
index 58a73e09e..58e6e6420 100644
--- a/src/render/texture/apitexturemanager_p.h
+++ b/src/render/texture/apitexturemanager_p.h
@@ -99,11 +99,16 @@ public:
         return m_sharedTextures.keys().toVector() + m_uniqueTextures + m_abandonedTextures;
     }
 
-    APITexture *lookupResource(Qt3DCore::QNodeId textureId)
+    APITexture *lookupResource(Qt3DCore::QNodeId textureId) const
     {
         return m_nodeIdToGLTexture.value(textureId);
     }
 
+    Qt3DCore::QNodeIdVector referencedTextureIds(APITexture *apiTexture) const
+    {
+        return m_sharedTextures.value(apiTexture);
+    }
+
     // Returns a APITexture that matches the given QTexture node. Will make sure
     // that texture data generator jobs are launched, if necessary. The APITexture
     // may be shared between multiple texture backend nodes
diff --git a/src/render/texture/qtexture.cpp b/src/render/texture/qtexture.cpp
index e7c312a1c..f7ee3f6df 100644
--- a/src/render/texture/qtexture.cpp
+++ b/src/render/texture/qtexture.cpp
@@ -56,6 +56,8 @@
 #include <Qt3DRender/private/managers_p.h>
 #include <Qt3DRender/private/texture_p.h>
 #include <Qt3DRender/private/qurlhelper_p.h>
+#include <Qt3DRender/private/texturedatamanager_p.h>
+#include <Qt3DRender/private/gltexturemanager_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -902,20 +904,21 @@ QTextureImageDataPtr TextureLoadingHelper::loadTextureData(QIODevice *data, cons
 QTextureDataPtr QTextureFromSourceGenerator::operator ()()
 {
     QTextureDataPtr generatedData = QTextureDataPtr::create();
-    m_status = QAbstractTexture::Loading;
     QTextureImageDataPtr textureData;
 
-    QRenderAspectPrivate *d_aspect = QRenderAspectPrivate::findPrivate(m_engine);
-    Render::Texture *texture = d_aspect ? d_aspect->m_nodeManagers->textureManager()->lookupResource(m_texture) : nullptr;
-    if (texture)
-        texture->notifyStatus(m_status);
-
+    // Note: First and Second call can be seen as operator() being called twice
+    // on the same object but actually call 2 will be made on a new generator
+    // which is the copy of the generator used for call 1 but with m_sourceData
+    // set.
+    // This is required because updating the same functor wouldn't be picked up
+    // by the backend texture sharing system.
     if (!Qt3DCore::QDownloadHelperService::isLocal(m_url)) {
         if (m_sourceData.isEmpty()) {
             // first time around, trigger a download
             if (m_texture) {
                 auto downloadService = Qt3DCore::QDownloadHelperService::getService(m_engine);
-                Qt3DCore::QDownloadRequestPtr request(new TextureDownloadRequest(m_texture, m_url,
+                Qt3DCore::QDownloadRequestPtr request(new TextureDownloadRequest(sharedFromThis(),
+                                                                                 m_url,
                                                                                  m_engine));
                 downloadService->submitRequest(request);
             }
@@ -936,7 +939,7 @@ QTextureDataPtr QTextureFromSourceGenerator::operator ()()
                 ext << mtype.suffixes();
             }
 
-            for (QString s: qAsConst(ext)) {
+            for (const QString &s: qAsConst(ext)) {
                 textureData = TextureLoadingHelper::loadTextureData(&buffer, s, true, m_mirrored);
                 if (textureData && textureData->data().length() > 0)
                     break;
@@ -947,7 +950,7 @@ QTextureDataPtr QTextureFromSourceGenerator::operator ()()
     }
 
     // Update any properties explicitly set by the user
-    if (m_format != QAbstractTexture::NoFormat && m_format != QAbstractTexture::Automatic)
+    if (textureData && m_format != QAbstractTexture::NoFormat && m_format != QAbstractTexture::Automatic)
         textureData->setFormat(static_cast<QOpenGLTexture::TextureFormat>(m_format));
 
     if (textureData && textureData->data().length() > 0) {
@@ -958,30 +961,22 @@ QTextureDataPtr QTextureFromSourceGenerator::operator ()()
         generatedData->setDepth(textureData->depth());
         generatedData->setLayers(textureData->layers());
         generatedData->addImageData(textureData);
-
-        if (texture)
-            texture->updateFromData(generatedData);
-
-        m_status = QAbstractTexture::Ready;
-    } else {
-        m_status = QAbstractTexture::Error;
     }
 
-    if (texture)
-        texture->notifyStatus(m_status);
     return generatedData;
 }
 
-TextureDownloadRequest::TextureDownloadRequest(Qt3DCore::QNodeId texture, const QUrl& source,
+TextureDownloadRequest::TextureDownloadRequest(const QTextureFromSourceGeneratorPtr &functor,
+                                               const QUrl &source,
                                                Qt3DCore::QAspectEngine *engine)
     : Qt3DCore::QDownloadRequest(source)
-    , m_texture(texture)
+    , m_functor(functor)
     , m_engine(engine)
 {
 
 }
 
-// Executed in download thread
+// Executed in aspect thread
 void TextureDownloadRequest::onCompleted()
 {
     if (cancelled() || !succeeded())
@@ -991,16 +986,46 @@ void TextureDownloadRequest::onCompleted()
     if (!d_aspect)
         return;
 
-    Render::Texture *texture = d_aspect->m_nodeManagers->textureManager()->lookupResource(m_texture);
-    if (!texture)
+    // Find all textures which share the same functor
+    // Note: this should be refactored to not pull in API specific managers
+    // but texture sharing forces us to do that currently
+    Render::TextureDataManager *textureDataManager = d_aspect->m_nodeManagers->textureDataManager();
+    const QVector<Render::GLTexture *> referencedGLTextures = textureDataManager->referencesForGenerator(m_functor);
+
+    // We should have at most 1 GLTexture referencing this
+    // Since all textures having the same source should have the same functor == same GLTexture
+    Q_ASSERT(referencedGLTextures.size() <= 1);
+
+    Render::GLTexture *glTex = referencedGLTextures.size() > 0 ? referencedGLTextures.first() : nullptr;
+    if (glTex == nullptr)
         return;
 
-    QSharedPointer<QTextureFromSourceGenerator> functor =
-            qSharedPointerCast<QTextureFromSourceGenerator>(texture->dataGenerator());
-    functor->m_sourceData = m_data;
+    Render::GLTextureManager *glTextureManager = d_aspect->m_nodeManagers->glTextureManager();
+    Qt3DCore::QNodeIdVector referencingTexturesIds = glTextureManager->referencedTextureIds(glTex);
+
+
+    Render::TextureManager *textureManager = d_aspect->m_nodeManagers->textureManager();
+    for (const Qt3DCore::QNodeId texId : referencingTexturesIds) {
+        Render::Texture *texture = textureManager->lookupResource(texId);
+        if (texture != nullptr) {
+            // Each texture has a QTextureFunctor which matches m_functor;
+            // Update m_sourceData on each functor as we don't know which one
+            // is used as the reference for texture sharing
+
+            QTextureFromSourceGeneratorPtr oldGenerator = qSharedPointerCast<QTextureFromSourceGenerator>(texture->dataGenerator());
 
-    // mark the component as dirty so that the functor runs again in the correct job
-    texture->addDirtyFlag(Render::Texture::DirtyDataGenerator);
+            // We create a new functor
+            // Which is a copy of the old one + the downloaded sourceData
+            auto newGenerator = QTextureFromSourceGeneratorPtr::create(*oldGenerator);
+
+            // Set raw data on functor so that it can really load something
+           newGenerator->m_sourceData = m_data;
+
+           // Set new generator on texture
+           // it implictely marks the texture as dirty so that the functor runs again with the downloaded data
+           texture->setDataGenerator(newGenerator);
+        }
+    }
 }
 
 /*!
@@ -1382,6 +1407,7 @@ QTextureFromSourceGenerator::QTextureFromSourceGenerator(QTextureLoader *texture
                                                          Qt3DCore::QAspectEngine *engine,
                                                          Qt3DCore::QNodeId textureId)
     : QTextureGenerator()
+    , QEnableSharedFromThis<QTextureFromSourceGenerator>()
     , m_url()
     , m_status(QAbstractTexture::None)
     , m_mirrored()
@@ -1405,6 +1431,19 @@ QTextureFromSourceGenerator::QTextureFromSourceGenerator(QTextureLoader *texture
     m_format = textureLoader->format();
 }
 
+QTextureFromSourceGenerator::QTextureFromSourceGenerator(const QTextureFromSourceGenerator &other)
+    : QTextureGenerator()
+    , QEnableSharedFromThis<QTextureFromSourceGenerator>()
+    , m_url(other.m_url)
+    , m_status(other.m_status)
+    , m_mirrored(other.m_mirrored)
+    , m_sourceData(other.m_sourceData)
+    , m_texture(other.m_texture)
+    , m_engine(other.m_engine)
+    , m_format(other.m_format)
+{
+}
+
 /*
  * Takes in a TextureGenerator via \a other and
  * \return whether generators have the same source.
@@ -1416,7 +1455,8 @@ bool QTextureFromSourceGenerator::operator ==(const QTextureGenerator &other) co
             otherFunctor->m_url == m_url &&
             otherFunctor->m_mirrored == m_mirrored &&
             otherFunctor->m_engine == m_engine &&
-            otherFunctor->m_format == m_format);
+            otherFunctor->m_format == m_format &&
+            otherFunctor->m_sourceData == m_sourceData);
 }
 
 QUrl QTextureFromSourceGenerator::url() const
diff --git a/src/render/texture/qtexture_p.h b/src/render/texture/qtexture_p.h
index 1c2f598db..087480340 100644
--- a/src/render/texture/qtexture_p.h
+++ b/src/render/texture/qtexture_p.h
@@ -76,25 +76,33 @@ public:
     bool m_mirrored;
 };
 
+class QTextureFromSourceGenerator;
+typedef QSharedPointer<QTextureFromSourceGenerator> QTextureFromSourceGeneratorPtr;
+
 class Q_AUTOTEST_EXPORT TextureDownloadRequest : public Qt3DCore::QDownloadRequest
 {
 public:
-    TextureDownloadRequest(Qt3DCore::QNodeId texture, const QUrl &url, Qt3DCore::QAspectEngine *engine);
+    TextureDownloadRequest(const QTextureFromSourceGeneratorPtr &functor,
+                           const QUrl &url,
+                           Qt3DCore::QAspectEngine *engine);
 
     void onCompleted() override;
 
 private:
-    Qt3DCore::QNodeId m_texture;
+    QTextureFromSourceGeneratorPtr m_functor;
     Qt3DCore::QAspectEngine *m_engine;
 };
 
-class Q_AUTOTEST_EXPORT QTextureFromSourceGenerator : public QTextureGenerator
+class Q_AUTOTEST_EXPORT QTextureFromSourceGenerator : public QTextureGenerator,
+                                                      public QEnableSharedFromThis<QTextureFromSourceGenerator>
 {
 public:
     explicit QTextureFromSourceGenerator(QTextureLoader *textureLoader,
                                          Qt3DCore::QAspectEngine *engine,
                                          Qt3DCore::QNodeId textureId);
 
+    QTextureFromSourceGenerator(const QTextureFromSourceGenerator &other);
+
     QTextureDataPtr operator ()() override;
     bool operator ==(const QTextureGenerator &other) const override;
     inline QAbstractTexture::Status status() const { return m_status; }
@@ -118,7 +126,6 @@ private:
     // Options that can be overridden on TextureLoader when loading
     QAbstractTexture::TextureFormat m_format;
 };
-typedef QSharedPointer<QTextureFromSourceGenerator> QTextureFromSourceGeneratorPtr;
 
 class Q_AUTOTEST_EXPORT TextureLoadingHelper
 {
diff --git a/src/render/texture/texture.cpp b/src/render/texture/texture.cpp
index 5a50fb30f..440a08e26 100644
--- a/src/render/texture/texture.cpp
+++ b/src/render/texture/texture.cpp
@@ -199,8 +199,7 @@ void Texture::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
             m_properties.samples = propertyChange->value().toInt();
             dirty = DirtyProperties;
         } else if (propertyChange->propertyName() == QByteArrayLiteral("generator")) {
-            m_dataFunctor = propertyChange->value().value<QTextureGeneratorPtr>();
-            dirty = DirtyDataGenerator;
+            setDataGenerator(propertyChange->value().value<QTextureGeneratorPtr>());
         }
     }
         break;
@@ -297,6 +296,13 @@ void Texture::updateFromData(QTextureDataPtr data)
     }
 }
 
+// Called by sceneChangeEvent or TextureDownloadRequest (both in AspectThread context)
+void Texture::setDataGenerator(const QTextureGeneratorPtr &generator)
+{
+    m_dataFunctor = generator;
+    addDirtyFlag(DirtyDataGenerator);
+}
+
 bool Texture::isValid(TextureImageManager *manager) const
 {
     for (const QNodeId id : m_textureImageIds) {
diff --git a/src/render/texture/texture_p.h b/src/render/texture/texture_p.h
index 789285644..b9ef0385f 100644
--- a/src/render/texture/texture_p.h
+++ b/src/render/texture/texture_p.h
@@ -157,7 +157,7 @@ public:
 
     void notifyStatus(QAbstractTexture::Status status);
     void updateFromData(QTextureDataPtr data);
-
+    void setDataGenerator(const QTextureGeneratorPtr &generator);
     bool isValid(TextureImageManager *manager) const;
 private:
     void initializeFromPeer(const Qt3DCore::QNodeCreatedChangeBasePtr &change) final;
diff --git a/src/render/texture/texturedatamanager_p.h b/src/render/texture/texturedatamanager_p.h
index ef330264c..9319a64e0 100644
--- a/src/render/texture/texturedatamanager_p.h
+++ b/src/render/texture/texturedatamanager_p.h
@@ -108,6 +108,17 @@ public:
         return needsToBeCreated;
     }
 
+    QVector<ReferencedType> referencesForGenerator(const GeneratorPtr &generator)
+    {
+        QMutexLocker lock(&m_mutex);
+
+        Entry *entry = findEntry(generator);
+        if (entry == nullptr)
+            return {};
+        return entry->referencingObjects;
+    }
+
+
     /*!
      * Dereference given generator from texture. If no other textures still reference
      * the generator, the associated data will be deleted