Change bounding update propagation

Previously, bounding computation results from the core aspect were
propagated to the render aspect via the front end objects.

This introduces a job watcher which gets called with the results
and the render aspect can update it's backend data directly.
The watcher process method is called in the thread of the core
aspect job but the render aspect job will wait for that complete
anyway (since it depends on the core aspect job).

Change-Id: Ie59337f00025fd55fc723a7d105342e0b1e91d6c
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
(cherry picked from commit e378ebd0db6c06af7709498122912284852e6bc9)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

7 files changed, 108 insertions, 43 deletions

diff --git a/src/core/jobs/calcboundingvolumejob.cpp b/src/core/jobs/calcboundingvolumejob.cpp
index 4bad98b16..3ebb9dc0a 100644
--- a/src/core/jobs/calcboundingvolumejob.cpp
+++ b/src/core/jobs/calcboundingvolumejob.cpp
@@ -242,7 +242,7 @@ void CalculateBoundingVolumeJob::run()
 
     QHash<QEntity *, BoundingVolumeComputeData> dirtyEntities;
     QNodeVisitor visitor;
-    visitor.traverse(m_root, [](QNode *) {}, [&dirtyEntities](QEntity *entity) {
+    visitor.traverse(m_root, [](QNode *) {}, [&dirtyEntities, this](QEntity *entity) {
         if (!isTreeEnabled(entity))
             return;
 
@@ -259,15 +259,32 @@ void CalculateBoundingVolumeJob::run()
                 continue;
 
             BoundingVolumeComputeData bvdata;
-            if (!dbv->m_explicitPointsValid && bv->view()) {
+            if (dbv->m_explicitPointsValid) {
+                // we have data explicitly set by the user, pass it to the
+                // watchers as computed data
+                BoundingVolumeComputeResult r;
+                r.entity = entity;
+                r.provider = bv;
+                r.m_min = dbv->m_minPoint;
+                r.m_max = dbv->m_maxPoint;
+                const auto diagonal = r.m_max - r.m_min;
+                r.m_center = r.m_min + diagonal * .5f;
+                r.m_radius = diagonal.length();
+
+                for (auto w: m_watchers) {
+                    auto wp = w.toStrongRef();
+                    if (wp)
+                        wp->process(r, false);
+                }
+                continue;
+            } else if (bv->view()) {
                 bvdata = findBoundingVolumeComputeData(bv->view());
                 if (!bvdata.valid())
                     continue;
                 bvdata.entity = entity;
                 bvdata.provider = bv;
             } else {
-                // bounds are explicitly set, don't bother computing
-                // or no view, can't compute
+                // no view, can't compute
                 continue;
             }
 
@@ -307,12 +324,14 @@ void CalculateBoundingVolumeJob::run()
         }
     }
 
-    // This is needed so that the matching job in the render aspect gets the right data.
-    // It is currently safe since the main thread is locked, there's no other
-    // core aspect jobs in existence, and other aspect jobs only access backend data.
-    // TODO: find a way for aspects to pass around data, or create groups of jobs
-    // that need to run first, sync, then process next group.
-    postFrame(nullptr);
+    // pass the computed results to the watchers
+    for (auto &watcher: m_watchers) {
+        auto watcherPtr = watcher.toStrongRef();
+        if (watcherPtr) {
+            for (const auto &r: m_results)
+                watcherPtr->process(r, true);
+        }
+    }
 }
 
 void CalculateBoundingVolumeJob::postFrame(QAspectEngine *aspectEngine)
@@ -338,6 +357,22 @@ void CalculateBoundingVolumeJob::postFrame(QAspectEngine *aspectEngine)
     m_results.clear();
 }
 
+void CalculateBoundingVolumeJob::addWatcher(QWeakPointer<BoundingVolumeJobProcessor> watcher)
+{
+    m_watchers.push_back(watcher);
+}
+
+void CalculateBoundingVolumeJob::removeWatcher(QWeakPointer<BoundingVolumeJobProcessor> watcher)
+{
+    if (watcher.isNull()) {
+        m_watchers.erase(std::remove_if(m_watchers.begin(), m_watchers.end(), [](const QWeakPointer<BoundingVolumeJobProcessor> &w) { return w.isNull(); }),
+                         m_watchers.end());
+    } else {
+        m_watchers.erase(std::remove(m_watchers.begin(), m_watchers.end(), watcher),
+                         m_watchers.end());
+    }
+}
+
 } // namespace Qt3DCore
 
 QT_END_NAMESPACE
diff --git a/src/core/jobs/calcboundingvolumejob_p.h b/src/core/jobs/calcboundingvolumejob_p.h
index b245308ae..715df9de4 100644
--- a/src/core/jobs/calcboundingvolumejob_p.h
+++ b/src/core/jobs/calcboundingvolumejob_p.h
@@ -94,6 +94,14 @@ struct Q_3DCORE_PRIVATE_EXPORT BoundingVolumeComputeData {
     BoundingVolumeComputeResult compute() const;
 };
 
+class Q_3DCORE_PRIVATE_EXPORT BoundingVolumeJobProcessor
+{
+public:
+    virtual ~BoundingVolumeJobProcessor() { }
+
+    virtual void process(const BoundingVolumeComputeResult &result, bool computedResult) = 0;
+};
+
 class Q_3DCORE_PRIVATE_EXPORT CalculateBoundingVolumeJob : public Qt3DCore::QAspectJob
 {
 public:
@@ -104,11 +112,15 @@ public:
     void run() override;
     void postFrame(QAspectEngine *aspectEngine) override;
 
+    void addWatcher(QWeakPointer<BoundingVolumeJobProcessor> watcher);
+    void removeWatcher(QWeakPointer<BoundingVolumeJobProcessor> watcher);
+
 private:
     Q_DECLARE_PRIVATE(CalculateBoundingVolumeJob)
     QCoreAspect *m_aspect;
     QEntity *m_root;
     std::vector<BoundingVolumeComputeResult> m_results;
+    std::vector<QWeakPointer<BoundingVolumeJobProcessor>> m_watchers;
 };
 
 typedef QSharedPointer<CalculateBoundingVolumeJob> CalculateBoundingVolumeJobPtr;
diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp
index dab3b8c8f..13587fda6 100644
--- a/src/render/frontend/qrenderaspect.cpp
+++ b/src/render/frontend/qrenderaspect.cpp
@@ -183,6 +183,7 @@
 #include <Qt3DCore/private/qentity_p.h>
 #include <Qt3DCore/private/qaspectmanager_p.h>
 #include <Qt3DCore/private/qeventfilterservice_p.h>
+#include <Qt3DCore/private/calcboundingvolumejob_p.h>
 
 #include <QThread>
 #include <QOpenGLContext>
@@ -402,6 +403,20 @@ void QRenderAspectPrivate::onEngineStartup()
         auto *coreAspect = qobject_cast<Qt3DCore::QCoreAspect *>(m_aspectManager->aspect(&Qt3DCore::QCoreAspect::staticMetaObject));
         Q_ASSERT(coreAspect);
         m_calculateBoundingVolumeJob->addDependency(coreAspect->calculateBoundingVolumeJob());
+
+        auto bvJob = qSharedPointerCast<Qt3DCore::CalculateBoundingVolumeJob>(coreAspect->calculateBoundingVolumeJob());
+        bvJob->addWatcher(m_calculateBoundingVolumeJob);
+    }
+}
+
+void QRenderAspectPrivate::onEngineAboutToShutdown()
+{
+    if (m_aspectManager) {
+        auto *coreAspect = qobject_cast<Qt3DCore::QCoreAspect *>(m_aspectManager->aspect(&Qt3DCore::QCoreAspect::staticMetaObject));
+        Q_ASSERT(coreAspect);
+
+        auto bvJob = qSharedPointerCast<Qt3DCore::CalculateBoundingVolumeJob>(coreAspect->calculateBoundingVolumeJob());
+        bvJob->removeWatcher(m_calculateBoundingVolumeJob);
     }
 }
 
diff --git a/src/render/frontend/qrenderaspect_p.h b/src/render/frontend/qrenderaspect_p.h
index b1b7bedc3..23a90e78f 100644
--- a/src/render/frontend/qrenderaspect_p.h
+++ b/src/render/frontend/qrenderaspect_p.h
@@ -108,6 +108,7 @@ public:
 
     void createNodeManagers();
     void onEngineStartup();
+    void onEngineAboutToShutdown() override;
 
     void registerBackendTypes();
     void unregisterBackendTypes();
diff --git a/src/render/jobs/calcboundingvolumejob.cpp b/src/render/jobs/calcboundingvolumejob.cpp
index 02cdce0ca..becfded8d 100644
--- a/src/render/jobs/calcboundingvolumejob.cpp
+++ b/src/render/jobs/calcboundingvolumejob.cpp
@@ -401,32 +401,6 @@ public:
         if (data.valid()) {
             // only valid if front end is a QGeometryRenderer without a view. All other cases handled by core aspect
             m_entities.push_back(data);
-        } else {
-            // TODO: this means data is copied at every frame, should track dirty
-            //       implicit or explicit objects and only copy data for those.
-            //       Ultimately would be best to avoid traversal altogether.
-            if (!data.renderer || data.renderer->primitiveType() == QGeometryRenderer::Patches
-                || !data.renderer->hasView())    // should have been handled above
-                return Continue;
-
-            // renderer has a view, we can pull the data from the front end
-            QBoundingVolume *frontEndBV = qobject_cast<QBoundingVolume *>(m_frontEndNodeManager->lookupNode(data.renderer->peerId()));
-            if (!frontEndBV)
-                return Continue;
-            auto dFrontEndBV = QGeometryRendererPrivate::get(frontEndBV);
-
-            // copy data to the entity
-            if (dFrontEndBV->m_explicitPointsValid) {
-                const auto diagonal = dFrontEndBV->m_maxPoint - dFrontEndBV->m_minPoint;
-                entity->localBoundingVolume()->setCenter(Vector3D(dFrontEndBV->m_minPoint + diagonal * .5f));
-                entity->localBoundingVolume()->setRadius(diagonal.length() * .5f);
-            } else {
-                entity->localBoundingVolume()->setCenter(Vector3D(dFrontEndBV->m_implicitCenter));
-                entity->localBoundingVolume()->setRadius(std::max(dFrontEndBV->m_implicitRadius, 0.0f));
-                entity->unsetBoundingVolumeDirty();
-                // copy the data to the geometry
-                data.geometry->updateExtent(dFrontEndBV->m_implicitMinPoint, dFrontEndBV->m_implicitMaxPoint);
-            }
         }
 
         return Continue;
@@ -436,6 +410,7 @@ public:
     std::vector<BoundingVolumeComputeData> m_entities;
 };
 
+
 } // anonymous
 
 
@@ -506,6 +481,31 @@ void CalculateBoundingVolumeJob::postFrame(QAspectEngine *aspectEngine)
     m_updatedGeometries.clear();
 }
 
+void CalculateBoundingVolumeJob::process(const Qt3DCore::BoundingVolumeComputeResult &result, bool computedResult)
+{
+    // This gets called from the thread of the CalculateBoundingVolumeJob in the core aspect.
+    // We receive the data calculated there and update our backend nodes
+
+    auto entity = m_manager->renderNodesManager()->lookupResource(result.entity->id());
+    if (!entity)
+        return;
+
+    // copy data to the entity
+    entity->localBoundingVolume()->setCenter(Vector3D(result.m_center));
+    entity->localBoundingVolume()->setRadius(std::max(result.m_radius, 0.0f));
+    entity->unsetBoundingVolumeDirty();
+    // copy the data to the geometry
+    if (computedResult) {
+        auto renderer = entity->renderComponent<GeometryRenderer>();
+        if (renderer) {
+            auto geometry = m_manager->geometryManager()->lookupResource(renderer->geometryId());
+
+            if (geometry)
+                geometry->updateExtent(result.m_min, result.m_max);
+        }
+    }
+}
+
 void CalculateBoundingVolumeJob::setRoot(Entity *node)
 {
     m_node = node;
diff --git a/src/render/jobs/calcboundingvolumejob_p.h b/src/render/jobs/calcboundingvolumejob_p.h
index 4b2352107..c3c89151e 100644
--- a/src/render/jobs/calcboundingvolumejob_p.h
+++ b/src/render/jobs/calcboundingvolumejob_p.h
@@ -54,7 +54,7 @@
 
 #include <Qt3DCore/qaspectjob.h>
 #include <Qt3DRender/private/qt3drender_global_p.h>
-
+#include <Qt3DCore/private/calcboundingvolumejob_p.h>
 #include <QSharedPointer>
 
 QT_BEGIN_NAMESPACE
@@ -70,7 +70,7 @@ class NodeManagers;
 class Entity;
 class Geometry;
 
-class Q_3DRENDERSHARED_PRIVATE_EXPORT CalculateBoundingVolumeJob : public Qt3DCore::QAspectJob
+class Q_3DRENDERSHARED_PRIVATE_EXPORT CalculateBoundingVolumeJob : public Qt3DCore::QAspectJob, public Qt3DCore::BoundingVolumeJobProcessor
 {
 public:
     explicit CalculateBoundingVolumeJob();
@@ -83,6 +83,8 @@ public:
     void postFrame(Qt3DCore::QAspectEngine *aspectEngine) override;
 
 private:
+    void process(const Qt3DCore::BoundingVolumeComputeResult &result, bool computedResult) override;
+
     NodeManagers *m_manager;
     Entity *m_node;
     Qt3DCore::QAbstractFrontEndNodeManager *m_frontEndNodeManager;
diff --git a/tests/manual/boundingvolumes/AnimatedEntity.qml b/tests/manual/boundingvolumes/AnimatedEntity.qml
index e3724968e..a432c055f 100644
--- a/tests/manual/boundingvolumes/AnimatedEntity.qml
+++ b/tests/manual/boundingvolumes/AnimatedEntity.qml
@@ -49,9 +49,9 @@
 ****************************************************************************/
 
 import Qt3D.Core 2.16
-import Qt3D.Render 2.15
+import Qt3D.Render 2.16
 import Qt3D.Input 2.0
-import Qt3D.Extras 2.15
+import Qt3D.Extras 2.16
 
 import QtQuick 2.0 as QQ2
 
@@ -100,7 +100,7 @@ Entity {
             SphereMesh { radius: 4 },
             PhongMaterial { diffuse: "red" },
             Transform { translation: Qt.vector3d(mainAnimation.x, -9, 0) },
-            BoundingVolume { view: SphereMesh { radius: 2 } }
+            BoundingVolume { view: SphereGeometryView { radius: 2 } }
         ]
     }
 
@@ -110,8 +110,8 @@ Entity {
         components: [
             SphereMesh {
                 radius: 4
-                view.minPoint: Qt.vector3d(-.5, -.5, -.5)
-                view.maxPoint: Qt.vector3d(.5, .5, .5)
+                minPoint: Qt.vector3d(-.5, -.5, -.5)
+                maxPoint: Qt.vector3d(.5, .5, .5)
             },
             PhongMaterial { diffuse: "green" },
             Transform { translation: Qt.vector3d(mainAnimation.x, 0 , 0) }