Fix crash when loading multiple assimp scenes

When a second scene is parsed, the importer for the first scene is
destroyed, and geometry renderers and materials in the parser's maps are
released, causing a crash.

Instead of converting all meshes and materials to Qt3D entities ahead of
time and storing them in maps, only convert entities that are needed by
the scene.

Task-number: QTBUG-62390
Change-Id: I1261d0dd01ced43b2731fdfe97b97885883dca9b
Reviewed-by: Massimo Callegari <massimocallegari@yahoo.it>
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
Reviewed-by: Antti Määttä <antti.maatta@qt.io>

2 files changed, 43 insertions, 84 deletions

diff --git a/src/plugins/sceneparsers/assimp/assimpimporter.cpp b/src/plugins/sceneparsers/assimp/assimpimporter.cpp
index 15a080ca7..af0f22f5d 100644
--- a/src/plugins/sceneparsers/assimp/assimpimporter.cpp
+++ b/src/plugins/sceneparsers/assimp/assimpimporter.cpp
@@ -486,14 +486,11 @@ Qt3DCore::QEntity *AssimpImporter::node(aiNode *node)
 
     // Add Meshes to the node
     for (uint i = 0; i < node->mNumMeshes; i++) {
-        uint meshIdx = node->mMeshes[i];
-        QMaterial *material = nullptr;
-        QGeometryRenderer *mesh = m_scene->m_meshes[meshIdx];
-        // mesh material
-        uint materialIndex = m_scene->m_aiScene->mMeshes[meshIdx]->mMaterialIndex;
+        uint meshIndex = node->mMeshes[i];
+        QGeometryRenderer *mesh = loadMesh(meshIndex);
 
-        if (m_scene->m_materials.contains(materialIndex))
-            material = m_scene->m_materials[materialIndex];
+        // mesh material
+        QMaterial *material;
 
         QList<Qt3DAnimation::QMorphingAnimation *> morphingAnimations
                 = mesh->findChildren<Qt3DAnimation::QMorphingAnimation *>();
@@ -517,6 +514,9 @@ Qt3DCore::QEntity *AssimpImporter::node(aiNode *node)
                                  &Qt3DExtras::QMorphPhongMaterial::setInterpolator);
             }
             morphingAnimations[0]->deleteLater();
+        } else {
+            uint materialIndex = m_scene->m_aiScene->mMeshes[meshIndex]->mMaterialIndex;
+            material = loadMaterial(materialIndex);
         }
 
         if (node->mNumMeshes == 1) {
@@ -564,8 +564,9 @@ Qt3DCore::QEntity *AssimpImporter::node(aiNode *node)
     }
 
     // Add Camera
-    if (m_scene->m_cameras.contains(node))
-        m_scene->m_cameras[node]->setParent(entityNode);
+    auto camera = loadCamera(node);
+    if (camera)
+        camera->setParent(entityNode);
 
     // TO DO : Add lights ....
 
@@ -622,16 +623,6 @@ void AssimpImporter::parse()
         // Set parsed flags
         m_sceneParsed = !m_sceneParsed;
 
-        for (uint i = 0; i < m_scene->m_aiScene->mNumTextures; i++)
-            loadEmbeddedTexture(i);
-        for (uint i = 0; i < m_scene->m_aiScene->mNumMaterials; i++)
-            loadMaterial(i);
-        for (uint i = 0; i < m_scene->m_aiScene->mNumMeshes; i++)
-            loadMesh(i);
-        for (uint i = 0; i < m_scene->m_aiScene->mNumCameras; i++)
-            loadCamera(i);
-        for (uint i = 0; i < m_scene->m_aiScene->mNumLights; i++)
-            loadLight(i);
         for (uint i = 0; i < m_scene->m_aiScene->mNumAnimations; i++)
             loadAnimation(i);
     }
@@ -639,10 +630,10 @@ void AssimpImporter::parse()
 
 /*!
  * Converts the provided Assimp aiMaterial identified by \a materialIndex to a
- * Qt3D material and adds it to a dictionary of materials.
+ * Qt3D material
  * \sa Material
  */
-void AssimpImporter::loadMaterial(uint materialIndex)
+QMaterial *AssimpImporter::loadMaterial(uint materialIndex)
 {
     // Generates default material based on what the assimp material contains
     aiMaterial *assimpMaterial = m_scene->m_aiScene->mMaterials[materialIndex];
@@ -656,22 +647,14 @@ void AssimpImporter::loadMaterial(uint materialIndex)
     // Add textures to materials dict
     copyMaterialTextures(material, assimpMaterial);
 
-    m_scene->m_materials.insert(materialIndex, material);
-
-    Qt3DRender::AssimpImporter::SceneImporter *scene = m_scene;
-    QObject::connect(material, &QObject::destroyed, [&, scene, materialIndex](QObject *object) {
-        QMaterial *r = static_cast<QMaterial *>(object);
-        if (scene->m_materials[materialIndex] == r)
-            scene->m_materials.remove(materialIndex);
-    });
+    return material;
 }
 
 /*!
  * Converts the Assimp aiMesh mesh identified by \a meshIndex to a QGeometryRenderer
- * and adds it to a dictionary of meshes.
  * \sa QGeometryRenderer
  */
-void AssimpImporter::loadMesh(uint meshIndex)
+QGeometryRenderer *AssimpImporter::loadMesh(uint meshIndex)
 {
     aiMesh *mesh = m_scene->m_aiScene->mMeshes[meshIndex];
 
@@ -813,21 +796,12 @@ void AssimpImporter::loadMesh(uint meshIndex)
 
     meshGeometry->addAttribute(indexAttribute);
 
-    m_scene->m_meshes[meshIndex] = geometryRenderer;
-
-    Qt3DRender::AssimpImporter::SceneImporter *scene = m_scene;
-    QObject::connect(geometryRenderer, &QObject::destroyed, [&, scene, meshIndex](QObject *object) {
-        QGeometryRenderer *r = static_cast<QGeometryRenderer *>(object);
-        if (scene->m_meshes[meshIndex] == r)
-            scene->m_meshes.remove(meshIndex);
-    });
-
     if (mesh->mNumAnimMeshes > 0) {
 
         aiAnimMesh *animesh = mesh->mAnimMeshes[0];
 
         if (animesh->mNumVertices != mesh->mNumVertices)
-            return;
+            return geometryRenderer;
 
         Qt3DAnimation::QMorphingAnimation *morphingAnimation
                 = new Qt3DAnimation::QMorphingAnimation(geometryRenderer);
@@ -952,14 +926,15 @@ void AssimpImporter::loadMesh(uint meshIndex)
     qCDebug(AssimpImporterLog) << Q_FUNC_INFO << " Mesh " << aiStringToQString(mesh->mName)
                                << " Vertices " << mesh->mNumVertices << " Faces "
                                << mesh->mNumFaces << " Indices " << indices;
+
+    return geometryRenderer;
 }
 
 /*!
- * Converts the provided Assimp aiTexture at \a textureIndex to a Texture and
- * adds it to a dictionary of textures.
+ * Converts the provided Assimp aiTexture at \a textureIndex to a Texture
  * \sa Texture
  */
-void AssimpImporter::loadEmbeddedTexture(uint textureIndex)
+QAbstractTexture *AssimpImporter::loadEmbeddedTexture(uint textureIndex)
 {
     aiTexture *assimpTexture = m_scene->m_aiScene->mTextures[textureIndex];
     QAbstractTexture *texture = QAbstractNodeFactory::createNode<QTexture2D>("QTexture2D");
@@ -981,37 +956,38 @@ void AssimpImporter::loadEmbeddedTexture(uint textureIndex)
     }
     imageData->setData(textureContent);
     texture->addTextureImage(imageData);
-    m_scene->m_embeddedTextures[textureIndex] = texture;
-
-    Qt3DRender::AssimpImporter::SceneImporter *scene = m_scene;
-    QObject::connect(texture, &QObject::destroyed, [&, scene, textureIndex](QObject *object) {
-        QAbstractTexture *r = static_cast<QAbstractTexture *>(object);
-        if (scene->m_embeddedTextures[textureIndex] == r)
-            scene->m_embeddedTextures.remove(textureIndex);
-    });
+
+    return texture;
 }
 
 /*!
  * Loads the light in the current scene located at \a lightIndex.
  */
-void AssimpImporter::loadLight(uint lightIndex)
+QAbstractLight *AssimpImporter::loadLight(uint lightIndex)
 {
     aiLight *light = m_scene->m_aiScene->mLights[lightIndex];
     // TODO: Implement me!
     Q_UNUSED(light);
+    return nullptr;
 }
 
 /*!
- * Parses the camera at cameraIndex and saves it to a dictionary of cameras.
+ * Converts the provided Assimp aiCamera in a node to a camera entity
  */
-void AssimpImporter::loadCamera(uint cameraIndex)
+Qt3DCore::QEntity *AssimpImporter::loadCamera(aiNode *node)
 {
-    aiCamera *assimpCamera = m_scene->m_aiScene->mCameras[cameraIndex];
-    aiNode *cameraNode = m_scene->m_aiScene->mRootNode->FindNode(assimpCamera->mName);
+    aiCamera *assimpCamera = nullptr;
 
-    // If no node is associated to the camera in the scene, camera not saved
-    if (cameraNode == nullptr)
-        return ;
+    for (uint i = 0; i < m_scene->m_aiScene->mNumCameras; ++i) {
+        auto camera = m_scene->m_aiScene->mCameras[i];
+        if (camera->mName == node->mName) {
+            assimpCamera = camera;
+            break;
+        }
+    }
+
+    if (assimpCamera == nullptr)
+        return nullptr;
 
     QEntity *camera = QAbstractNodeFactory::createNode<Qt3DCore::QEntity>("QEntity");
     QCameraLens *lens = QAbstractNodeFactory::createNode<QCameraLens>("QCameraLens");
@@ -1031,14 +1007,7 @@ void AssimpImporter::loadCamera(uint cameraIndex)
     transform->setMatrix(m);
     camera->addComponent(transform);
 
-    m_scene->m_cameras[cameraNode] = camera;
-
-    Qt3DRender::AssimpImporter::SceneImporter *scene = m_scene;
-    QObject::connect(camera, &QObject::destroyed, [&, scene, cameraNode](QObject *object) {
-        QEntity *r = static_cast<QEntity *>(object);
-        if (scene->m_cameras[cameraNode] == r)
-            scene->m_cameras.remove(cameraNode);
-    });
+    return camera;
 }
 
 int findTimeIndex(const QVector<float> &times, float time) {
@@ -1374,10 +1343,6 @@ AssimpImporter::SceneImporter::SceneImporter()
 AssimpImporter::SceneImporter::~SceneImporter()
 {
     delete m_importer;
-    qDeleteAll(m_materials.values());
-    qDeleteAll(m_meshes.values());
-    qDeleteAll(m_embeddedTextures.values());
-    qDeleteAll(m_cameras.values());
 }
 
 } // namespace Qt3DRender
diff --git a/src/plugins/sceneparsers/assimp/assimpimporter.h b/src/plugins/sceneparsers/assimp/assimpimporter.h
index e37950981..c461cda08 100644
--- a/src/plugins/sceneparsers/assimp/assimpimporter.h
+++ b/src/plugins/sceneparsers/assimp/assimpimporter.h
@@ -82,10 +82,8 @@ class QMorphingAnimation;
 namespace Qt3DRender {
 
 class QMaterial;
-class QShaderProgram;
-class QEffect;
 class QAbstractTexture;
-class QMesh;
+class QAbstractLight;
 class QGeometryRenderer;
 
 Q_DECLARE_LOGGING_CATEGORY(AssimpImporterLog)
@@ -115,11 +113,11 @@ private:
     void cleanup();
     void parse();
 
-    void loadMaterial(uint materialIndex);
-    void loadMesh(uint meshIndex);
-    void loadEmbeddedTexture(uint textureIndex);
-    void loadLight(uint lightIndex);
-    void loadCamera(uint cameraIndex);
+    QMaterial *loadMaterial(uint materialIndex);
+    QGeometryRenderer *loadMesh(uint meshIndex);
+    QAbstractTexture *loadEmbeddedTexture(uint textureIndex);
+    QAbstractLight *loadLight(uint lightIndex);
+    Qt3DCore::QEntity *loadCamera(aiNode *node);
     void loadAnimation(uint animationIndex);
 
     void copyMaterialName(QMaterial *material, aiMaterial *assimpMaterial);
@@ -139,11 +137,7 @@ private:
         Assimp::Importer *m_importer;
         mutable const aiScene *m_aiScene;
 
-        QMap<uint, QGeometryRenderer *> m_meshes;
-        QMap<uint, QMaterial*> m_materials;
-        QMap<uint, QEffect *> m_effects;
         QMap<uint, QAbstractTexture *> m_embeddedTextures;
-        QMap<aiNode*, Qt3DCore::QEntity*> m_cameras;
         QHash<aiTextureType, QString> m_textureToParameterName;
         QVector<Qt3DAnimation::QKeyframeAnimation *> m_animations;
         QVector<Qt3DAnimation::QMorphingAnimation *> m_morphAnimations;