Picking: reuse LayerFilterJob to perform layer filtering

This fixes picking for cases where multiple LayerFilters using different
filter modes are present in the FrameGraph. This also reduces code
duplication.

Change-Id: I19d0c2c5777930820ab950cbf2bfe08ef7d2484f
Reviewed-by: Mike Krus <mike.krus@kdab.com>
(cherry picked from commit 406ea4134444a05dd89f215b3144181c0b0ce924)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

7 files changed, 89 insertions, 98 deletions

diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp
index e742b1555..2bc31506f 100644
--- a/src/render/frontend/qrenderaspect.cpp
+++ b/src/render/frontend/qrenderaspect.cpp
@@ -318,7 +318,9 @@ QRenderAspectPrivate::QRenderAspectPrivate(QRenderAspect::SubmissionType submiss
     m_expandBoundingVolumeJob->addDependency(m_updateWorldBoundingVolumeJob);
     m_updateLevelOfDetailJob->addDependency(m_expandBoundingVolumeJob);
     m_pickBoundingVolumeJob->addDependency(m_expandBoundingVolumeJob);
+    m_pickBoundingVolumeJob->addDependency(m_updateEntityLayersJob);
     m_rayCastingJob->addDependency(m_expandBoundingVolumeJob);
+    m_rayCastingJob->addDependency(m_updateEntityLayersJob);
 }
 
 /*! \internal */
diff --git a/src/render/jobs/filterlayerentityjob.cpp b/src/render/jobs/filterlayerentityjob.cpp
index ddea1ebf7..c7db55c53 100644
--- a/src/render/jobs/filterlayerentityjob.cpp
+++ b/src/render/jobs/filterlayerentityjob.cpp
@@ -75,6 +75,33 @@ void FilterLayerEntityJob::run()
     std::sort(m_filteredEntities.begin(), m_filteredEntities.end());
 }
 
+void FilterLayerEntityJob::filterEntityAgainstLayers(Entity *entity,
+                                                     const Qt3DCore::QNodeIdVector &layerIds,
+                                                     const QLayerFilter::FilterMode filterMode)
+{
+    // Perform filtering
+    switch (filterMode) {
+    case QLayerFilter::AcceptAnyMatchingLayers: {
+        filterAcceptAnyMatchingLayers(entity, layerIds);
+        break;
+    }
+    case QLayerFilter::AcceptAllMatchingLayers: {
+        filterAcceptAllMatchingLayers(entity, layerIds);
+        break;
+    }
+    case QLayerFilter::DiscardAnyMatchingLayers: {
+        filterDiscardAnyMatchingLayers(entity, layerIds);
+        break;
+    }
+    case QLayerFilter::DiscardAllMatchingLayers: {
+        filterDiscardAllMatchingLayers(entity, layerIds);
+        break;
+    }
+    default:
+        Q_UNREACHABLE();
+    }
+}
+
 // We accept the entity if it contains any of the layers that are in the layer filter
 void FilterLayerEntityJob::filterAcceptAnyMatchingLayers(Entity *entity,
                                                          const Qt3DCore::QNodeIdVector &layerIds)
@@ -181,28 +208,8 @@ void FilterLayerEntityJob::filterLayerAndEntity()
         const QLayerFilter::FilterMode filterMode = layerFilter->filterMode();
 
         // Perform filtering
-        for (Entity *entity : entitiesToFilter) {
-            switch (filterMode) {
-            case QLayerFilter::AcceptAnyMatchingLayers: {
-                filterAcceptAnyMatchingLayers(entity, layerIds);
-                break;
-            }
-            case QLayerFilter::AcceptAllMatchingLayers: {
-                filterAcceptAllMatchingLayers(entity, layerIds);
-                break;
-            }
-            case QLayerFilter::DiscardAnyMatchingLayers: {
-                filterDiscardAnyMatchingLayers(entity, layerIds);
-                break;
-            }
-            case QLayerFilter::DiscardAllMatchingLayers: {
-                filterDiscardAllMatchingLayers(entity, layerIds);
-                break;
-            }
-            default:
-                Q_UNREACHABLE();
-            }
-        }
+        for (Entity *entity : entitiesToFilter)
+            filterEntityAgainstLayers(entity, layerIds, filterMode);
 
         // Entities to filter for the next frame are the filtered result of the
         // current LayerFilter
diff --git a/src/render/jobs/filterlayerentityjob_p.h b/src/render/jobs/filterlayerentityjob_p.h
index cdd309443..691e71c4b 100644
--- a/src/render/jobs/filterlayerentityjob_p.h
+++ b/src/render/jobs/filterlayerentityjob_p.h
@@ -80,6 +80,7 @@ public:
     // QAspectJob interface
     void run() final;
 
+    void filterEntityAgainstLayers(Entity *entity, const Qt3DCore::QNodeIdVector &layerIds, const QLayerFilter::FilterMode filterMode);
     void filterAcceptAnyMatchingLayers(Entity *entity, const Qt3DCore::QNodeIdVector &layerIds);
     void filterAcceptAllMatchingLayers(Entity *entity, const Qt3DCore::QNodeIdVector &layerIds);
     void filterDiscardAnyMatchingLayers(Entity *entity, const Qt3DCore::QNodeIdVector &layerIds);
diff --git a/src/render/jobs/pickboundingvolumejob.cpp b/src/render/jobs/pickboundingvolumejob.cpp
index 720a7e1d0..c3e3cb9bf 100644
--- a/src/render/jobs/pickboundingvolumejob.cpp
+++ b/src/render/jobs/pickboundingvolumejob.cpp
@@ -329,7 +329,8 @@ void PickBoundingVolumeJob::processPickEvent(const PickingUtils::PickConfigurati
         }
 
         PickingUtils::HierarchicalEntityPicker entityPicker(ray);
-        entityPicker.setFilterLayers(vca.layers, vca.layerFilterMode);
+        entityPicker.setLayerFilterIds(vca.layersFilters);
+
         if (entityPicker.collectHits(m_manager, m_node)) {
             if (pickConfiguration.trianglePickingRequested) {
                 PickingUtils::TriangleCollisionGathererFunctor gathererFunctor;
diff --git a/src/render/jobs/pickboundingvolumeutils.cpp b/src/render/jobs/pickboundingvolumeutils.cpp
index fbd12eb4b..10536c3c2 100644
--- a/src/render/jobs/pickboundingvolumeutils.cpp
+++ b/src/render/jobs/pickboundingvolumeutils.cpp
@@ -54,6 +54,7 @@
 #include <Qt3DRender/private/layer_p.h>
 #include <Qt3DRender/private/layerfilternode_p.h>
 #include <Qt3DRender/private/rendersettings_p.h>
+#include <Qt3DRender/private/filterlayerentityjob_p.h>
 
 #include <vector>
 #include <algorithm>
@@ -134,15 +135,7 @@ ViewportCameraAreaDetails ViewportCameraAreaGatherer::gatherUpViewportCameraArea
             }
             case FrameGraphNode::LayerFilter: {
                 auto fnode = static_cast<const LayerFilterNode *>(node);
-                const auto &layers = fnode->layerIds();
-                for (const auto &id: layers)
-                    vca.layers.append(id);
-                switch (fnode->filterMode()) {
-                case Qt3DRender::QLayerFilter::AcceptAllMatchingLayers: vca.layerFilterMode = Qt3DRender::QAbstractRayCaster::AcceptAllMatchingLayers; break;
-                case Qt3DRender::QLayerFilter::AcceptAnyMatchingLayers: vca.layerFilterMode = Qt3DRender::QAbstractRayCaster::AcceptAnyMatchingLayers; break;
-                case Qt3DRender::QLayerFilter::DiscardAllMatchingLayers: vca.layerFilterMode = Qt3DRender::QAbstractRayCaster::DiscardAllMatchingLayers; break;
-                case Qt3DRender::QLayerFilter::DiscardAnyMatchingLayers: vca.layerFilterMode = Qt3DRender::QAbstractRayCaster::DiscardAnyMatchingLayers; break;
-                }
+                vca.layersFilters.push_back(fnode->peerId());
                 break;
             }
             default:
@@ -179,7 +172,8 @@ bool ViewportCameraAreaGatherer::isUnique(const std::vector<ViewportCameraAreaDe
         if (vca.cameraId == listItem.cameraId &&
                 vca.viewport == listItem.viewport &&
                 vca.surface == listItem.surface &&
-                vca.area == listItem.area)
+                vca.area == listItem.area &&
+                vca.layersFilters == listItem.layersFilters)
             return false;
     }
     return true;
@@ -775,16 +769,19 @@ HitList PointCollisionGathererFunctor::pick(const Entity *entity) const
 HierarchicalEntityPicker::HierarchicalEntityPicker(const QRay3D &ray, bool requireObjectPicker)
     : m_ray(ray)
     , m_objectPickersRequired(requireObjectPicker)
-    , m_filterMode(QAbstractRayCaster::AcceptAnyMatchingLayers)
 {
+}
 
+void HierarchicalEntityPicker::setLayerFilterIds(const Qt3DCore::QNodeIdVector &layerFilterIds)
+{
+    m_layerFilterIds = layerFilterIds;
 }
 
-void HierarchicalEntityPicker::setFilterLayers(const Qt3DCore::QNodeIdVector &layerIds, QAbstractRayCaster::FilterMode mode)
+void HierarchicalEntityPicker::setLayerIds(const Qt3DCore::QNodeIdVector &layerIds,
+                                           QAbstractRayCaster::FilterMode mode)
 {
-    m_filterMode = mode;
     m_layerIds = layerIds;
-    std::sort(m_layerIds.begin(), m_layerIds.end());
+    m_layerFilterMode = mode;
 }
 
 bool HierarchicalEntityPicker::collectHits(NodeManagers *manager, Entity *root)
@@ -797,60 +794,37 @@ bool HierarchicalEntityPicker::collectHits(NodeManagers *manager, Entity *root)
     struct EntityData {
         Entity* entity;
         bool hasObjectPicker;
-        Qt3DCore::QNodeIdVector recursiveLayers;
         int priority;
     };
     std::vector<EntityData> worklist;
-    worklist.push_back({root, !root->componentHandle<ObjectPicker>().isNull(), {}, 0});
-
-    LayerManager *layerManager = manager->layerManager();
+    worklist.push_back({root, !root->componentHandle<ObjectPicker>().isNull(), 0});
+
+    // Record all entities that satisfy layerFiltering. We can then check against
+    // that to see if a picked Entity also satisfies the layer filtering
+
+    // Note: PickBoundingVolumeJob filters against LayerFilter nodes (FG) whereas
+    // the RayCastingJob filters only against a set of Layers and a filter Mode
+    const bool hasLayerFilters = m_layerFilterIds.size() > 0;
+    const bool hasLayers = m_layerIds.size() > 0;
+    const bool hasLayerFiltering = hasLayerFilters || hasLayers;
+    std::vector<Entity *> layerFilterEntities;
+    FilterLayerEntityJob layerFilterJob;
+    layerFilterJob.setManager(manager);
+
+    if (hasLayerFilters) {
+        // Note: we expect UpdateEntityLayersJob was called beforehand to handle layer recursivness
+        // Filtering against LayerFilters (PickBoundingVolumeJob)
+        if (m_layerFilterIds.size()) {
+            layerFilterJob.setLayerFilters(m_layerFilterIds);
+            layerFilterJob.run();
+            layerFilterEntities = layerFilterJob.filteredEntities();
+        }
+    }
 
     while (!worklist.empty()) {
         EntityData current = worklist.back();
         worklist.pop_back();
 
-        bool accepted = true;
-        if (m_layerIds.size()) {
-            // TODO investigate reusing logic from LayerFilter job
-            Qt3DCore::QNodeIdVector filterLayers = current.recursiveLayers + current.entity->componentsUuid<Layer>();
-
-            // remove disabled layers
-            filterLayers.erase(std::remove_if(filterLayers.begin(), filterLayers.end(),
-                                              [layerManager](const Qt3DCore::QNodeId layerId) {
-                Layer *layer = layerManager->lookupResource(layerId);
-                return !layer || !layer->isEnabled();
-            }), filterLayers.end());
-
-            std::sort(filterLayers.begin(), filterLayers.end());
-
-            Qt3DCore::QNodeIdVector commonIds;
-            std::set_intersection(m_layerIds.cbegin(), m_layerIds.cend(),
-                                  filterLayers.cbegin(), filterLayers.cend(),
-                                  std::back_inserter(commonIds));
-
-            switch (m_filterMode) {
-            case QAbstractRayCaster::AcceptAnyMatchingLayers: {
-                accepted = !commonIds.empty();
-                break;
-            }
-            case QAbstractRayCaster::AcceptAllMatchingLayers: {
-                accepted = commonIds == m_layerIds;
-                break;
-            }
-            case QAbstractRayCaster::DiscardAnyMatchingLayers: {
-                accepted = commonIds.empty();
-                break;
-            }
-            case QAbstractRayCaster::DiscardAllMatchingLayers: {
-                accepted = !(commonIds == m_layerIds);
-                break;
-            }
-            default:
-                Q_UNREACHABLE();
-                break;
-            }
-        }
-
         // first pick entry sub-scene-graph
         QCollisionQueryResult::Hit queryResult =
                 rayCasting.query(m_ray, current.entity->worldBoundingVolumeWithChildren());
@@ -859,21 +833,27 @@ bool HierarchicalEntityPicker::collectHits(NodeManagers *manager, Entity *root)
 
         // if we get a hit, we check again for this specific entity
         queryResult = rayCasting.query(m_ray, current.entity->worldBoundingVolume());
-        if (accepted && queryResult.m_distance >= 0.f && (current.hasObjectPicker || !m_objectPickersRequired)) {
+
+        // Check Entity is in selected Layers if we have LayerIds or LayerFilterIds
+        // Note: it's not because a parent doesn't satisfy the layerFiltering that a child might not.
+        // Therefore we need to keep traversing children in all cases
+
+        // Are we filtering against layerIds (RayCastingJob)
+        if (hasLayers) {
+            // QLayerFilter::FilterMode and QAbstractRayCaster::FilterMode are the same
+            layerFilterJob.filterEntityAgainstLayers(current.entity, m_layerIds, static_cast<QLayerFilter::FilterMode>(m_layerFilterMode));
+            layerFilterEntities = layerFilterJob.filteredEntities();
+        }
+
+        const bool isInLayers = !hasLayerFiltering || Qt3DCore::contains(layerFilterEntities, current.entity);
+
+        if (isInLayers && queryResult.m_distance >= 0.f && (current.hasObjectPicker || !m_objectPickersRequired)) {
             m_entities.push_back(current.entity);
             m_hits.push_back(queryResult);
             // Record entry for entity/priority
             m_entityToPriorityTable.insert(current.entity->peerId(), current.priority);
         }
 
-        Qt3DCore::QNodeIdVector recursiveLayers;
-        const Qt3DCore::QNodeIdVector entityLayers = current.entity->componentsUuid<Layer>();
-        for (const Qt3DCore::QNodeId &layerId : entityLayers) {
-            Layer *layer = layerManager->lookupResource(layerId);
-            if (layer->recursive())
-                recursiveLayers << layerId;
-        }
-
         // and pick children
         const auto &childrenHandles = current.entity->childrenHandles();
         for (const HEntity &handle : childrenHandles) {
@@ -881,7 +861,6 @@ bool HierarchicalEntityPicker::collectHits(NodeManagers *manager, Entity *root)
             if (child) {
                 ObjectPicker *childPicker = child->renderComponent<ObjectPicker>();
                 worklist.push_back({child, current.hasObjectPicker || childPicker,
-                                    current.recursiveLayers + recursiveLayers,
                                     (childPicker ? childPicker->priority() : current.priority)});
             }
         }
diff --git a/src/render/jobs/pickboundingvolumeutils_p.h b/src/render/jobs/pickboundingvolumeutils_p.h
index d9c8eee22..6a637a1d7 100644
--- a/src/render/jobs/pickboundingvolumeutils_p.h
+++ b/src/render/jobs/pickboundingvolumeutils_p.h
@@ -84,8 +84,7 @@ struct Q_AUTOTEST_EXPORT ViewportCameraAreaDetails
     QRectF viewport;
     QSize area;
     QSurface *surface = nullptr;
-    Qt3DCore::QNodeIdVector layers;
-    QAbstractRayCaster::FilterMode layerFilterMode = QAbstractRayCaster::AcceptAnyMatchingLayers;
+    Qt3DCore::QNodeIdVector layersFilters;
 };
 QT3D_DECLARE_TYPEINFO_3(Qt3DRender, Render, PickingUtils, ViewportCameraAreaDetails, Q_COMPLEX_TYPE)
 
@@ -124,7 +123,8 @@ class Q_AUTOTEST_EXPORT HierarchicalEntityPicker
 public:
     explicit HierarchicalEntityPicker(const RayCasting::QRay3D &ray, bool requireObjectPicker = true);
 
-    void setFilterLayers(const Qt3DCore::QNodeIdVector &layerIds, QAbstractRayCaster::FilterMode mode);
+    void setLayerFilterIds(const Qt3DCore::QNodeIdVector &layerFilterIds);
+    void setLayerIds(const Qt3DCore::QNodeIdVector &layerIds, QAbstractRayCaster::FilterMode mode);
 
     bool collectHits(NodeManagers *manager, Entity *root);
     inline HitList hits() const { return m_hits; }
@@ -136,8 +136,9 @@ private:
     HitList m_hits;
     std::vector<Entity *> m_entities;
     bool m_objectPickersRequired;
+    Qt3DCore::QNodeIdVector m_layerFilterIds;
     Qt3DCore::QNodeIdVector m_layerIds;
-    QAbstractRayCaster::FilterMode m_filterMode;
+    QAbstractRayCaster::FilterMode m_layerFilterMode = QAbstractRayCaster::AcceptAnyMatchingLayers;
     QHash<Qt3DCore::QNodeId, int> m_entityToPriorityTable;
 };
 
diff --git a/src/render/jobs/raycastingjob.cpp b/src/render/jobs/raycastingjob.cpp
index 7c7ddc71a..b6ebdad99 100644
--- a/src/render/jobs/raycastingjob.cpp
+++ b/src/render/jobs/raycastingjob.cpp
@@ -201,7 +201,7 @@ bool RayCastingJob::pick(const QList<QPair<Entity *, RayCaster *>> &entities)
         for (const QRay3D &ray: qAsConst(rays)) {
             PickingUtils::HitList sphereHits;
             PickingUtils::HierarchicalEntityPicker entityPicker(ray, false);
-            entityPicker.setFilterLayers(pair.second->layerIds(), pair.second->filterMode());
+            entityPicker.setLayerIds(pair.second->layerIds(), pair.second->filterMode());
             if (entityPicker.collectHits(m_manager, m_node)) {
                 if (pickConfiguration.trianglePickingRequested) {
                     PickingUtils::TriangleCollisionGathererFunctor gathererFunctor;