CalcBoundingVolumeJob: don't rely on QChangeArbiter for extent changes

Relying on sync messages is deprecated, CalcBoundingVolumeJob appears to have
slipped through when we updated all the other classes. CalcBoundingVolumeJob
has therefore been updated to use the postFrame and directly update the
frontend Geometry nodes.

Note: Qt 3D on Qt6 is correct.
Change-Id: I2257bc157474efac8190d763ea7dc331a1d69a86
Reviewed-by: Mike Krus <mike.krus@kdab.com>

7 files changed, 53 insertions, 47 deletions

diff --git a/src/render/geometry/geometry.cpp b/src/render/geometry/geometry.cpp
index dd099fc44..5e653581a 100644
--- a/src/render/geometry/geometry.cpp
+++ b/src/render/geometry/geometry.cpp
@@ -55,8 +55,6 @@ namespace Render {
 Geometry::Geometry()
     : BackendNode(ReadWrite)
     , m_geometryDirty(false)
-    , m_shouldNotifyMinExtentChanged(false)
-    , m_shouldNotifyMaxExtentChanged(false)
 {
 }
 
@@ -72,8 +70,6 @@ void Geometry::cleanup()
     m_boundingPositionAttribute = Qt3DCore::QNodeId();
     m_min = QVector3D();
     m_max = QVector3D();
-    m_shouldNotifyMinExtentChanged = false;
-    m_shouldNotifyMaxExtentChanged = false;
 }
 
 void Geometry::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
@@ -113,27 +109,10 @@ void Geometry::updateExtent(const QVector3D &min, const QVector3D &max)
     // Send notification to frontend
     if (m_min != min) {
         m_min = min;
-        m_shouldNotifyMinExtentChanged = true;
     }
 
     if (m_max != max) {
         m_max = max;
-        m_shouldNotifyMaxExtentChanged = true;
-    }
-}
-
-// Called from calcboundingvolumejob after all bounding volumes have been
-// updated (in an aspect thread)
-void Geometry::notifyExtentChanged()
-{
-    if (m_shouldNotifyMinExtentChanged || m_shouldNotifyMaxExtentChanged) {
-        auto change = Qt3DCore::QPropertyUpdatedChangePtr::create(peerId());
-        change->setDeliveryFlags(Qt3DCore::QSceneChange::Nodes);
-        change->setPropertyName("extent");
-        change->setValue(QVariant::fromValue(QPair<QVector3D, QVector3D>(m_min, m_max)));
-        notifyObservers(change);
-        m_shouldNotifyMinExtentChanged = false;
-        m_shouldNotifyMaxExtentChanged = false;
     }
 }
 
diff --git a/src/render/geometry/geometry_p.h b/src/render/geometry/geometry_p.h
index f07db902e..d5353e92b 100644
--- a/src/render/geometry/geometry_p.h
+++ b/src/render/geometry/geometry_p.h
@@ -79,7 +79,6 @@ public:
     inline QVector3D max() const { return m_max; }
 
     void updateExtent(const QVector3D &min, const QVector3D &max);
-    void notifyExtentChanged();
 
 private:
     QVector<Qt3DCore::QNodeId> m_attributes;
@@ -87,8 +86,6 @@ private:
     Qt3DCore::QNodeId m_boundingPositionAttribute;
     QVector3D m_min;
     QVector3D m_max;
-    bool m_shouldNotifyMinExtentChanged;
-    bool m_shouldNotifyMaxExtentChanged;
 };
 
 } // namespace Render
diff --git a/src/render/geometry/qgeometry.cpp b/src/render/geometry/qgeometry.cpp
index 383e71b8b..5189cb753 100644
--- a/src/render/geometry/qgeometry.cpp
+++ b/src/render/geometry/qgeometry.cpp
@@ -64,6 +64,24 @@ QGeometryPrivate::~QGeometryPrivate()
 {
 }
 
+void QGeometryPrivate::setExtent(const QVector3D &minExtent, const QVector3D &maxExtent)
+{
+    Q_Q(QGeometry);
+    if (m_minExtent != minExtent) {
+        m_minExtent = minExtent;
+        const auto wasBlocked = q->blockNotifications(true);
+        emit q->minExtentChanged(minExtent);
+        q->blockNotifications(wasBlocked);
+    }
+
+    if (m_maxExtent != maxExtent) {
+        m_maxExtent = maxExtent;
+        const auto wasBlocked = q->blockNotifications(true);
+        emit q->maxExtentChanged(maxExtent);
+        q->blockNotifications(wasBlocked);
+    }
+}
+
 /*!
     \qmltype Geometry
     \instantiates Qt3DRender::QGeometry
@@ -151,26 +169,9 @@ QGeometry::QGeometry(QGeometryPrivate &dd, QNode *parent)
 {
 }
 
-void QGeometry::sceneChangeEvent(const QSceneChangePtr &change)
+// TODO Unused remove in Qt6
+void QGeometry::sceneChangeEvent(const QSceneChangePtr &)
 {
-    Q_D(QGeometry);
-    QPropertyUpdatedChangePtr e = qSharedPointerCast<QPropertyUpdatedChange>(change);
-    if (e->type() == PropertyUpdated) {
-        const bool blocked = blockNotifications(true);
-        if (e->propertyName() == QByteArrayLiteral("extent")) {
-            const QPair<QVector3D, QVector3D> extent = e->value().value<QPair<QVector3D, QVector3D>>();
-
-            if (extent.first != d->m_minExtent) {
-                d->m_minExtent = extent.first;
-                emit minExtentChanged(extent.first);
-            }
-            if (extent.second != d->m_maxExtent) {
-                d->m_maxExtent = extent.second;
-                emit maxExtentChanged(d->m_maxExtent);
-            }
-        }
-        blockNotifications(blocked);
-    }
 }
 
 /*!
diff --git a/src/render/geometry/qgeometry.h b/src/render/geometry/qgeometry.h
index b49de39fc..31d5759b7 100644
--- a/src/render/geometry/qgeometry.h
+++ b/src/render/geometry/qgeometry.h
@@ -77,6 +77,7 @@ Q_SIGNALS:
     Q_REVISION(13) void maxExtentChanged(const QVector3D &maxExtent);
 protected:
     explicit QGeometry(QGeometryPrivate &dd, Qt3DCore::QNode *parent = nullptr);
+    // TODO Unused remove in Qt6
     void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &change) override;
 
 private:
diff --git a/src/render/geometry/qgeometry_p.h b/src/render/geometry/qgeometry_p.h
index f5b6ad7d8..a6af8c205 100644
--- a/src/render/geometry/qgeometry_p.h
+++ b/src/render/geometry/qgeometry_p.h
@@ -66,6 +66,7 @@ public:
     QGeometryPrivate();
     ~QGeometryPrivate();
 
+    void setExtent(const QVector3D &minExtent, const QVector3D &maxExtent);
 
     QVector<QAttribute *> m_attributes;
     QAttribute *m_boundingVolumePositionAttribute;
diff --git a/src/render/jobs/calcboundingvolumejob.cpp b/src/render/jobs/calcboundingvolumejob.cpp
index 4fed1e9f0..ab14cce87 100644
--- a/src/render/jobs/calcboundingvolumejob.cpp
+++ b/src/render/jobs/calcboundingvolumejob.cpp
@@ -53,6 +53,7 @@
 #include <Qt3DRender/private/buffervisitor_p.h>
 #include <Qt3DRender/private/entityvisitor_p.h>
 #include <Qt3DCore/private/qaspectmanager_p.h>
+#include <Qt3DRender/private/qgeometry_p.h>
 
 #include <QtCore/qmath.h>
 #if QT_CONFIG(concurrent)
@@ -399,8 +400,33 @@ public:
 
 } // anonymous
 
+class CalculateBoundingVolumeJobPrivate : public Qt3DCore::QAspectJobPrivate
+{
+public:
+    CalculateBoundingVolumeJobPrivate(CalculateBoundingVolumeJob *q) : q_ptr(q) {}
+    ~CalculateBoundingVolumeJobPrivate() override = default;
+
+    void postFrame(Qt3DCore::QAspectManager *aspectManager) override
+    {
+        for (Geometry *backend : m_updatedGeometries) {
+            Qt3DRender::QGeometry *node = qobject_cast<Qt3DRender::QGeometry *>(aspectManager->lookupNode(backend->peerId()));
+            if (!node)
+                continue;
+            Qt3DRender::QGeometryPrivate *dNode = static_cast<Qt3DRender::QGeometryPrivate *>(Qt3DCore::QNodePrivate::get(node));
+            dNode->setExtent(backend->min(), backend->max());
+        }
+
+        m_updatedGeometries.clear();
+    }
+
+    QVector<Geometry *> m_updatedGeometries;
+    CalculateBoundingVolumeJob *q_ptr;
+    Q_DECLARE_PUBLIC(CalculateBoundingVolumeJob)
+};
+
 CalculateBoundingVolumeJob::CalculateBoundingVolumeJob()
-    : m_manager(nullptr)
+    : Qt3DCore::QAspectJob(*new CalculateBoundingVolumeJobPrivate(this))
+    , m_manager(nullptr)
     , m_node(nullptr)
 {
     SET_JOB_RUN_STAT_TYPE(this, JobTypes::CalcBoundingVolume, 0)
@@ -429,9 +455,8 @@ void CalculateBoundingVolumeJob::run()
             updatedGeometries += calculateLocalBoundingVolume(m_manager, data);
     }
 
-    // Send extent updates to frontend
-    for (Geometry *geometry : updatedGeometries)
-        geometry->notifyExtentChanged();
+    Q_D(CalculateBoundingVolumeJob);
+    d->m_updatedGeometries = std::move(updatedGeometries);
 }
 
 void CalculateBoundingVolumeJob::setRoot(Entity *node)
diff --git a/src/render/jobs/calcboundingvolumejob_p.h b/src/render/jobs/calcboundingvolumejob_p.h
index 604e790f6..6227c3c21 100644
--- a/src/render/jobs/calcboundingvolumejob_p.h
+++ b/src/render/jobs/calcboundingvolumejob_p.h
@@ -64,6 +64,7 @@ namespace Render {
 
 class NodeManagers;
 class Entity;
+class CalculateBoundingVolumeJobPrivate;
 
 class Q_3DRENDERSHARED_PRIVATE_EXPORT CalculateBoundingVolumeJob : public Qt3DCore::QAspectJob
 {
@@ -75,6 +76,7 @@ public:
     void run() override;
 
 private:
+    Q_DECLARE_PRIVATE(CalculateBoundingVolumeJob);
     NodeManagers *m_manager;
     Entity *m_node;
 };