Fix screen position based queries in threaded rendering environment

QtQuick's threaded renderer doesn't seem to always pair up sync and
render (or more accurately, beforeSynchronizing and node's
preprocess calls). Sometimes sync comes without a followup render,
or there are two syncs in a row, even though the sync is using
direct connection. Both of these cases broke the old
logic for handling position based queries.

Changed the logic to actually ensure we have resolved the query
before emitting the relevant signal.

Task-number: QTRD-3358
Change-Id: Ica6c8c311f53a06311c21532aaabc18c28556655
Reviewed-by: Tomi Korpipää <tomi.korpipaa@digia.com>

7 files changed, 46 insertions, 21 deletions

diff --git a/src/datavisualization/engine/abstract3dcontroller.cpp b/src/datavisualization/engine/abstract3dcontroller.cpp
index 27d01029..f8a1a813 100644
--- a/src/datavisualization/engine/abstract3dcontroller.cpp
+++ b/src/datavisualization/engine/abstract3dcontroller.cpp
@@ -170,16 +170,14 @@ void Abstract3DController::synchDataToRenderer()
 {
     // Subclass implementations check for renderer validity already, so no need to check here.
 
+    m_renderPending = false;
+
     // If there are pending queries, handle those first
-    if (m_renderer->isGraphPositionQueryPending()) {
+    if (m_renderer->isGraphPositionQueryResolved())
         handlePendingGraphPositionQuery();
-        m_renderer->clearGraphPositionQueryPending();
-    }
 
-    if (m_renderer->isClickPending()) {
+    if (m_renderer->isClickQueryResolved())
         handlePendingClick();
-        m_renderer->clearClickPending();
-    }
 
     startRecordingRemovesAndInserts();
 
@@ -518,8 +516,6 @@ void Abstract3DController::synchDataToRenderer()
 
 void Abstract3DController::render(const GLuint defaultFboHandle)
 {
-    m_renderPending = false;
-
     // If not initialized, do nothing.
     if (!m_renderer)
         return;
@@ -1489,12 +1485,27 @@ void Abstract3DController::handlePendingClick()
     m_selectedLabelIndex = m_renderer->m_selectedLabelIndex;
     m_selectedCustomItemIndex = m_renderer->m_selectedCustomItemIndex;
 
+    // Invalidate query position to indicate the query has been handled, unless another
+    // point has been queried.
+    if (m_renderer->cachedClickQuery() == m_scene->selectionQueryPosition())
+        m_scene->setSelectionQueryPosition(Q3DScene::invalidSelectionPoint());
+
+    m_renderer->clearClickQueryResolved();
+
     emit elementSelected(m_clickedType);
 }
 
 void Abstract3DController::handlePendingGraphPositionQuery()
 {
     m_queriedGraphPosition = m_renderer->queriedGraphPosition();
+
+    // Invalidate query position to indicate the query has been handled, unless another
+    // point has been queried.
+    if (m_renderer->cachedGraphPositionQuery() == m_scene->graphPositionQuery())
+        m_scene->setGraphPositionQuery(Q3DScene::invalidSelectionPoint());
+
+    m_renderer->clearGraphPositionQueryResolved();
+
     emit queriedGraphPositionChanged(m_queriedGraphPosition);
 }
 
diff --git a/src/datavisualization/engine/abstract3drenderer.cpp b/src/datavisualization/engine/abstract3drenderer.cpp
index a5b517fd..c47e2b29 100644
--- a/src/datavisualization/engine/abstract3drenderer.cpp
+++ b/src/datavisualization/engine/abstract3drenderer.cpp
@@ -53,8 +53,9 @@ Abstract3DRenderer::Abstract3DRenderer(Abstract3DController *controller)
       m_selectionState(SelectNone),
       m_devicePixelRatio(1.0f),
       m_selectionLabelDirty(true),
-      m_clickPending(false),
+      m_clickResolved(false),
       m_graphPositionQueryPending(false),
+      m_graphPositionQueryResolved(false),
       m_clickedSeries(0),
       m_clickedType(QAbstract3DGraph::ElementNone),
       m_selectedLabelIndex(-1),
@@ -337,10 +338,6 @@ void Abstract3DRenderer::updateScene(Q3DScene *scene)
     if (Q3DScene::invalidSelectionPoint() == logicalPixelPosition) {
         updateSelectionState(SelectNone);
     } else {
-        // Selections are one-shot, reset selection active to false before processing
-        scene->setSelectionQueryPosition(Q3DScene::invalidSelectionPoint());
-        m_clickPending = true;
-
         if (scene->isSlicingActive()) {
             if (scene->isPointInPrimarySubView(logicalPixelPosition))
                 updateSelectionState(SelectOnOverview);
@@ -353,10 +350,14 @@ void Abstract3DRenderer::updateScene(Q3DScene *scene)
         }
     }
 
-    if (Q3DScene::invalidSelectionPoint() != logicalGraphPosition) {
+    if (Q3DScene::invalidSelectionPoint() != logicalGraphPosition)
         m_graphPositionQueryPending = true;
-        scene->setGraphPositionQuery(Q3DScene::invalidSelectionPoint());
-    }
+
+    // Queue up another render when we have a query that needs resolving.
+    // This is needed because QtQuick scene graph can sometimes do a sync without following it up
+    // with a render.
+    if (m_graphPositionQueryPending || m_selectionState != SelectNone)
+        emit needRender();
 }
 
 void Abstract3DRenderer::updateTextures()
@@ -1767,6 +1768,8 @@ void Abstract3DRenderer::queriedGraphPosition(const QMatrix4x4 &projectionViewMa
     m_queriedGraphPosition = QVector3D(normalizedValues.x(),
                                        normalizedValues.y(),
                                        normalizedValues.z());
+    m_graphPositionQueryResolved = true;
+    m_graphPositionQueryPending = false;
 }
 
 void Abstract3DRenderer::calculatePolarXZ(const QVector3D &dataPos, float &x, float &z) const
diff --git a/src/datavisualization/engine/abstract3drenderer_p.h b/src/datavisualization/engine/abstract3drenderer_p.h
index 4833afaa..d8ca7696 100644
--- a/src/datavisualization/engine/abstract3drenderer_p.h
+++ b/src/datavisualization/engine/abstract3drenderer_p.h
@@ -148,13 +148,15 @@ public:
     void generateBaseColorTexture(const QColor &color, GLuint *texture);
     void fixGradientAndGenerateTexture(QLinearGradient *gradient, GLuint *gradientTexture);
 
-    inline bool isClickPending() { return m_clickPending; }
-    inline void clearClickPending() { m_clickPending = false; }
+    inline bool isClickQueryResolved() const { return m_clickResolved; }
+    inline void clearClickQueryResolved() { m_clickResolved = false; }
+    inline QPoint cachedClickQuery() const { return m_cachedScene->selectionQueryPosition(); }
     inline QAbstract3DSeries *clickedSeries() const { return m_clickedSeries; }
     inline QAbstract3DGraph::ElementType clickedType() { return m_clickedType; }
-    inline bool isGraphPositionQueryPending() { return m_graphPositionQueryPending; }
-    inline void clearGraphPositionQueryPending() { m_graphPositionQueryPending = false; }
+    inline bool isGraphPositionQueryResolved() const { return m_graphPositionQueryResolved; }
+    inline void clearGraphPositionQueryResolved() { m_graphPositionQueryResolved = false; }
     inline QVector3D queriedGraphPosition() const { return m_queriedGraphPosition; }
+    inline QPoint cachedGraphPositionQuery() const { return m_cachedScene->graphPositionQuery(); }
 
     LabelItem &selectionLabelItem();
     void setSelectionLabel(const QString &label);
@@ -253,8 +255,9 @@ protected:
     QRect m_secondarySubViewport;
     float m_devicePixelRatio;
     bool m_selectionLabelDirty;
-    bool m_clickPending;
+    bool m_clickResolved;
     bool m_graphPositionQueryPending;
+    bool m_graphPositionQueryResolved;
     QAbstract3DSeries *m_clickedSeries;
     QAbstract3DGraph::ElementType m_clickedType;
     int m_selectedLabelIndex;
diff --git a/src/datavisualization/engine/bars3drenderer.cpp b/src/datavisualization/engine/bars3drenderer.cpp
index 7e2de98f..e18e4fa5 100644
--- a/src/datavisualization/engine/bars3drenderer.cpp
+++ b/src/datavisualization/engine/bars3drenderer.cpp
@@ -1250,6 +1250,7 @@ void Bars3DRenderer::drawScene(GLuint defaultFboHandle)
         QVector4D clickedColor = Utils::getSelection(m_inputPosition, m_viewport.height());
         m_clickedPosition = selectionColorToArrayPosition(clickedColor);
         m_clickedSeries = selectionColorToSeries(clickedColor);
+        m_clickResolved = true;
 
         emit needRender();
 
diff --git a/src/datavisualization/engine/q3dscene.cpp b/src/datavisualization/engine/q3dscene.cpp
index 6ea41d6a..e4015c2b 100644
--- a/src/datavisualization/engine/q3dscene.cpp
+++ b/src/datavisualization/engine/q3dscene.cpp
@@ -560,6 +560,11 @@ void Q3DScenePrivate::sync(Q3DScenePrivate &other)
         m_changeTracker.selectionQueryPositionChanged = false;
         other.m_changeTracker.selectionQueryPositionChanged = false;
     }
+    if (m_changeTracker.graphPositionQueryPositionChanged) {
+        other.q_ptr->setGraphPositionQuery(q_ptr->graphPositionQuery());
+        m_changeTracker.graphPositionQueryPositionChanged = false;
+        other.m_changeTracker.graphPositionQueryPositionChanged = false;
+    }
     if (m_changeTracker.cameraChanged) {
         m_camera->setDirty(true);
         m_changeTracker.cameraChanged = false;
diff --git a/src/datavisualization/engine/scatter3drenderer.cpp b/src/datavisualization/engine/scatter3drenderer.cpp
index 62496538..f5d0793f 100644
--- a/src/datavisualization/engine/scatter3drenderer.cpp
+++ b/src/datavisualization/engine/scatter3drenderer.cpp
@@ -802,6 +802,7 @@ void Scatter3DRenderer::drawScene(const GLuint defaultFboHandle)
         QVector4D clickedColor = Utils::getSelection(m_inputPosition,
                                                      m_viewport.height());
         selectionColorToSeriesAndIndex(clickedColor, m_clickedIndex, m_clickedSeries);
+        m_clickResolved = true;
 
         emit needRender();
 
diff --git a/src/datavisualization/engine/surface3drenderer.cpp b/src/datavisualization/engine/surface3drenderer.cpp
index df78d491..15df5598 100644
--- a/src/datavisualization/engine/surface3drenderer.cpp
+++ b/src/datavisualization/engine/surface3drenderer.cpp
@@ -1353,6 +1353,7 @@ void Surface3DRenderer::drawScene(GLuint defaultFboHandle)
                 + uint(clickedColor.w()) * alphaMultiplier;
 
         m_clickedPosition = selectionIdToSurfacePoint(selectionId);
+        m_clickResolved = true;
 
         emit needRender();