Fix picking with Scene3D

Pass the correct picking area to pick job using rendersurfaceselector

Task-number: QTBUG-49271
Change-Id: I30511199b9b35c0e70d03f31165eb35c8c015cd1
Reviewed-by: Tomi Korpipää <tomi.korpipaa@theqtcompany.com>
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

10 files changed, 178 insertions, 43 deletions

diff --git a/src/quick3d/imports/scene3d/scene3ditem.cpp b/src/quick3d/imports/scene3d/scene3ditem.cpp
index 457a38316..1364f3c05 100644
--- a/src/quick3d/imports/scene3d/scene3ditem.cpp
+++ b/src/quick3d/imports/scene3d/scene3ditem.cpp
@@ -218,6 +218,31 @@ void Scene3DItem::setWindowSurface(QObject *rootObject)
         surfaceSelector->setWindow(this->window());
 }
 
+void Scene3DItem::setItemArea(const QSize &area)
+{
+    // Find surface selector in framegraph and set the area
+    Qt3DRender::QRenderSettings *renderSettings
+        = m_entity->findChild<Qt3DRender::QRenderSettings *>();
+    if (!renderSettings) {
+        qWarning() << "No renderer settings component found";
+        return;
+    }
+
+    Qt3DCore::QNode *frameGraphRoot = renderSettings->activeFrameGraph();
+    if (!frameGraphRoot) {
+        qWarning() << "No active frame graph found";
+        return;
+    }
+
+    Qt3DRender::QRenderSurfaceSelector *surfaceSelector
+        = frameGraphRoot->findChild<Qt3DRender::QRenderSurfaceSelector *>();
+    if (!surfaceSelector) {
+        qWarning() << "No render surface selector found in frame graph";
+        return;
+    }
+    surfaceSelector->setExternalRenderTargetSize(area);
+}
+
 void Scene3DItem::setCameraAspectModeHelper()
 {
     switch (m_cameraAspectRatioMode) {
diff --git a/src/quick3d/imports/scene3d/scene3ditem_p.h b/src/quick3d/imports/scene3d/scene3ditem_p.h
index 577287cde..1419d9826 100644
--- a/src/quick3d/imports/scene3d/scene3ditem_p.h
+++ b/src/quick3d/imports/scene3d/scene3ditem_p.h
@@ -85,6 +85,7 @@ public:
 
     bool multisample() const;
     void setMultisample(bool enable);
+    void setItemArea(const QSize &area);
 
     enum CameraAspectRatioMode {
         AutomaticAspectRatio,
diff --git a/src/quick3d/imports/scene3d/scene3drenderer.cpp b/src/quick3d/imports/scene3d/scene3drenderer.cpp
index 690822280..61f2aafb9 100644
--- a/src/quick3d/imports/scene3d/scene3drenderer.cpp
+++ b/src/quick3d/imports/scene3d/scene3drenderer.cpp
@@ -219,9 +219,13 @@ void Scene3DRenderer::render()
 
     ContextSaver saver;
 
-    const QSize currentSize = m_item->boundingRect().size().toSize() * window->effectiveDevicePixelRatio();
+    const QSize boundingRectSize = m_item->boundingRect().size().toSize();
+    const QSize currentSize = boundingRectSize * window->effectiveDevicePixelRatio();
     const bool forceRecreate = currentSize != m_lastSize || m_multisample != m_lastMultisample;
 
+    if (currentSize != m_lastSize)
+        m_item->setItemArea(boundingRectSize);
+
     // Rebuild FBO and textures if never created or a resize has occurred
     if ((m_multisampledFBO.isNull() || forceRecreate) && m_multisample) {
         qCDebug(Scene3D) << Q_FUNC_INFO << "Creating multisample framebuffer";
diff --git a/src/render/framegraph/qrendersurfaceselector.cpp b/src/render/framegraph/qrendersurfaceselector.cpp
index bb92770d3..a0d7e3a47 100644
--- a/src/render/framegraph/qrendersurfaceselector.cpp
+++ b/src/render/framegraph/qrendersurfaceselector.cpp
@@ -42,27 +42,77 @@
 
 #include <QtGui/qwindow.h>
 #include <QtGui/qoffscreensurface.h>
+#include <Qt3DCore/qscenepropertychange.h>
 
 QT_BEGIN_NAMESPACE
 
 namespace Qt3DRender {
 
+/*!
+ * \class Qt3DRender::QRenderSurfaceSelector
+ * \inmodule Qt3DRender
+ * \brief QRenderSurfaceSelector
+ * \since
+ *
+ * \inherits Qt3DRender::QFrameGraphNode
+ *
+ */
+
+/*!
+ * \qmltype RenderSurfaceSelector
+ * \inqmlmodule Qt3D.Render
+ * \since
+ * \ingroup
+ * \instantiates Qt3DRender::QRenderSurfaceSelector
+ * \brief RenderSurfaceSelector
+ *
+ */
+
+/*! \qmlproperty QSurface Qt3D.Render::RenderSurfaceSelector::surface
+ *
+ * Holds the surface.
+ */
+
+/*! \qmlproperty QWindow Qt3D.Render::RenderSurfaceSelector::window
+ *
+ * Holds the window.
+ */
+
+/*! \qmlproperty QSize Qt3D.Render::RenderSurfaceSelector::externalRenderTargetSize
+ *
+ * Holds the size of the external render target.
+ */
+
 QRenderSurfaceSelectorPrivate::QRenderSurfaceSelectorPrivate()
     : Qt3DRender::QFrameGraphNodePrivate()
     , m_surface(Q_NULLPTR)
 {
 }
 
+void QRenderSurfaceSelectorPrivate::setExternalRenderTargetSize(const QSize &size)
+{
+    m_externalRenderTargetSize = size;
+}
+
+/*!
+ * Constructs QRenderSurfaceSelector with given \a parent.
+ */
 QRenderSurfaceSelector::QRenderSurfaceSelector(Qt3DCore::QNode *parent)
     : Qt3DRender::QFrameGraphNode(*new QRenderSurfaceSelectorPrivate, parent)
 {
 }
 
+/*!
+ * \internal
+ */
 QRenderSurfaceSelector::QRenderSurfaceSelector(QRenderSurfaceSelectorPrivate &dd, Qt3DCore::QNode *parent)
     : Qt3DRender::QFrameGraphNode(dd, parent)
 {
 }
 
+/*!
+ * Destroys QRenderSurfaceSelector.
+ */
 QRenderSurfaceSelector::~QRenderSurfaceSelector()
 {
     QNode::cleanup();
@@ -82,6 +132,10 @@ QWindow *QRenderSurfaceSelector::window() const
     return Q_NULLPTR;
 }
 
+/*! \property QRenderSurfaceSelector::surface
+ *
+ * Sets \a surface.
+ */
 void QRenderSurfaceSelector::setSurface(QSurface *surface)
 {
     Q_D(QRenderSurfaceSelector);
@@ -92,6 +146,27 @@ void QRenderSurfaceSelector::setSurface(QSurface *surface)
     emit surfaceChanged(surface);
 }
 
+QSize QRenderSurfaceSelector::externalRenderTargetSize() const
+{
+    Q_D(const QRenderSurfaceSelector);
+    return d->externalRenderTargetSize();
+}
+
+/*!
+ * Sets render target \a size if different than underlying surface size.
+ * Tells picking the correct size.
+ */
+void QRenderSurfaceSelector::setExternalRenderTargetSize(const QSize &size)
+{
+    Q_D(QRenderSurfaceSelector);
+    d->setExternalRenderTargetSize(size);
+    emit externalRenderTargetSizeChanged(size);
+}
+
+/*! \property QRenderSurfaceSelector::window
+ *
+ * Sets \a window.
+ */
 void QRenderSurfaceSelector::setWindow(QWindow *window)
 {
     Q_D(QRenderSurfaceSelector);
@@ -106,12 +181,16 @@ void QRenderSurfaceSelector::setWindow(QWindow *window)
     emit surfaceChanged(d->m_surface);
 }
 
+/*!
+ * \internal
+ */
 void QRenderSurfaceSelector::copy(const QNode *ref)
 {
     QFrameGraphNode::copy(ref);
     const QRenderSurfaceSelector *object = static_cast<const QRenderSurfaceSelector *>(ref);
 
     d_func()->m_surface = object->d_func()->m_surface;
+    d_func()->m_externalRenderTargetSize = object->d_func()->m_externalRenderTargetSize;
 }
 
 } // namespace Qt3DRender
diff --git a/src/render/framegraph/qrendersurfaceselector.h b/src/render/framegraph/qrendersurfaceselector.h
index 165ab11ec..83d10d180 100644
--- a/src/render/framegraph/qrendersurfaceselector.h
+++ b/src/render/framegraph/qrendersurfaceselector.h
@@ -42,6 +42,7 @@
 
 #include <Qt3DRender/qframegraphnode.h>
 #include <Qt3DRender/qt3drender_global.h>
+#include <QtCore/QSize>
 
 QT_BEGIN_NAMESPACE
 
@@ -57,6 +58,7 @@ class QT3DRENDERSHARED_EXPORT QRenderSurfaceSelector : public Qt3DRender::QFrame
     Q_OBJECT
     Q_PROPERTY(QSurface *surface READ surface WRITE setSurface NOTIFY surfaceChanged)
     Q_PROPERTY(QWindow *window READ window WRITE setWindow NOTIFY windowChanged)
+    Q_PROPERTY(QSize externalRenderTargetSize READ externalRenderTargetSize NOTIFY externalRenderTargetSizeChanged)
 
 public:
     explicit QRenderSurfaceSelector(Qt3DCore::QNode *parent = nullptr);
@@ -64,6 +66,8 @@ public:
 
     QSurface *surface() const;
     QWindow *window() const;
+    QSize externalRenderTargetSize() const;
+    void setExternalRenderTargetSize(const QSize &size);
 
 public Q_SLOTS:
     void setSurface(QSurface *surface);
@@ -72,6 +76,7 @@ public Q_SLOTS:
 Q_SIGNALS:
     void surfaceChanged(QSurface *surface);
     void windowChanged(QWindow *window);
+    void externalRenderTargetSizeChanged(const QSize &size);
 
 protected:
     Q_DECLARE_PRIVATE(QRenderSurfaceSelector)
diff --git a/src/render/framegraph/qrendersurfaceselector_p.h b/src/render/framegraph/qrendersurfaceselector_p.h
index f18a376d3..136ad0991 100644
--- a/src/render/framegraph/qrendersurfaceselector_p.h
+++ b/src/render/framegraph/qrendersurfaceselector_p.h
@@ -64,7 +64,11 @@ class QRenderSurfaceSelectorPrivate : public Qt3DRender::QFrameGraphNodePrivate
 public:
     QRenderSurfaceSelectorPrivate();
 
+    void setExternalRenderTargetSize(const QSize &size);
+    QSize externalRenderTargetSize() const { return m_externalRenderTargetSize; }
+
     QSurface *m_surface;
+    QSize m_externalRenderTargetSize;
 };
 
 } // namespace Qt3DRender
diff --git a/src/render/framegraph/rendersurfaceselector.cpp b/src/render/framegraph/rendersurfaceselector.cpp
index 785054119..738e1f231 100644
--- a/src/render/framegraph/rendersurfaceselector.cpp
+++ b/src/render/framegraph/rendersurfaceselector.cpp
@@ -63,6 +63,7 @@ void RenderSurfaceSelector::updateFromPeer(Qt3DCore::QNode *peer)
     QRenderSurfaceSelector *selector = static_cast<QRenderSurfaceSelector *>(peer);
     m_surface = selector->surface();
     setEnabled(selector->isEnabled());
+    setRenderTargetSize(selector->externalRenderTargetSize());
 }
 
 void RenderSurfaceSelector::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
@@ -74,10 +75,21 @@ void RenderSurfaceSelector::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e)
             m_surface = propertyChange->value().value<QSurface *>();
         else if (propertyChange->propertyName() == QByteArrayLiteral("enabled"))
             setEnabled(propertyChange->value().toBool());
+        else if (propertyChange->propertyName() == QByteArrayLiteral("externalRenderTargetSize"))
+            setRenderTargetSize(propertyChange->value().toSize());
         markDirty(AbstractRenderer::AllDirty);
     }
 }
 
+QSize RenderSurfaceSelector::renderTargetSize() const
+{
+    if (m_renderTargetSize.isValid())
+        return m_renderTargetSize;
+    if (m_surface && m_surface->size().isValid())
+        return m_surface->size();
+    return QSize();
+}
+
 } // namespace Render
 } // namespace Qt3DRender
 
diff --git a/src/render/framegraph/rendersurfaceselector_p.h b/src/render/framegraph/rendersurfaceselector_p.h
index e76c9bb43..114a52a31 100644
--- a/src/render/framegraph/rendersurfaceselector_p.h
+++ b/src/render/framegraph/rendersurfaceselector_p.h
@@ -65,6 +65,9 @@ class RenderSurfaceSelector : public Qt3DRender::Render::FrameGraphNode
 public:
     RenderSurfaceSelector();
 
+    QSize renderTargetSize() const;
+    void setRenderTargetSize(const QSize &size) { m_renderTargetSize = size; }
+
     void updateFromPeer(Qt3DCore::QNode *peer) Q_DECL_OVERRIDE;
     void sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) Q_DECL_OVERRIDE;
 
@@ -72,6 +75,7 @@ public:
 
 private:
     QSurface *m_surface;
+    QSize m_renderTargetSize;
 };
 
 } // namespace Render
diff --git a/src/render/jobs/pickboundingvolumejob.cpp b/src/render/jobs/pickboundingvolumejob.cpp
index dcfd545aa..4907d6ec2 100644
--- a/src/render/jobs/pickboundingvolumejob.cpp
+++ b/src/render/jobs/pickboundingvolumejob.cpp
@@ -53,6 +53,7 @@
 #include <Qt3DRender/private/trianglesvisitor_p.h>
 #include <Qt3DRender/private/triangleboundingvolume_p.h>
 #include <Qt3DRender/private/qraycastingservice_p.h>
+#include <Qt3DRender/private/rendersurfaceselector_p.h>
 #include <Qt3DRender/private/qray3d_p.h>
 #include <Qt3DRender/qgeometryrenderer.h>
 #include <Qt3DCore/private/qservicelocator_p.h>
@@ -67,13 +68,14 @@ namespace Render {
 
 namespace {
 
-struct ViewportCameraPair
+struct ViewportCameraAreaTriplet
 {
     Qt3DCore::QNodeId cameraId;
     QRectF viewport;
+    QSize area;
 };
 
-class ViewportCameraGatherer
+class ViewportCameraAreaGatherer
 {
 private:
     QVector<FrameGraphNode *> m_leaves;
@@ -86,19 +88,22 @@ private:
             m_leaves.push_back(node);
     }
 
-    ViewportCameraPair gatherUpViewportCameras(Render::FrameGraphNode *node) const
+    ViewportCameraAreaTriplet gatherUpViewportCameraAreas(Render::FrameGraphNode *node) const
     {
-        ViewportCameraPair vc;
-        vc.viewport = QRectF(0.0f, 0.0f, 1.0f, 1.0f);
+        ViewportCameraAreaTriplet vca;
+        vca.viewport = QRectF(0.0f, 0.0f, 1.0f, 1.0f);
 
         while (node) {
             if (node->isEnabled()) {
                 switch (node->nodeType()) {
                 case FrameGraphNode::CameraSelector:
-                    vc.cameraId = static_cast<const CameraSelector *>(node)->cameraUuid();
+                    vca.cameraId = static_cast<const CameraSelector *>(node)->cameraUuid();
                     break;
                 case FrameGraphNode::Viewport:
-                    vc.viewport = computeViewport(vc.viewport, static_cast<const ViewportNode *>(node));
+                    vca.viewport = computeViewport(vca.viewport, static_cast<const ViewportNode *>(node));
+                    break;
+                case FrameGraphNode::Surface:
+                    vca.area = static_cast<const RenderSurfaceSelector *>(node)->renderTargetSize();
                     break;
                 default:
                     break;
@@ -106,24 +111,24 @@ private:
             }
             node = node->parent();
         }
-        return vc;
+        return vca;
     }
 
 public:
-    QVector<ViewportCameraPair> gather(FrameGraphNode *root)
+    QVector<ViewportCameraAreaTriplet> gather(FrameGraphNode *root)
     {
         // Retrieve all leaves
         visit(root);
-        QVector<ViewportCameraPair> vcPairs;
-        vcPairs.reserve(m_leaves.count());
+        QVector<ViewportCameraAreaTriplet> vcaTriplets;
+        vcaTriplets.reserve(m_leaves.count());
 
         // Find all viewport/camera pairs by traversing from leaf to root
         Q_FOREACH (Render::FrameGraphNode *leaf, m_leaves) {
-            ViewportCameraPair vcPair = gatherUpViewportCameras(leaf);
-            if (!vcPair.cameraId.isNull())
-                vcPairs.push_back(vcPair);
+            ViewportCameraAreaTriplet vcaTriplet = gatherUpViewportCameraAreas(leaf);
+            if (!vcaTriplet.cameraId.isNull())
+                vcaTriplets.push_back(vcaTriplet);
         }
-        return vcPairs;
+        return vcaTriplets;
     }
 
 };
@@ -315,21 +320,21 @@ void PickBoundingVolumeJob::run()
     if (m_mouseEvents.empty())
         return;
 
-    ViewportCameraGatherer vcGatherer;
-    const QVector<ViewportCameraPair> vcPairs = vcGatherer.gather(m_renderer->frameGraphRoot());
+    ViewportCameraAreaGatherer vcaGatherer;
+    const QVector<ViewportCameraAreaTriplet> vcaTriplets = vcaGatherer.gather(m_renderer->frameGraphRoot());
 
     EntityGatherer entitiesGatherer(m_node);
 
-    if (!vcPairs.empty()) {
+    if (!vcaTriplets.empty()) {
         Q_FOREACH (const QMouseEvent &event, m_mouseEvents) {
             m_hoveredPickersToClear = m_hoveredPickers;
             ObjectPicker *lastCurrentPicker = m_manager->objectPickerManager()->data(m_currentPicker);
 
-            Q_FOREACH (const ViewportCameraPair &vc, vcPairs) {
+            Q_FOREACH (const ViewportCameraAreaTriplet &vca, vcaTriplets) {
                 typedef CollisionGathererFunctor::result_type HitList;
                 CollisionGathererFunctor gathererFunctor;
                 gathererFunctor.m_renderer = m_renderer;
-                gathererFunctor.m_ray = rayForViewportAndCamera(event.pos(), vc.viewport, vc.cameraId);
+                gathererFunctor.m_ray = rayForViewportAndCamera(vca.area, event.pos(), vca.viewport, vca.cameraId);
                 HitList sphereHits = QtConcurrent::blockingMappedReduced<HitList>(entitiesGatherer.entities(), gathererFunctor, reduceToAllHits);
 
                 // If we have hits
@@ -461,36 +466,31 @@ void PickBoundingVolumeJob::viewMatrixForCamera(Qt3DCore::QNodeId cameraId,
     }
 }
 
-QRect PickBoundingVolumeJob::windowViewport(const QRectF &relativeViewport) const
+QRect PickBoundingVolumeJob::windowViewport(const QSize &area, const QRectF &relativeViewport) const
 {
-    // TO DO: find another way to retrieve the size since this won't work with Scene3D
-    // const QSize s = m_renderer->surfaceSize();
-    // if (s.isValid()) {
-    //     const int surfaceWidth = s.width();
-    //     const int surfaceHeight = s.height();
-    //     return QRect(relativeViewport.x() * surfaceWidth,
-    //                  (1.0 - relativeViewport.y() - relativeViewport.height()) * surfaceHeight,
-    //                  relativeViewport.width() * surfaceWidth,
-    //                  relativeViewport.height() * surfaceHeight);
-    // }
-    // return relativeViewport.toRect();
-    return QRect();
+    if (area.isValid()) {
+        const int areaWidth = area.width();
+        const int areaHeight = area.height();
+        return QRect(relativeViewport.x() * areaWidth,
+                     (1.0 - relativeViewport.y() - relativeViewport.height()) * areaHeight,
+                     relativeViewport.width() * areaWidth,
+                     relativeViewport.height() * areaHeight);
+    }
+    return relativeViewport.toRect();
 }
 
-QRay3D PickBoundingVolumeJob::rayForViewportAndCamera(const QPoint &pos,
+QRay3D PickBoundingVolumeJob::rayForViewportAndCamera(const QSize &area,
+                                                      const QPoint &pos,
                                                       const QRectF &relativeViewport,
                                                       Qt3DCore::QNodeId cameraId) const
 {
     QMatrix4x4 viewMatrix;
     QMatrix4x4 projectionMatrix;
     viewMatrixForCamera(cameraId, viewMatrix, projectionMatrix);
-    const QRect viewport = windowViewport(relativeViewport);
+    const QRect viewport = windowViewport(area, relativeViewport);
 
-    // TO DO: find another way to retrieve the size since this won't work with Scene3D
-    // const QSize s = m_renderer->surfaceSize();
-    // // In GL the y is inverted compared to Qt
-    // const QPoint glCorrectPos = s.isValid() ? QPoint(pos.x(), s.height() - pos.y()) : pos;
-    const QPoint glCorrectPos = pos;
+    // In GL the y is inverted compared to Qt
+    const QPoint glCorrectPos = QPoint(pos.x(), area.isValid() ? area.height() - pos.y() : pos.y());
     const QRay3D 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 4dd0590d1..56b25df2b 100644
--- a/src/render/jobs/pickboundingvolumejob_p.h
+++ b/src/render/jobs/pickboundingvolumejob_p.h
@@ -98,8 +98,9 @@ private:
     void viewMatrixForCamera(Qt3DCore::QNodeId cameraId,
                              QMatrix4x4 &viewMatrix,
                              QMatrix4x4 &projectionMatrix) const;
-    QRect windowViewport(const QRectF &relativeViewport) const;
-    QRay3D rayForViewportAndCamera(const QPoint &pos,
+    QRect windowViewport(const QSize &area, const QRectF &relativeViewport) const;
+    QRay3D rayForViewportAndCamera(const QSize &area,
+                                   const QPoint &pos,
                                    const QRectF &relativeViewport,
                                    Qt3DCore::QNodeId cameraId) const;
     void clearPreviouslyHoveredPickers();