Refactor job to prepare for line and point picking

This moves much of the job utils into private code, simplifies
the various class types, clarifies names to avoid clashes and
simplifies some APIs.

Introduces changes in headers requiring extra includes in
various files.

Task-number: QTBUG-58071
Change-Id: I20a8251abcef140d9d71b372a66282e242a21ad2
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

9 files changed, 125 insertions, 110 deletions

diff --git a/src/quick3d/quick3dscene2d/items/scene2d.cpp b/src/quick3d/quick3dscene2d/items/scene2d.cpp
index b0c58c6c5..7bcc23077 100644
--- a/src/quick3d/quick3dscene2d/items/scene2d.cpp
+++ b/src/quick3d/quick3dscene2d/items/scene2d.cpp
@@ -58,6 +58,8 @@
 #include <private/qpickevent_p.h>
 #include <private/entity_p.h>
 #include <private/platformsurfacefilter_p.h>
+#include <private/trianglesvisitor_p.h>
+
 
 QT_BEGIN_NAMESPACE
 
diff --git a/src/render/backend/renderviewbuilder.cpp b/src/render/backend/renderviewbuilder.cpp
index f47c6f419..06cf1d22e 100644
--- a/src/render/backend/renderviewbuilder.cpp
+++ b/src/render/backend/renderviewbuilder.cpp
@@ -39,6 +39,8 @@
 
 #include "renderviewbuilder_p.h"
 
+#include <QThread>
+
 QT_BEGIN_NAMESPACE
 
 namespace Qt3DRender {
diff --git a/src/render/jobs/loadscenejob.cpp b/src/render/jobs/loadscenejob.cpp
index 3d6326e93..5be733a4d 100644
--- a/src/render/jobs/loadscenejob.cpp
+++ b/src/render/jobs/loadscenejob.cpp
@@ -47,6 +47,9 @@
 #include <Qt3DRender/private/qurlhelper_p.h>
 #include <Qt3DRender/qsceneloader.h>
 
+#include <QFileInfo>
+#include <QMimeDatabase>
+
 QT_BEGIN_NAMESPACE
 
 namespace Qt3DRender {
diff --git a/src/render/jobs/pickboundingvolumejob.cpp b/src/render/jobs/pickboundingvolumejob.cpp
index f9c7a390c..a1c8dd864 100644
--- a/src/render/jobs/pickboundingvolumejob.cpp
+++ b/src/render/jobs/pickboundingvolumejob.cpp
@@ -39,17 +39,17 @@
 
 #include "pickboundingvolumejob_p.h"
 #include "qpicktriangleevent.h"
+#include <Qt3DRender/qgeometryrenderer.h>
 #include <Qt3DRender/private/renderer_p.h>
 #include <Qt3DRender/private/nodemanagers_p.h>
 #include <Qt3DRender/private/entity_p.h>
 #include <Qt3DRender/private/objectpicker_p.h>
 #include <Qt3DRender/private/managers_p.h>
 #include <Qt3DRender/private/geometryrenderer_p.h>
-
 #include <Qt3DRender/private/rendersettings_p.h>
-#include <Qt3DRender/qgeometryrenderer.h>
 #include <Qt3DRender/private/job_common_p.h>
 #include <Qt3DRender/private/qpickevent_p.h>
+#include <Qt3DRender/private/pickboundingvolumeutils_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -60,8 +60,6 @@ namespace Render {
 
 namespace {
 
-typedef PickingUtils::AbstractCollisionGathererFunctor::result_type HitList;
-
 void setEventButtonAndModifiers(const QMouseEvent &event, QPickEvent::Buttons &eventButton, int &eventButtons, int &eventModifiers)
 {
     switch (event.button()) {
@@ -223,9 +221,6 @@ bool PickBoundingVolumeJob::runHelper()
     // If we have move or hover move events that someone cares about, we try to avoid expensive computations
     // by compressing them into a single one
 
-    // Store the reducer function which varies depending on the picking settings set on the renderer
-    using ReducerFunction = PickingUtils::CollisionVisitor::HitList (*)(PickingUtils::CollisionVisitor::HitList &results, const PickingUtils::CollisionVisitor::HitList &intermediate);
-
     const bool trianglePickingRequested = (m_renderSettings->pickMethod() == QPickingSettings::TrianglePicking);
     const bool allHitsRequested = (m_renderSettings->pickResultMode() == QPickingSettings::AllPicks);
     const bool frontFaceRequested =
@@ -233,9 +228,6 @@ bool PickBoundingVolumeJob::runHelper()
     const bool backFaceRequested =
             m_renderSettings->faceOrientationPickingMode() != QPickingSettings::FrontFace;
 
-    // Select the best reduction function based on the settings
-    const ReducerFunction reducerOp = allHitsRequested ? PickingUtils::reduceToAllHits : PickingUtils::reduceToFirstHit;
-
     // For each mouse event
     for (const QMouseEvent &event : mouseEvents) {
         m_hoveredPickersToClear = m_hoveredPickers;
@@ -248,7 +240,7 @@ bool PickBoundingVolumeJob::runHelper()
 
         // For each triplet of Viewport / Camera and Area
         for (const PickingUtils::ViewportCameraAreaTriplet &vca : vcaTriplets) {
-            HitList sphereHits;
+            PickingUtils::HitList sphereHits;
             QRay3D ray = rayForViewportAndCamera(vca.area, event.pos(), vca.viewport, vca.cameraId);
 
             PickingUtils::HierarchicalEntityPicker entityPicker(ray);
@@ -259,8 +251,7 @@ bool PickBoundingVolumeJob::runHelper()
                     gathererFunctor.m_backFaceRequested = backFaceRequested;
                     gathererFunctor.m_manager = m_manager;
                     gathererFunctor.m_ray = ray;
-                    sphereHits = QtConcurrent::blockingMappedReduced<HitList>(entityPicker.entities(),
-                                                                              gathererFunctor, reducerOp);
+                    sphereHits = gathererFunctor.computeHits(entityPicker.entities(), allHitsRequested);
                 } else {
                     sphereHits = entityPicker.hits();
                     PickingUtils::AbstractCollisionGathererFunctor::sortHits(sphereHits);
@@ -300,7 +291,7 @@ void PickBoundingVolumeJob::run()
 }
 
 void PickBoundingVolumeJob::dispatchPickEvents(const QMouseEvent &event,
-                                               const PickingUtils::CollisionVisitor::HitList &sphereHits,
+                                               const PickingUtils::HitList &sphereHits,
                                                QPickEvent::Buttons eventButton,
                                                int eventButtons,
                                                int eventModifiers,
diff --git a/src/render/jobs/pickboundingvolumejob_p.h b/src/render/jobs/pickboundingvolumejob_p.h
index 5239c5c6c..f13e4e48b 100644
--- a/src/render/jobs/pickboundingvolumejob_p.h
+++ b/src/render/jobs/pickboundingvolumejob_p.h
@@ -56,7 +56,6 @@
 #include <Qt3DRender/private/handle_types_p.h>
 #include <Qt3DRender/private/qboundingvolumeprovider_p.h>
 #include <Qt3DRender/private/qcollisionqueryresult_p.h>
-#include <Qt3DRender/private/pickboundingvolumeutils_p.h>
 #include <Qt3DRender/qpickevent.h>
 #include <QMouseEvent>
 #include <QKeyEvent>
@@ -76,6 +75,10 @@ class Renderer;
 class NodeManagers;
 class RenderSettings;
 
+namespace PickingUtils {
+typedef QVector<RayCasting::QCollisionQueryResult::Hit> HitList;
+}
+
 class Q_AUTOTEST_EXPORT PickBoundingVolumeJob : public Qt3DCore::QAspectJob
 {
 public:
@@ -102,7 +105,8 @@ public:
 
 protected:
     void run() Q_DECL_FINAL;
-    void dispatchPickEvents(const QMouseEvent &event, const PickingUtils::CollisionVisitor::HitList &sphereHits,
+    void dispatchPickEvents(const QMouseEvent &event,
+                            const PickingUtils::HitList &sphereHits,
                             QPickEvent::Buttons eventButton,
                             int eventButtons,
                             int eventModifiers,
diff --git a/src/render/jobs/pickboundingvolumeutils.cpp b/src/render/jobs/pickboundingvolumeutils.cpp
index f08398a5a..a8367dc20 100644
--- a/src/render/jobs/pickboundingvolumeutils.cpp
+++ b/src/render/jobs/pickboundingvolumeutils.cpp
@@ -110,7 +110,8 @@ QVector<ViewportCameraAreaTriplet> ViewportCameraAreaGatherer::gather(FrameGraph
     return vcaTriplets;
 }
 
-bool PickingUtils::ViewportCameraAreaGatherer::isUnique(const QVector<ViewportCameraAreaTriplet> &vcaTriplets, const ViewportCameraAreaTriplet &vca) const
+bool ViewportCameraAreaGatherer::isUnique(const QVector<ViewportCameraAreaTriplet> &vcaTriplets,
+                                          const ViewportCameraAreaTriplet &vca) const
 {
     for (const ViewportCameraAreaTriplet &triplet : vcaTriplets) {
         if (vca.cameraId == triplet.cameraId && vca.viewport == triplet.viewport && vca.area == triplet.area)
@@ -147,7 +148,35 @@ QVector<Entity *> EntityGatherer::entities() const
     return m_entities;
 }
 
-void CollisionVisitor::visit(uint andx, const QVector3D &a, uint bndx, const QVector3D &b, uint cndx, const QVector3D &c)
+
+class TriangleCollisionVisitor : public TrianglesVisitor
+{
+public:
+    HitList hits;
+
+    TriangleCollisionVisitor(NodeManagers* manager, const Entity *root, const RayCasting::QRay3D& ray,
+                     bool frontFaceRequested, bool backFaceRequested)
+        : TrianglesVisitor(manager), m_root(root), m_ray(ray), m_triangleIndex(0)
+        , m_frontFaceRequested(frontFaceRequested), m_backFaceRequested(backFaceRequested)
+    {
+    }
+
+private:
+    const Entity *m_root;
+    RayCasting::QRay3D m_ray;
+    uint m_triangleIndex;
+    bool m_frontFaceRequested;
+    bool m_backFaceRequested;
+
+    void visit(uint andx, const QVector3D &a,
+               uint bndx, const QVector3D &b,
+               uint cndx, const QVector3D &c) Q_DECL_OVERRIDE;
+    bool intersectsSegmentTriangle(uint andx, const QVector3D &a,
+                                   uint bndx, const QVector3D &b,
+                                   uint cndx, const QVector3D &c);
+};
+
+void TriangleCollisionVisitor::visit(uint andx, const QVector3D &a, uint bndx, const QVector3D &b, uint cndx, const QVector3D &c)
 {
     const QMatrix4x4 &mat = *m_root->worldTransform();
     const QVector3D tA = mat * a;
@@ -163,7 +192,7 @@ void CollisionVisitor::visit(uint andx, const QVector3D &a, uint bndx, const QVe
     m_triangleIndex++;
 }
 
-bool CollisionVisitor::intersectsSegmentTriangle(uint andx, const QVector3D &a, uint bndx, const QVector3D &b, uint cndx, const QVector3D &c)
+bool TriangleCollisionVisitor::intersectsSegmentTriangle(uint andx, const QVector3D &a, uint bndx, const QVector3D &b, uint cndx, const QVector3D &c)
 {
     float t = 0.0f;
     QVector3D uvw;
@@ -183,6 +212,32 @@ bool CollisionVisitor::intersectsSegmentTriangle(uint andx, const QVector3D &a,
     return intersected;
 }
 
+HitList reduceToFirstHit(HitList &result, const HitList &intermediate)
+{
+    if (!intermediate.empty()) {
+        if (result.empty())
+            result.push_back(intermediate.front());
+        float closest = result.front().m_distance;
+        for (const auto &v : intermediate) {
+            if (v.m_distance < closest) {
+                result.push_front(v);
+                closest = v.m_distance;
+            }
+        }
+
+        while (result.size() > 1)
+            result.pop_back();
+    }
+    return result;
+}
+
+HitList reduceToAllHits(HitList &results, const HitList &intermediate)
+{
+    if (!intermediate.empty())
+        results << intermediate;
+    return results;
+}
+
 AbstractCollisionGathererFunctor::AbstractCollisionGathererFunctor()
     : m_manager(nullptr)
 { }
@@ -190,7 +245,7 @@ AbstractCollisionGathererFunctor::AbstractCollisionGathererFunctor()
 AbstractCollisionGathererFunctor::~AbstractCollisionGathererFunctor()
 { }
 
-AbstractCollisionGathererFunctor::result_type AbstractCollisionGathererFunctor::operator ()(const Entity *entity) const
+HitList AbstractCollisionGathererFunctor::operator ()(const Entity *entity) const
 {
     HObjectPicker objectPickerHandle = entity->componentHandle<ObjectPicker, 16>();
 
@@ -205,44 +260,62 @@ AbstractCollisionGathererFunctor::result_type AbstractCollisionGathererFunctor::
 
     ObjectPicker *objectPicker = m_manager->objectPickerManager()->data(objectPickerHandle);
     if (objectPicker == nullptr || !objectPicker->isEnabled())
-        return result_type();   // don't bother picking entities that don't
-                                // have an object picker, or if it's disabled
+        return {};   // don't bother picking entities that don't
+                     // have an object picker, or if it's disabled
 
-    RayCasting::QRayCastingService rayCasting;
+    return pick(entity);
+}
 
-    return pick(&rayCasting, entity);
+bool AbstractCollisionGathererFunctor::rayHitsEntity(const Entity *entity) const
+{
+    QRayCastingService rayCasting;
+    const QCollisionQueryResult::Hit queryResult = rayCasting.query(m_ray, entity->worldBoundingVolume());
+    return queryResult.m_distance >= 0.f;
 }
 
-void AbstractCollisionGathererFunctor::sortHits(CollisionVisitor::HitList &results)
+void AbstractCollisionGathererFunctor::sortHits(HitList &results)
 {
-    auto compareHitsDistance = [](const CollisionVisitor::HitList::value_type &a,
-                                  const CollisionVisitor::HitList::value_type &b) {
+    auto compareHitsDistance = [](const HitList::value_type &a,
+                                  const HitList::value_type &b) {
         return a.m_distance < b.m_distance;
     };
     std::sort(results.begin(), results.end(), compareHitsDistance);
 }
 
-AbstractCollisionGathererFunctor::result_type EntityCollisionGathererFunctor::pick(RayCasting::QAbstractCollisionQueryService *rayCasting, const Entity *entity) const
+HitList EntityCollisionGathererFunctor::computeHits(const QVector<Entity *> &entities, bool allHitsRequested)
 {
-    result_type result;
+    const auto reducerOp = allHitsRequested ? PickingUtils::reduceToAllHits : PickingUtils::reduceToFirstHit;
+    return QtConcurrent::blockingMappedReduced<HitList>(entities, *this, reducerOp);
+}
 
-    const QCollisionQueryResult::Hit queryResult = rayCasting->query(m_ray, entity->worldBoundingVolume());
+HitList EntityCollisionGathererFunctor::pick(const Entity *entity) const
+{
+    HitList result;
+
+    QRayCastingService rayCasting;
+    const QCollisionQueryResult::Hit queryResult = rayCasting.query(m_ray, entity->worldBoundingVolume());
     if (queryResult.m_distance >= 0.f)
         result.push_back(queryResult);
 
     return result;
 }
 
-AbstractCollisionGathererFunctor::result_type TriangleCollisionGathererFunctor::pick(RayCasting::QAbstractCollisionQueryService *rayCasting, const Entity *entity) const
+HitList TriangleCollisionGathererFunctor::computeHits(const QVector<Entity *> &entities, bool allHitsRequested)
+{
+    const auto reducerOp = allHitsRequested ? PickingUtils::reduceToAllHits : PickingUtils::reduceToFirstHit;
+    return QtConcurrent::blockingMappedReduced<HitList>(entities, *this, reducerOp);
+}
+
+HitList TriangleCollisionGathererFunctor::pick(const Entity *entity) const
 {
-    result_type result;
+    HitList result;
 
     GeometryRenderer *gRenderer = entity->renderComponent<GeometryRenderer>();
     if (!gRenderer)
         return result;
 
-    if (rayHitsEntity(rayCasting, entity)) {
-        CollisionVisitor visitor(m_manager, entity, m_ray, m_frontFaceRequested, m_backFaceRequested);
+    if (rayHitsEntity(entity)) {
+        TriangleCollisionVisitor visitor(m_manager, entity, m_ray, m_frontFaceRequested, m_backFaceRequested);
         visitor.apply(gRenderer, entity->peerId());
         result = visitor.hits;
 
@@ -252,38 +325,6 @@ AbstractCollisionGathererFunctor::result_type TriangleCollisionGathererFunctor::
     return result;
 }
 
-bool TriangleCollisionGathererFunctor::rayHitsEntity(RayCasting::QAbstractCollisionQueryService *rayCasting, const Entity *entity) const
-{
-    const QCollisionQueryResult::Hit queryResult = rayCasting->query(m_ray, entity->worldBoundingVolume());
-    return queryResult.m_distance >= 0.f;
-}
-
-CollisionVisitor::HitList reduceToFirstHit(CollisionVisitor::HitList &result, const CollisionVisitor::HitList &intermediate)
-{
-    if (!intermediate.empty()) {
-        if (result.empty())
-            result.push_back(intermediate.front());
-        float closest = result.front().m_distance;
-        for (const auto &v : intermediate) {
-            if (v.m_distance < closest) {
-                result.push_front(v);
-                closest = v.m_distance;
-            }
-        }
-
-        while (result.size() > 1)
-            result.pop_back();
-    }
-    return result;
-}
-
-CollisionVisitor::HitList reduceToAllHits(CollisionVisitor::HitList &results, const CollisionVisitor::HitList &intermediate)
-{
-    if (!intermediate.empty())
-        results << intermediate;
-    return results;
-}
-
 HierarchicalEntityPicker::HierarchicalEntityPicker(const QRay3D &ray)
     : m_ray(ray)
 {
diff --git a/src/render/jobs/pickboundingvolumeutils_p.h b/src/render/jobs/pickboundingvolumeutils_p.h
index 08615c094..eb272ce41 100644
--- a/src/render/jobs/pickboundingvolumeutils_p.h
+++ b/src/render/jobs/pickboundingvolumeutils_p.h
@@ -108,33 +108,7 @@ private:
     mutable bool m_needsRefresh;
 };
 
-class Q_AUTOTEST_EXPORT CollisionVisitor : public TrianglesVisitor
-{
-public:
-    typedef QVector<RayCasting::QCollisionQueryResult::Hit> HitList;
-    HitList hits;
-
-    CollisionVisitor(NodeManagers* manager, const Entity *root, const RayCasting::QRay3D& ray,
-                     bool frontFaceRequested, bool backFaceRequested)
-        : TrianglesVisitor(manager), m_root(root), m_ray(ray), m_triangleIndex(0)
-        , m_frontFaceRequested(frontFaceRequested), m_backFaceRequested(backFaceRequested)
-    {
-    }
-
-private:
-    const Entity *m_root;
-    RayCasting::QRay3D m_ray;
-    uint m_triangleIndex;
-    bool m_frontFaceRequested;
-    bool m_backFaceRequested;
-
-    void visit(uint andx, const QVector3D &a,
-               uint bndx, const QVector3D &b,
-               uint cndx, const QVector3D &c) Q_DECL_OVERRIDE;
-    bool intersectsSegmentTriangle(uint andx, const QVector3D &a,
-                                   uint bndx, const QVector3D &b,
-                                   uint cndx, const QVector3D &c);
-};
+typedef QVector<RayCasting::QCollisionQueryResult::Hit> HitList;
 
 class Q_AUTOTEST_EXPORT HierarchicalEntityPicker
 {
@@ -142,12 +116,12 @@ public:
     explicit HierarchicalEntityPicker(const RayCasting::QRay3D &ray);
 
     bool collectHits(Entity *root);
-    inline CollisionVisitor::HitList hits() const { return m_hits; }
+    inline HitList hits() const { return m_hits; }
     inline QVector<Entity *> entities() const { return m_entities; }
 
 private:
     RayCasting::QRay3D m_ray;
-    CollisionVisitor::HitList m_hits;
+    HitList m_hits;
     QVector<Entity *> m_entities;
 };
 
@@ -159,17 +133,21 @@ struct Q_AUTOTEST_EXPORT AbstractCollisionGathererFunctor
     NodeManagers *m_manager;
     RayCasting::QRay3D m_ray;
 
+    virtual HitList computeHits(const QVector<Entity *> &entities, bool allHitsRequested) = 0;
+
     // This define is required to work with QtConcurrent
-    typedef CollisionVisitor::HitList result_type;
-    result_type operator ()(const Entity *entity) const;
-    virtual result_type pick(RayCasting::QAbstractCollisionQueryService *rayCasting, const Entity *entity) const = 0;
+    typedef HitList result_type;
+    HitList operator ()(const Entity *entity) const;
+    virtual HitList pick(const Entity *entity) const = 0;
 
-    static void sortHits(CollisionVisitor::HitList &results);
+    bool rayHitsEntity(const Entity *entity) const;
+    static void sortHits(HitList &results);
 };
 
 struct Q_AUTOTEST_EXPORT EntityCollisionGathererFunctor : public AbstractCollisionGathererFunctor
 {
-    result_type pick(RayCasting::QAbstractCollisionQueryService *rayCasting, const Entity *entity) const Q_DECL_OVERRIDE;
+    HitList computeHits(const QVector<Entity *> &entities, bool allHitsRequested) Q_DECL_OVERRIDE;
+    HitList pick(const Entity *entity) const Q_DECL_OVERRIDE;
 };
 
 struct Q_AUTOTEST_EXPORT TriangleCollisionGathererFunctor : public AbstractCollisionGathererFunctor
@@ -177,18 +155,10 @@ struct Q_AUTOTEST_EXPORT TriangleCollisionGathererFunctor : public AbstractColli
     bool m_frontFaceRequested;
     bool m_backFaceRequested;
 
-    result_type pick(RayCasting::QAbstractCollisionQueryService *rayCasting, const Entity *entity) const Q_DECL_OVERRIDE;
-
-    bool rayHitsEntity(RayCasting::QAbstractCollisionQueryService *rayCasting, const Entity *entity) const;
+    HitList computeHits(const QVector<Entity *> &entities, bool allHitsRequested) Q_DECL_OVERRIDE;
+    HitList pick(const Entity *entity) const Q_DECL_OVERRIDE;
 };
 
-Q_AUTOTEST_EXPORT QVector<Entity *> gatherEntities(Entity *entity, QVector<Entity *> entities);
-
-Q_AUTOTEST_EXPORT CollisionVisitor::HitList reduceToFirstHit(CollisionVisitor::HitList &result, const CollisionVisitor::HitList &intermediate);
-
-// Unordered
-Q_AUTOTEST_EXPORT CollisionVisitor::HitList reduceToAllHits(CollisionVisitor::HitList &results, const CollisionVisitor::HitList &intermediate);
-
 } // PickingUtils
 
 } // Render
diff --git a/src/render/texture/qtexture.cpp b/src/render/texture/qtexture.cpp
index 931d66c64..3ef3200a6 100644
--- a/src/render/texture/qtexture.cpp
+++ b/src/render/texture/qtexture.cpp
@@ -46,6 +46,7 @@
 #include <QFileInfo>
 #include <QMimeDatabase>
 #include <QMimeType>
+#include <QtCore/QBuffer>
 #include <qendian.h>
 #include <Qt3DCore/private/qscene_p.h>
 #include <Qt3DCore/qaspectengine.h>
diff --git a/tests/auto/render/pickboundingvolumejob/tst_pickboundingvolumejob.cpp b/tests/auto/render/pickboundingvolumejob/tst_pickboundingvolumejob.cpp
index 1d391101e..e2cc190df 100644
--- a/tests/auto/render/pickboundingvolumejob/tst_pickboundingvolumejob.cpp
+++ b/tests/auto/render/pickboundingvolumejob/tst_pickboundingvolumejob.cpp
@@ -46,6 +46,7 @@
 #include <Qt3DRender/qrenderaspect.h>
 #include <Qt3DRender/private/qrenderaspect_p.h>
 #include <Qt3DRender/private/pickboundingvolumejob_p.h>
+#include <Qt3DRender/private/pickboundingvolumeutils_p.h>
 #include <Qt3DRender/private/updatemeshtrianglelistjob_p.h>
 #include <Qt3DRender/private/updateworldboundingvolumejob_p.h>
 #include <Qt3DRender/private/updateworldtransformjob_p.h>