Handle multiple surfaces properly for picking

This fixes issues with picking when having multiple viewports and/or
multiple windows.

Now check that the mouse event actually hits inside the viewport extents.

Also track the source of events and check that the surface matches the
source of the event.

Remaining issue is overlapping viewports within the same window

Task-number: QTBUG-59567
Change-Id: I76a4ee2bec7300d893fef6040d89bf81f2109795
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

10 files changed, 89 insertions, 53 deletions

diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp
index acb640caf..e45314900 100644
--- a/src/render/backend/renderer.cpp
+++ b/src/render/backend/renderer.cpp
@@ -1821,7 +1821,7 @@ void Renderer::cleanGraphicsResources()
     }
 }
 
-QList<QMouseEvent> Renderer::pendingPickingEvents() const
+QList<QPair<QObject *, QMouseEvent>> Renderer::pendingPickingEvents() const
 {
     return m_pickEventFilter->pendingMouseEvents();
 }
diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h
index 6311f35d6..b18b78a7c 100644
--- a/src/render/backend/renderer_p.h
+++ b/src/render/backend/renderer_p.h
@@ -234,7 +234,7 @@ public:
 
     inline RenderStateSet *defaultRenderState() const { return m_defaultRenderStateSet; }
 
-    QList<QMouseEvent> pendingPickingEvents() const;
+    QList<QPair<QObject*, QMouseEvent>> pendingPickingEvents() const;
     QList<QKeyEvent> pendingKeyEvents() const;
 
     void addRenderCaptureSendRequest(Qt3DCore::QNodeId nodeId);
diff --git a/src/render/jobs/pickboundingvolumejob.cpp b/src/render/jobs/pickboundingvolumejob.cpp
index 0e751a6e0..4eeb8fa6e 100644
--- a/src/render/jobs/pickboundingvolumejob.cpp
+++ b/src/render/jobs/pickboundingvolumejob.cpp
@@ -51,6 +51,10 @@
 #include <Qt3DRender/private/job_common_p.h>
 #include <Qt3DRender/private/qpickevent_p.h>
 
+#include <QSurface>
+#include <QWindow>
+#include <QOffscreenSurface>
+
 QT_BEGIN_NAMESPACE
 
 namespace Qt3DRender {
@@ -118,7 +122,7 @@ void PickBoundingVolumeJob::setRoot(Entity *root)
     m_node = root;
 }
 
-void PickBoundingVolumeJob::setMouseEvents(const QList<QMouseEvent> &pendingEvents)
+void PickBoundingVolumeJob::setMouseEvents(const QList<QPair<QObject*, QMouseEvent>> &pendingEvents)
 {
     m_pendingMouseEvents = pendingEvents;
 }
@@ -184,8 +188,8 @@ bool PickBoundingVolumeJob::runHelper()
     bool hasMoveEvent = false;
     bool hasOtherEvent = false;
     // Quickly look which types of events we've got
-    for (const QMouseEvent &event : mouseEvents) {
-        const bool isMove = (event.type() == QEvent::MouseMove);
+    for (const auto &event : mouseEvents) {
+        const bool isMove = (event.second.type() == QEvent::MouseMove);
         hasMoveEvent |= isMove;
         hasOtherEvent |= !isMove;
     }
@@ -213,10 +217,10 @@ bool PickBoundingVolumeJob::runHelper()
 
     PickingUtils::ViewportCameraAreaGatherer vcaGatherer;
     // TO DO: We could cache this and only gather when we know the FrameGraph tree has changed
-    const QVector<PickingUtils::ViewportCameraAreaTriplet> vcaTriplets = vcaGatherer.gather(m_frameGraphRoot);
+    const QVector<PickingUtils::ViewportCameraAreaDetails> vcaDetails = vcaGatherer.gather(m_frameGraphRoot);
 
     // If we have no viewport / camera or area, return early
-    if (vcaTriplets.empty())
+    if (vcaDetails.empty())
         return false;
 
     // TO DO:
@@ -237,19 +241,21 @@ bool PickBoundingVolumeJob::runHelper()
     const ReducerFunction reducerOp = allHitsRequested ? PickingUtils::reduceToAllHits : PickingUtils::reduceToFirstHit;
 
     // For each mouse event
-    for (const QMouseEvent &event : mouseEvents) {
+    for (const auto &event : mouseEvents) {
         m_hoveredPickersToClear = m_hoveredPickers;
 
         QPickEvent::Buttons eventButton = QPickEvent::NoButton;
         int eventButtons = 0;
         int eventModifiers = QPickEvent::NoModifier;
 
-        setEventButtonAndModifiers(event, eventButton, eventButtons, eventModifiers);
+        setEventButtonAndModifiers(event.second, eventButton, eventButtons, eventModifiers);
 
-        // For each triplet of Viewport / Camera and Area
-        for (const PickingUtils::ViewportCameraAreaTriplet &vca : vcaTriplets) {
+        // For each Viewport / Camera and Area entry
+        for (const PickingUtils::ViewportCameraAreaDetails &vca : vcaDetails) {
             HitList sphereHits;
-            QRay3D ray = rayForViewportAndCamera(vca.area, event.pos(), vca.viewport, vca.cameraId);
+            QRay3D ray = rayForViewportAndCamera(vca, event.first, event.second);
+            if (!ray.isValid())
+                continue;
 
             PickingUtils::HierarchicalEntityPicker entityPicker(ray);
             if (entityPicker.collectHits(m_node)) {
@@ -270,7 +276,7 @@ bool PickBoundingVolumeJob::runHelper()
             }
 
             // Dispatch events based on hit results
-            dispatchPickEvents(event, sphereHits, eventButton, eventButtons, eventModifiers,
+            dispatchPickEvents(event.second, sphereHits, eventButton, eventButtons, eventModifiers,
                                trianglePickingRequested, allHitsRequested);
         }
     }
@@ -451,18 +457,38 @@ QRect PickBoundingVolumeJob::windowViewport(const QSize &area, const QRectF &rel
     return relativeViewport.toRect();
 }
 
-RayCasting::QRay3D PickBoundingVolumeJob::rayForViewportAndCamera(const QSize &area,
-                                                                  const QPoint &pos,
-                                                                  const QRectF &relativeViewport,
-                                                                  const Qt3DCore::QNodeId cameraId) const
+RayCasting::QRay3D PickBoundingVolumeJob::rayForViewportAndCamera(const PickingUtils::ViewportCameraAreaDetails &vca,
+                                                                  QObject *eventSource,
+                                                                  const QMouseEvent &event) const
 {
+    static RayCasting::QRay3D invalidRay({}, {}, 0.f);
+
     QMatrix4x4 viewMatrix;
     QMatrix4x4 projectionMatrix;
-    viewMatrixForCamera(cameraId, viewMatrix, projectionMatrix);
-    const QRect viewport = windowViewport(area, relativeViewport);
+    viewMatrixForCamera(vca.cameraId, viewMatrix, projectionMatrix);
+    const QRect viewport = windowViewport(vca.area, vca.viewport);
+    const QPoint pos = event.pos();
+
+    if (vca.area.isValid() && !viewport.contains(pos))
+        return invalidRay;
+    if (vca.surface) {
+        QSurface *surface = nullptr;
+        if (eventSource) {
+            QWindow *window = qobject_cast<QWindow *>(eventSource);
+            if (window) {
+                surface = static_cast<QSurface *>(window);
+            } else {
+                QOffscreenSurface *offscreen = qobject_cast<QOffscreenSurface *>(eventSource);
+                if (offscreen)
+                    surface = static_cast<QSurface *>(offscreen);
+            }
+        }
+        if (surface && vca.surface != surface)
+            return invalidRay;
+    }
 
     // In GL the y is inverted compared to Qt
-    const QPoint glCorrectPos = QPoint(pos.x(), area.isValid() ? area.height() - pos.y() : pos.y());
+    const QPoint glCorrectPos = QPoint(pos.x(), vca.area.isValid() ? vca.area.height() - pos.y() : pos.y());
     const auto ray = intersectionRay(glCorrectPos, viewMatrix, projectionMatrix, viewport);
     return ray;
 }
diff --git a/src/render/jobs/pickboundingvolumejob_p.h b/src/render/jobs/pickboundingvolumejob_p.h
index 5239c5c6c..163c5b62d 100644
--- a/src/render/jobs/pickboundingvolumejob_p.h
+++ b/src/render/jobs/pickboundingvolumejob_p.h
@@ -82,7 +82,7 @@ public:
     PickBoundingVolumeJob();
 
     void setRoot(Entity *root);
-    void setMouseEvents(const QList<QMouseEvent> &pendingEvents);
+    void setMouseEvents(const QList<QPair<QObject*, QMouseEvent>> &pendingEvents);
     void setKeyEvents(const QList<QKeyEvent> &pendingEvents);
     void setFrameGraphRoot(FrameGraphNode *frameGraphRoot);
     void setRenderSettings(RenderSettings *settings);
@@ -114,7 +114,7 @@ private:
     Entity *m_node;
     FrameGraphNode *m_frameGraphRoot;
     RenderSettings *m_renderSettings;
-    QList<QMouseEvent> m_pendingMouseEvents;
+    QList<QPair<QObject*, QMouseEvent>> m_pendingMouseEvents;
     bool m_pickersDirty;
     bool m_oneEnabledAtLeast;
     bool m_oneHoverAtLeast;
@@ -125,10 +125,9 @@ private:
                              QMatrix4x4 &viewMatrix,
                              QMatrix4x4 &projectionMatrix) const;
     QRect windowViewport(const QSize &area, const QRectF &relativeViewport) const;
-    RayCasting::QRay3D rayForViewportAndCamera(const QSize &area,
-                                               const QPoint &pos,
-                                               const QRectF &relativeViewport,
-                                               const Qt3DCore::QNodeId cameraId) const;
+    RayCasting::QRay3D rayForViewportAndCamera(const PickingUtils::ViewportCameraAreaDetails &vca,
+                                               QObject *eventSource,
+                                               const QMouseEvent &event) const;
     void clearPreviouslyHoveredPickers();
     HObjectPicker m_currentPicker;
     QVector<HObjectPicker> m_hoveredPickers;
diff --git a/src/render/jobs/pickboundingvolumeutils.cpp b/src/render/jobs/pickboundingvolumeutils.cpp
index 5c778bf29..5de551cf6 100644
--- a/src/render/jobs/pickboundingvolumeutils.cpp
+++ b/src/render/jobs/pickboundingvolumeutils.cpp
@@ -68,9 +68,9 @@ void ViewportCameraAreaGatherer::visit(FrameGraphNode *node)
         m_leaves.push_back(node);
 }
 
-ViewportCameraAreaTriplet ViewportCameraAreaGatherer::gatherUpViewportCameraAreas(Render::FrameGraphNode *node) const
+ViewportCameraAreaDetails ViewportCameraAreaGatherer::gatherUpViewportCameraAreas(Render::FrameGraphNode *node) const
 {
-    ViewportCameraAreaTriplet vca;
+    ViewportCameraAreaDetails vca;
     vca.viewport = QRectF(0.0f, 0.0f, 1.0f, 1.0f);
 
     while (node) {
@@ -82,9 +82,12 @@ ViewportCameraAreaTriplet ViewportCameraAreaGatherer::gatherUpViewportCameraArea
             case FrameGraphNode::Viewport:
                 vca.viewport = computeViewport(vca.viewport, static_cast<const ViewportNode *>(node));
                 break;
-            case FrameGraphNode::Surface:
-                vca.area = static_cast<const RenderSurfaceSelector *>(node)->renderTargetSize();
+            case FrameGraphNode::Surface: {
+                auto selector = static_cast<const RenderSurfaceSelector *>(node);
+                vca.area = selector->renderTargetSize();
+                vca.surface = selector->surface();
                 break;
+            }
             default:
                 break;
             }
@@ -94,28 +97,31 @@ ViewportCameraAreaTriplet ViewportCameraAreaGatherer::gatherUpViewportCameraArea
     return vca;
 }
 
-QVector<ViewportCameraAreaTriplet> ViewportCameraAreaGatherer::gather(FrameGraphNode *root)
+QVector<ViewportCameraAreaDetails> ViewportCameraAreaGatherer::gather(FrameGraphNode *root)
 {
     // Retrieve all leaves
     visit(root);
-    QVector<ViewportCameraAreaTriplet> vcaTriplets;
+    QVector<ViewportCameraAreaDetails> vcaTriplets;
     vcaTriplets.reserve(m_leaves.count());
 
     // Find all viewport/camera pairs by traversing from leaf to root
     for (Render::FrameGraphNode *leaf : qAsConst(m_leaves)) {
-        ViewportCameraAreaTriplet vcaTriplet = gatherUpViewportCameraAreas(leaf);
-        if (!m_targetCamera.isNull() && vcaTriplet.cameraId != m_targetCamera)
+        ViewportCameraAreaDetails vcaDetails = gatherUpViewportCameraAreas(leaf);
+        if (!m_targetCamera.isNull() && vcaDetails.cameraId != m_targetCamera)
             continue;
-        if (!vcaTriplet.cameraId.isNull() && isUnique(vcaTriplets, vcaTriplet))
-            vcaTriplets.push_back(vcaTriplet);
+        if (!vcaDetails.cameraId.isNull() && isUnique(vcaTriplets, vcaDetails))
+            vcaTriplets.push_back(vcaDetails);
     }
     return vcaTriplets;
 }
 
-bool PickingUtils::ViewportCameraAreaGatherer::isUnique(const QVector<ViewportCameraAreaTriplet> &vcaTriplets, const ViewportCameraAreaTriplet &vca) const
+bool PickingUtils::ViewportCameraAreaGatherer::isUnique(const QVector<ViewportCameraAreaDetails> &vcaList, const ViewportCameraAreaDetails &vca) const
 {
-    for (const ViewportCameraAreaTriplet &triplet : vcaTriplets) {
-        if (vca.cameraId == triplet.cameraId && vca.viewport == triplet.viewport && vca.area == triplet.area)
+    for (const ViewportCameraAreaDetails &listItem : vcaList) {
+        if (vca.cameraId == listItem.cameraId &&
+                vca.viewport == listItem.viewport &&
+                vca.surface == listItem.surface &&
+                vca.area == listItem.area)
             return false;
     }
     return true;
diff --git a/src/render/jobs/pickboundingvolumeutils_p.h b/src/render/jobs/pickboundingvolumeutils_p.h
index 08615c094..8bc108d89 100644
--- a/src/render/jobs/pickboundingvolumeutils_p.h
+++ b/src/render/jobs/pickboundingvolumeutils_p.h
@@ -59,6 +59,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class QSurface;
+
 namespace Qt3DRender {
 namespace RayCasting {
 class QAbstractCollisionQueryService;
@@ -72,27 +74,28 @@ class FrameGraphNode;
 
 namespace PickingUtils {
 
-struct Q_AUTOTEST_EXPORT ViewportCameraAreaTriplet
+struct Q_AUTOTEST_EXPORT ViewportCameraAreaDetails
 {
     Qt3DCore::QNodeId cameraId;
     QRectF viewport;
     QSize area;
+    QSurface *surface = nullptr;
 };
-QT3D_DECLARE_TYPEINFO_3(Qt3DRender, Render, PickingUtils, ViewportCameraAreaTriplet, Q_PRIMITIVE_TYPE)
+QT3D_DECLARE_TYPEINFO_3(Qt3DRender, Render, PickingUtils, ViewportCameraAreaDetails, Q_PRIMITIVE_TYPE)
 
 class Q_AUTOTEST_EXPORT ViewportCameraAreaGatherer
 {
 public:
     ViewportCameraAreaGatherer(const Qt3DCore::QNodeId &nodeId = Qt3DCore::QNodeId()) : m_targetCamera(nodeId) { }
-    QVector<ViewportCameraAreaTriplet> gather(FrameGraphNode *root);
+    QVector<ViewportCameraAreaDetails> gather(FrameGraphNode *root);
 
 private:
     Qt3DCore::QNodeId m_targetCamera;
     QVector<FrameGraphNode *> m_leaves;
 
     void visit(FrameGraphNode *node);
-    ViewportCameraAreaTriplet gatherUpViewportCameraAreas(Render::FrameGraphNode *node) const;
-    bool isUnique(const QVector<ViewportCameraAreaTriplet> &vcaTriplets, const ViewportCameraAreaTriplet &vca) const;
+    ViewportCameraAreaDetails gatherUpViewportCameraAreas(Render::FrameGraphNode *node) const;
+    bool isUnique(const QVector<ViewportCameraAreaDetails> &vcaList, const ViewportCameraAreaDetails &vca) const;
 };
 
 class Q_AUTOTEST_EXPORT EntityGatherer
diff --git a/src/render/jobs/updatelevelofdetailjob.cpp b/src/render/jobs/updatelevelofdetailjob.cpp
index 709c73cf1..441d86a27 100644
--- a/src/render/jobs/updatelevelofdetailjob.cpp
+++ b/src/render/jobs/updatelevelofdetailjob.cpp
@@ -190,11 +190,11 @@ void UpdateLevelOfDetailJob::updateEntityLodByScreenArea(Entity *entity, LevelOf
         return;
 
     PickingUtils::ViewportCameraAreaGatherer vcaGatherer(lod->camera());
-    const QVector<PickingUtils::ViewportCameraAreaTriplet> vcaTriplets = vcaGatherer.gather(m_frameGraphRoot);
+    const QVector<PickingUtils::ViewportCameraAreaDetails> vcaTriplets = vcaGatherer.gather(m_frameGraphRoot);
     if (vcaTriplets.isEmpty())
         return;
 
-    const PickingUtils::ViewportCameraAreaTriplet &vca = vcaTriplets.front();
+    const PickingUtils::ViewportCameraAreaDetails &vca = vcaTriplets.front();
 
     const QVector<qreal> thresholds = lod->thresholds();
     Sphere bv(lod->center(), lod->radius());
diff --git a/src/render/picking/pickeventfilter.cpp b/src/render/picking/pickeventfilter.cpp
index 297911e45..b10383c72 100644
--- a/src/render/picking/pickeventfilter.cpp
+++ b/src/render/picking/pickeventfilter.cpp
@@ -62,10 +62,10 @@ PickEventFilter::~PickEventFilter()
     Called from a worker thread in the thread pool so be sure to
     mutex protect the data.
 */
-QList<QMouseEvent> PickEventFilter::pendingMouseEvents()
+QList<QPair<QObject *, QMouseEvent> > PickEventFilter::pendingMouseEvents()
 {
     QMutexLocker locker(&m_mutex);
-    QList<QMouseEvent> pendingEvents(m_pendingMouseEvents);
+    QList<QPair<QObject*, QMouseEvent>> pendingEvents(m_pendingMouseEvents);
     m_pendingMouseEvents.clear();
     return pendingEvents;
 }
@@ -90,14 +90,14 @@ bool PickEventFilter::eventFilter(QObject *obj, QEvent *e)
     case QEvent::MouseButtonRelease:
     case QEvent::MouseMove: {
         QMutexLocker locker(&m_mutex);
-        m_pendingMouseEvents.push_back(QMouseEvent(*static_cast<QMouseEvent *>(e)));
+        m_pendingMouseEvents.push_back({obj, QMouseEvent(*static_cast<QMouseEvent *>(e))});
     } break;
     case QEvent::HoverMove: {
         QMutexLocker locker(&m_mutex);
         QHoverEvent *he = static_cast<QHoverEvent *>(e);
-        m_pendingMouseEvents.push_back(QMouseEvent(QEvent::MouseMove,
+        m_pendingMouseEvents.push_back({obj, QMouseEvent(QEvent::MouseMove,
                                                    he->pos(), Qt::NoButton, Qt::NoButton,
-                                                   he->modifiers()));
+                                                   he->modifiers())});
     } break;
     case QEvent::KeyPress:
     case QEvent::KeyRelease: {
diff --git a/src/render/picking/pickeventfilter_p.h b/src/render/picking/pickeventfilter_p.h
index fc4b00ddc..da41598b3 100644
--- a/src/render/picking/pickeventfilter_p.h
+++ b/src/render/picking/pickeventfilter_p.h
@@ -69,14 +69,14 @@ public:
     explicit PickEventFilter(QObject *parent = nullptr);
     ~PickEventFilter();
 
-    QList<QMouseEvent> pendingMouseEvents();
+    QList<QPair<QObject*, QMouseEvent>> pendingMouseEvents();
     QList<QKeyEvent> pendingKeyEvents();
 
 protected:
     bool eventFilter(QObject *obj, QEvent *e) Q_DECL_FINAL;
 
 private:
-    QList<QMouseEvent> m_pendingMouseEvents;
+    QList<QPair<QObject*, QMouseEvent>> m_pendingMouseEvents;
     QList<QKeyEvent> m_pendingKeyEvents;
     QMutex m_mutex;
 };
diff --git a/src/render/raycasting/qray3d_p.h b/src/render/raycasting/qray3d_p.h
index 7c1156b76..64716307c 100644
--- a/src/render/raycasting/qray3d_p.h
+++ b/src/render/raycasting/qray3d_p.h
@@ -93,6 +93,8 @@ public:
     bool operator==(const QRay3D &other) const;
     bool operator!=(const QRay3D &other) const;
 
+    bool isValid() const { return !m_direction.isNull() && !qFuzzyIsNull(m_distance); }
+
 private:
     QVector3D m_origin;
     QVector3D m_direction;