Make QTexture use QTextureSourceGenerator

Instead of using a QTextureImageDataGenerator which was confusing

Change-Id: Ibbda6b2e8ddf6a917e1d1c0b1e086596c8c30510
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

9 files changed, 144 insertions, 51 deletions

diff --git a/src/render/jobs/loadtexturedatajob.cpp b/src/render/jobs/loadtexturedatajob.cpp
index 2d61bfded..65bcd70bc 100644
--- a/src/render/jobs/loadtexturedatajob.cpp
+++ b/src/render/jobs/loadtexturedatajob.cpp
@@ -43,6 +43,7 @@
 #include <Qt3DRender/private/texturedatamanager_p.h>
 #include <Qt3DRender/private/qtextureimage_p.h>
 #include <Qt3DRender/qtextureimagedata.h>
+#include <Qt3DRender/qtexturedata.h>
 #include <QThread>
 #include <Qt3DRender/private/job_common_p.h>
 
@@ -51,18 +52,12 @@ QT_BEGIN_NAMESPACE
 namespace Qt3DRender {
 namespace Render {
 
-LoadTextureDataJob::LoadTextureDataJob(Qt3DCore::QNodeId textureId)
-    : m_textureId(textureId)
-{
-    SET_JOB_RUN_STAT_TYPE(this, JobTypes::LoadTextureData, 0);
-}
+namespace {
 
-LoadTextureDataJob::~LoadTextureDataJob()
-{
-}
+typedef QPair<HTextureData, QTextureImageData *> HandleDataPair;
 
-static QPair<HTextureData, QTextureImageData *> textureDataFromGenerator(TextureDataManager *textureDataManager,
-                                                                         QTextureImageDataGeneratorPtr generator)
+HandleDataPair textureDataFromGenerator(TextureDataManager *textureDataManager,
+                                        QTextureImageDataGeneratorPtr generator)
 {
     HTextureData textureDataHandle;
     QTextureImageData *data = nullptr;
@@ -77,6 +72,7 @@ static QPair<HTextureData, QTextureImageData *> textureDataFromGenerator(Texture
     if (!textureDataHandle.isNull()) {
         data = textureDataManager->data(textureDataHandle);
     } else {
+        // Texture data is null -> we need to generate it
         QTextureImageDataPtr dataPtr = generator->operator ()();
         if (dataPtr.isNull()) {
             qCDebug(Jobs) << Q_FUNC_INFO << "Texture has no raw data";
@@ -92,35 +88,76 @@ static QPair<HTextureData, QTextureImageData *> textureDataFromGenerator(Texture
     return qMakePair(textureDataHandle, data);
 }
 
-void LoadTextureDataJob::run()
+void createTextureFromGenerator(TextureDataManager *textureDataManager,
+                                Texture *texture)
 {
-    qCDebug(Jobs) << "Entering" << Q_FUNC_INFO << QThread::currentThread();
+    QTextureGeneratorPtr generator = texture->dataGenerator();
+    const QTextureDataPtr generatedData = generator->operator ()();
 
-    Texture *txt = m_manager->textureManager()->lookupResource(m_textureId);
-    TextureDataManager *textureDataManager = m_manager->manager<QTextureImageData, TextureDataManager>();
+    // TO DO set the status of the texture based on the status of the functor
 
-    if (txt != nullptr) {
-        if (txt->dataGenerator()) {
-            QTextureImageDataGeneratorPtr generator = txt->dataGenerator();
+    // Use the first QTexImageData loaded to determine the target / mipmaps
+    // if not specified
+
+    if (texture->target() != QAbstractTexture::TargetAutomatic)
+        qWarning() << "When a texture provides a generator, it's target is expected to be TargetAutomatic";
+
+    texture->setTarget(static_cast<QAbstractTexture::Target>(generatedData->target()));
 
-            QPair<HTextureData, QTextureImageData *> handleData = textureDataFromGenerator(textureDataManager, generator);
+    texture->setSize(generatedData->width(), generatedData->height(), generatedData->depth());
+    texture->setLayers(generatedData->layers());
+    texture->setFormat(generatedData->format());
+    texture->setFormat(static_cast<QAbstractTexture::TextureFormat>(generatedData->format()));
 
-            HTextureData textureDataHandle = handleData.first;
-            QTextureImageData *data = handleData.second;
-            if (!data)
-                return;
+    // Note: These texture data handles aren't associated with a QTextureImageDataGenerator
+    // and will therefore be destroyed when the Texture element is destroyed or cleaned up
+    const QVector<QTextureImageDataPtr> imageData = generatedData->imageData();
 
-            if (txt->target() == QAbstractTexture::TargetAutomatic)
-                txt->setTarget(static_cast<QAbstractTexture::Target>(data->target()));
+    if (imageData.size() > 0) {
+        QMutexLocker locker(textureDataManager->mutex());
+        // We don't want to take the chance of having two jobs uploading the same functor
+        // because of sync issues
 
-            if (!txt->isAutoMipMapGenerationEnabled())
-                txt->setMipLevels(data->mipLevels());
+        // Set the mips level based on the first image if autoMipMapGeneration is disabled
+        if (!texture->isAutoMipMapGenerationEnabled())
+            texture->setMipLevels(imageData.first()->mipLevels());
 
-            txt->setSize(data->width(), data->height(), data->depth());
-            txt->setLayers(data->layers());
-            txt->setFormat(static_cast<QAbstractTexture::TextureFormat>(data->format()));
-            txt->setTextureDataHandle(textureDataHandle);
+        for (QTextureImageDataPtr dataPtr : imageData) {
+            HTextureData textureDataHandle = textureDataManager->acquire();
+            QTextureImageData *data = textureDataManager->data(textureDataHandle);
+            *data = *(dataPtr.data());
+            texture->addTextureDataHandle(textureDataHandle);
         }
+    }
+}
+
+} // anonymous
+
+LoadTextureDataJob::LoadTextureDataJob(Qt3DCore::QNodeId textureId)
+    : m_textureId(textureId)
+{
+    SET_JOB_RUN_STAT_TYPE(this, JobTypes::LoadTextureData, 0);
+}
+
+LoadTextureDataJob::~LoadTextureDataJob()
+{
+}
+
+void LoadTextureDataJob::run()
+{
+    qCDebug(Jobs) << "Entering" << Q_FUNC_INFO << QThread::currentThread();
+
+    Texture *txt = m_manager->textureManager()->lookupResource(m_textureId);
+    TextureDataManager *textureDataManager = m_manager->manager<QTextureImageData, TextureDataManager>();
+
+    if (txt != nullptr) {
+        // We need to clear the TextureData handles of the texture in case it was previously
+        // loaded with a different functor
+        txt->releaseTextureDataHandles();
+
+        // If the texture has a functor we used it to generate embedded TextureImages
+        if (txt->dataGenerator())
+            createTextureFromGenerator(textureDataManager, txt);
 
         // Load update each TextureImage
         const auto texImgHandles = txt->textureImages();
@@ -166,10 +203,11 @@ void LoadTextureDataJob::run()
                     }
                 }
                 // Set the textureDataHandle on the texture image
+                // Note: this internally updates the DNA of the TextureImage
                 texImg->setTextureDataHandle(textureDataHandle);
             }
         }
-        // Tell the renderer to reload the TextureImage for the Texture
+        // Tell the renderer to reload/upload to GPU the TextureImage for the Texture
         // next frame
         txt->requestTextureDataUpdate();
     }
diff --git a/src/render/texture/qabstracttexture.cpp b/src/render/texture/qabstracttexture.cpp
index 557498b5c..36ee6851f 100644
--- a/src/render/texture/qabstracttexture.cpp
+++ b/src/render/texture/qabstracttexture.cpp
@@ -473,7 +473,7 @@ QAbstractTexture::ComparisonMode QAbstractTexture::comparisonMode() const
     return d->m_comparisonMode;
 }
 
-QTextureImageDataGeneratorPtr QAbstractTexture::dataGenerator() const
+QTextureGeneratorPtr QAbstractTexture::dataGenerator() const
 {
     Q_D(const QAbstractTexture);
     return d->m_dataFunctor;
diff --git a/src/render/texture/qabstracttexture.h b/src/render/texture/qabstracttexture.h
index 854c19b75..eb4ad66f8 100644
--- a/src/render/texture/qabstracttexture.h
+++ b/src/render/texture/qabstracttexture.h
@@ -51,9 +51,9 @@ namespace Qt3DRender {
 class QAbstractTexturePrivate;
 class QTextureWrapMode;
 class QAbstractTextureImage;
-class QTextureImageDataGenerator;
+class QTextureGenerator;
 
-typedef QSharedPointer<QTextureImageDataGenerator> QTextureImageDataGeneratorPtr;
+typedef QSharedPointer<QTextureGenerator> QTextureGeneratorPtr;
 
 class QT3DRENDERSHARED_EXPORT QAbstractTexture : public Qt3DCore::QNode
 {
@@ -294,7 +294,7 @@ public:
     int height() const;
     int depth() const;
     int layers() const;
-    QTextureImageDataGeneratorPtr dataGenerator() const;
+    QTextureGeneratorPtr dataGenerator() const;
 
 public Q_SLOTS:
     void setFormat(TextureFormat format);
diff --git a/src/render/texture/qabstracttexture_p.h b/src/render/texture/qabstracttexture_p.h
index 3bc895e57..dca912611 100644
--- a/src/render/texture/qabstracttexture_p.h
+++ b/src/render/texture/qabstracttexture_p.h
@@ -55,6 +55,7 @@
 #include <Qt3DCore/private/qnode_p.h>
 #include <Qt3DRender/qabstracttexture.h>
 #include <Qt3DRender/qtexturewrapmode.h>
+#include <Qt3DRender/qtexturegenerator.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -84,7 +85,7 @@ public :
     QVector<QAbstractTextureImage *> m_textureImages;
     int m_layers;
 
-    QTextureImageDataGeneratorPtr m_dataFunctor;
+    QTextureGeneratorPtr m_dataFunctor;
 };
 
 struct QAbstractTextureData
@@ -105,7 +106,7 @@ struct QAbstractTextureData
     QAbstractTexture::ComparisonMode comparisonMode;
     Qt3DCore::QNodeIdVector textureImageIds;
     int layers;
-    QTextureImageDataGeneratorPtr dataFunctor;
+    QTextureGeneratorPtr dataFunctor;
 };
 
 } // QT3D
diff --git a/src/render/texture/qabstracttextureimage.h b/src/render/texture/qabstracttextureimage.h
index 2959b156d..698539ab3 100644
--- a/src/render/texture/qabstracttextureimage.h
+++ b/src/render/texture/qabstracttextureimage.h
@@ -41,6 +41,7 @@
 #define QT3DRENDER_QABSTRACTTEXTUREIMAGE_H
 
 #include <Qt3DRender/qabstracttexture.h>
+#include <Qt3DRender/qtextureimagedatagenerator.h>
 
 QT_BEGIN_NAMESPACE
 
diff --git a/src/render/texture/qtexture.cpp b/src/render/texture/qtexture.cpp
index 40b5c59c3..c2f243f57 100644
--- a/src/render/texture/qtexture.cpp
+++ b/src/render/texture/qtexture.cpp
@@ -295,7 +295,7 @@ void QTextureLoader::setSource(const QUrl& source)
     Q_D(QTextureLoader);
     if (source != d->m_source) {
         d->m_source = source;
-        d->m_dataFunctor.reset(new QImageTextureDataFunctor(source));
+        d->m_dataFunctor.reset(new QTextureFromSourceGenerator(source));
         emit sourceChanged(source);
     }
 }
@@ -307,6 +307,13 @@ bool QTextureFromSourceGenerator::operator ==(const QTextureGenerator &other) co
 
 }
 
+
+QTextureFromSourceGenerator::QTextureFromSourceGenerator(const QUrl &url)
+    : QTextureGenerator()
+    , m_url(url)
+{
+}
+
 // Note: Maybe this should return a struct containing information such as
 // the format, number of layers ....
 // This would also give more flexibility for the future
diff --git a/src/render/texture/qtexture_p.h b/src/render/texture/qtexture_p.h
index b4faa9cc1..7f295bbbe 100644
--- a/src/render/texture/qtexture_p.h
+++ b/src/render/texture/qtexture_p.h
@@ -73,6 +73,9 @@ public:
     QTextureDataPtr operator ()() Q_DECL_OVERRIDE;
     bool operator ==(const QTextureGenerator &other) const Q_DECL_OVERRIDE;
     inline QAbstractTexture::Status status() const { return m_status; }
+
+    QT3D_FUNCTOR(QTextureFromSourceGenerator)
+
 private:
     QUrl m_url;
     QAbstractTexture::Status m_status;
diff --git a/src/render/texture/texture.cpp b/src/render/texture/texture.cpp
index 5037d395a..f0aef843e 100644
--- a/src/render/texture/texture.cpp
+++ b/src/render/texture/texture.cpp
@@ -60,6 +60,18 @@ using namespace Qt3DCore;
 namespace Qt3DRender {
 namespace Render {
 
+/* A Texture can get its data in two complementary ways
+ * - Usually when a texture is created it is associated with a various number of
+ *   QTextureImages <-> TextureImage which will internally contain a set of QTexImageData
+ * - A QTexture can also provide a QTextureGenerator functor which might also
+ *   return a vector of QTexImageData
+ * So internally a Texture has a vector of HTextureImage which allow to retrieve a TextureImage and HTextureData
+ * but also a vector of HTextureData filled by the QTextureGenerator if present.
+ * From a memory management point of view, the texture needs to make sure it releases the HTextureData
+ * that were generated from the QTextureGenerator as these are not shared and belong to the Texture object.
+ * The HTextureData associated to a HTextureImage are managed by the TextureImage.
+ */
+
 Texture::Texture()
     : BackendNode()
     , m_gl(nullptr)
@@ -92,10 +104,17 @@ Texture::Texture()
 
 Texture::~Texture()
 {
+    // Release the texture data handles that may have been loaded
+    // by a QTextureGenerator functor
+    releaseTextureDataHandles();
 }
 
 void Texture::cleanup()
 {
+    // Release the texture data handles that may have been loaded
+    // by a QTextureGenerator functor
+    releaseTextureDataHandles();
+
     QBackendNode::setEnabled(false);
     m_gl = nullptr;
     m_width = 1;
@@ -123,7 +142,6 @@ void Texture::cleanup()
     m_textureImageManager = nullptr;
     m_textureDataManager = nullptr;
     m_dataFunctor.clear();
-    m_textureDataHandle = HTextureData();
 }
 
 // AspectThread
@@ -449,11 +467,11 @@ void Texture::updateDNA()
         if (img)
             m_textureDNA += img->dna();
     }
-    if (!m_textureDataHandle.isNull())
-        m_textureDNA += ::qHash(m_textureDataHandle.index());
+    for (const HTextureData textureDataHandle : qAsConst(m_textureDataHandles))
+        m_textureDNA += ::qHash(textureDataHandle.index());
 
     // if texture contains no potentially shared image data: texture is unique
-    if (m_textureImages.empty() && m_textureDataHandle.isNull()) // Ensures uniqueness by adding unique QNode id to the dna
+    if (m_textureImages.empty() && m_textureDataHandles.isEmpty()) // Ensures uniqueness by adding unique QNode id to the dna
         m_textureDNA += qHash(peerId());
 }
 
@@ -581,6 +599,7 @@ void Texture::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
             m_layers = propertyChange->value().toInt();
             m_isDirty |= (oldLayers != m_layers);
         }
+        // TO DO: Handle the textureGenerator change
     }
         break;
 
@@ -634,14 +653,16 @@ void Texture::setTextureDataManager(TextureDataManager *manager)
 // RenderThread
 void Texture::updateAndLoadTextureImage()
 {
-    if (!m_textureDataHandle.isNull()) {
-        QTextureImageData *data = m_textureDataManager->data(m_textureDataHandle);
-        if (data != nullptr)
+    // Upload all QTexImageData set by the QTextureGenerator
+    for (const HTextureData textureDataHandle : qAsConst(m_textureDataHandles)) {
+        QTextureImageData *data = m_textureDataManager->data(textureDataHandle);
+        if (data != Q_NULLPTR)
             setToGLTexture(data);
     }
 
+    // Upload all QTexImageData references by the TextureImages
     QVector<TextureImageDNA> dnas;
-    for (HTextureImage t : qAsConst(m_textureImages)) {
+    for (const HTextureImage t : qAsConst(m_textureImages)) {
         TextureImage *img = m_textureImageManager->data(t);
         if (img != nullptr && img->isDirty()) {
             if (dnas.contains(img->dna())) {
@@ -714,6 +735,26 @@ void TextureFunctor::destroy(Qt3DCore::QNodeId id) const
     m_textureManager->releaseResource(id);
 }
 
+void Texture::addTextureDataHandle(HTextureData handle)
+{
+    m_textureDataHandles.push_back(handle);
+    // Request a new upload to the GPU
+    requestTextureDataUpdate();
+}
+
+void Texture::releaseTextureDataHandles()
+{
+    if (m_textureDataHandles.size() > 0) {
+        m_isDirty = true;
+        Q_ASSERT(m_textureDataManager);
+        for (HTextureData textureData : qAsConst(m_textureDataHandles))
+            m_textureDataManager->release(textureData);
+        m_textureDataHandles.clear();
+        // Request a new upload to the GPU
+        requestTextureDataUpdate();
+    }
+}
+
 } // namespace Render
 } // namespace Qt3DRender
 
diff --git a/src/render/texture/texture_p.h b/src/render/texture/texture_p.h
index 4b14304f0..b3cd1c273 100644
--- a/src/render/texture/texture_p.h
+++ b/src/render/texture/texture_p.h
@@ -55,6 +55,7 @@
 #include <Qt3DRender/private/handle_types_p.h>
 #include <Qt3DRender/qtexture.h>
 #include <Qt3DRender/qtextureimagedata.h>
+#include <Qt3DRender/qtexturegenerator.h>
 #include <QOpenGLContext>
 #include <QMutex>
 
@@ -111,8 +112,10 @@ public:
     inline QAbstractTexture::Target target() const { return m_target; }
     inline bool isAutoMipMapGenerationEnabled() const { return m_generateMipMaps; }
 
-    inline QTextureImageDataGeneratorPtr dataGenerator() const { return m_dataFunctor; }
-    void setTextureDataHandle(HTextureData handle) { m_textureDataHandle = handle; }
+    inline QTextureGeneratorPtr dataGenerator() const { return m_dataFunctor; }
+    void addTextureDataHandle(HTextureData handle);
+    inline QVector<HTextureData> textureDataHandles() const { return m_textureDataHandles; }
+    void releaseTextureDataHandles();
 
     inline bool dataUploadRequired() const { return m_dataUploadRequired; }
 
@@ -143,9 +146,8 @@ private:
     QAbstractTexture::ComparisonFunction m_comparisonFunction;
     QAbstractTexture::ComparisonMode m_comparisonMode;
 
-    QTextureImageDataGeneratorPtr m_dataFunctor;
-    HTextureData m_textureDataHandle;
-
+    QTextureGeneratorPtr m_dataFunctor;
+    QVector<HTextureData> m_textureDataHandles;
     QVector<HTextureImage> m_textureImages;
 
     bool m_isDirty;