SceneLoader: fix broken scene tree unloading

To unload the scene, we need to process the empty path in the backend to reset
the status and send a change to the frontend QSceneLoader that will take care
of deleting the previously created scene tree.

With the introduction of remote urls support, the case of the empty path was
not properly handled. It would end up being recognized as a remote path, a
download request would be made. Eventually it would silently fail. A warning
was added when a download request fails.

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

3 files changed, 45 insertions, 1 deletions

diff --git a/src/render/io/scene.cpp b/src/render/io/scene.cpp
index 7ae6f9473..d95f42a48 100644
--- a/src/render/io/scene.cpp
+++ b/src/render/io/scene.cpp
@@ -94,7 +94,14 @@ void Scene::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
         QPropertyUpdatedChangePtr propertyChange = qSharedPointerCast<QPropertyUpdatedChange>(e);
         if (propertyChange->propertyName() == QByteArrayLiteral("source")) {
             m_source = propertyChange->value().toUrl();
-            if (Qt3DCore::QDownloadHelperService::isLocal(m_source))
+
+            // If the source is empty -> we need to unload anything that was
+            // previously loaded and reset the status accordingly. This means
+            // we need to call addSceneData with the empty source to send a
+            // change to the frontend that will trigger the removal of the
+            // previous scene. The reason this scheme is employed is because
+            // the backend also takes care of updating the status.
+            if (m_source.isEmpty() || Qt3DCore::QDownloadHelperService::isLocal(m_source))
                 m_sceneManager->addSceneData(m_source, peerId());
             else
                 m_sceneManager->startSceneDownload(m_source, peerId());
diff --git a/src/render/io/scenemanager.cpp b/src/render/io/scenemanager.cpp
index fddb814c4..464871b48 100644
--- a/src/render/io/scenemanager.cpp
+++ b/src/render/io/scenemanager.cpp
@@ -118,6 +118,8 @@ void SceneDownloader::onCompleted()
         return;
     if (succeeded())
         m_manager->addSceneData(url(), m_sceneComponent, m_data);
+    else
+        qWarning() << "Failed to download scene at" << url();
     m_manager->clearSceneDownload(this);
 }
 
diff --git a/tests/auto/render/sceneloader/tst_sceneloader.cpp b/tests/auto/render/sceneloader/tst_sceneloader.cpp
index 257b1dbb8..8da82b609 100644
--- a/tests/auto/render/sceneloader/tst_sceneloader.cpp
+++ b/tests/auto/render/sceneloader/tst_sceneloader.cpp
@@ -172,6 +172,41 @@ private Q_SLOTS:
 
         arbiter.events.clear();
     }
+
+    void checkProcessEmptyPath()
+    {
+        // GIVEN
+        TestRenderer renderer;
+        Qt3DRender::Render::Scene sceneLoader;
+        Qt3DRender::Render::SceneManager sceneManager;
+
+        sceneLoader.setRenderer(&renderer);
+        sceneLoader.setSceneManager(&sceneManager);
+
+        // THEN
+        QVERIFY(sceneManager.takePendingSceneLoaderJobs().isEmpty());
+
+        // WHEN
+        Qt3DCore::QPropertyUpdatedChangePtr updateChange(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
+        const QUrl newUrl(QStringLiteral("file:///Bownling_Green_KY"));
+        updateChange->setValue(newUrl);
+        updateChange->setPropertyName("source");
+        sceneLoader.sceneChangeEvent(updateChange);
+
+        // THEN
+        QCOMPARE(sceneLoader.source(), newUrl);
+        QVERIFY(!sceneManager.takePendingSceneLoaderJobs().isEmpty());
+
+        // WHEN
+        updateChange.reset(new Qt3DCore::QPropertyUpdatedChange(Qt3DCore::QNodeId()));
+        updateChange->setValue(QUrl());
+        updateChange->setPropertyName("source");
+        sceneLoader.sceneChangeEvent(updateChange);
+
+        // THEN -> we should still have generated a job to reset the scene (immediately)
+        QCOMPARE(sceneLoader.source(), QUrl());
+        QVERIFY(!sceneManager.takePendingSceneLoaderJobs().isEmpty());
+    }
 };
 
 // Note: setSceneSubtree needs a QCoreApplication