Synchronize clicked detection properly

Implements item 2) in QTRD-2645

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

16 files changed, 106 insertions, 61 deletions

diff --git a/src/datavisualization/engine/abstract3dcontroller.cpp b/src/datavisualization/engine/abstract3dcontroller.cpp
index 7e938387..9942a2fe 100644
--- a/src/datavisualization/engine/abstract3dcontroller.cpp
+++ b/src/datavisualization/engine/abstract3dcontroller.cpp
@@ -151,6 +151,14 @@ void Abstract3DController::synchDataToRenderer()
     if (!m_renderer)
         return;
 
+    // If there is a pending click from renderer, handle that first.
+    if (m_renderer->isClickPending()) {
+        handlePendingClick();
+        m_renderer->clearClickPending();
+    }
+
+    // TODO: start recording inserts/removals
+
     if (m_scene->d_ptr->m_sceneDirty)
         m_renderer->updateScene(m_scene);
 
diff --git a/src/datavisualization/engine/abstract3dcontroller_p.h b/src/datavisualization/engine/abstract3dcontroller_p.h
index aa3235ee..7947bd7d 100644
--- a/src/datavisualization/engine/abstract3dcontroller_p.h
+++ b/src/datavisualization/engine/abstract3dcontroller_p.h
@@ -233,6 +233,7 @@ public:
             QAbstract3DAxis::AxisOrientation orientation, bool autoAdjust) = 0;
     virtual void handleAxisLabelFormatChangedBySender(QObject *sender);
     virtual void handleSeriesVisibilityChangedBySender(QObject *sender);
+    virtual void handlePendingClick() = 0;
 
 public slots:
     void handleAxisTitleChanged(const QString &title);
diff --git a/src/datavisualization/engine/abstract3drenderer.cpp b/src/datavisualization/engine/abstract3drenderer.cpp
index 6e286095..29262f41 100644
--- a/src/datavisualization/engine/abstract3drenderer.cpp
+++ b/src/datavisualization/engine/abstract3drenderer.cpp
@@ -46,11 +46,14 @@ Abstract3DRenderer::Abstract3DRenderer(Abstract3DController *controller)
       m_cachedScene(new Q3DScene()),
       m_selectionDirty(true),
       m_selectionState(SelectNone),
-      m_devicePixelRatio(1.0f)
-    #ifdef DISPLAY_RENDER_SPEED
+      m_devicePixelRatio(1.0f),
+      m_selectionLabelDirty(true),
+      m_clickPending(false),
+      m_clickedSeries(0)
+#ifdef DISPLAY_RENDER_SPEED
     , m_isFirstFrame(true),
       m_numFrames(0)
-    #endif
+#endif
 
 {
     QObject::connect(m_drawer, &Drawer::drawerChanged, this, &Abstract3DRenderer::updateTextures);
@@ -207,6 +210,7 @@ void Abstract3DRenderer::updateScene(Q3DScene *scene)
     } 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))
diff --git a/src/datavisualization/engine/abstract3drenderer_p.h b/src/datavisualization/engine/abstract3drenderer_p.h
index 944c00b1..f9dd56d8 100644
--- a/src/datavisualization/engine/abstract3drenderer_p.h
+++ b/src/datavisualization/engine/abstract3drenderer_p.h
@@ -95,6 +95,10 @@ public:
     virtual void fixMeshFileName(QString &fileName, QAbstract3DSeries::Mesh mesh);
     void fixGradientAndGenerateTexture(QLinearGradient *gradient, GLuint *gradientTexture);
 
+    inline bool isClickPending() { return m_clickPending; }
+    inline void clearClickPending() { m_clickPending = false; }
+    inline QAbstract3DSeries *clickedSeries() const { return m_clickedSeries; }
+
 signals:
     void needRender(); // Emit this if something in renderer causes need for another render pass.
     void requestShadowQuality(QAbstract3DGraph::ShadowQuality quality); // For automatic quality adjustments
@@ -138,6 +142,8 @@ protected:
     QRect m_secondarySubViewport;
     float m_devicePixelRatio;
     bool m_selectionLabelDirty;
+    bool m_clickPending;
+    QAbstract3DSeries *m_clickedSeries;
 
 #ifdef DISPLAY_RENDER_SPEED
     bool m_isFirstFrame;
diff --git a/src/datavisualization/engine/bars3dcontroller.cpp b/src/datavisualization/engine/bars3dcontroller.cpp
index a67f1adb..ae6a1c48 100644
--- a/src/datavisualization/engine/bars3dcontroller.cpp
+++ b/src/datavisualization/engine/bars3dcontroller.cpp
@@ -75,8 +75,6 @@ void Bars3DController::initializeOpenGL()
     setRenderer(m_renderer);
     synchDataToRenderer();
 
-    QObject::connect(m_renderer, &Bars3DRenderer::barClicked, this,
-                     &Bars3DController::handleBarClicked, Qt::QueuedConnection);
     emitNeedRender();
 }
 
@@ -211,14 +209,6 @@ void Bars3DController::handleDataColumnLabelsChanged()
     }
 }
 
-void Bars3DController::handleBarClicked(const QPoint &position, QBar3DSeries *series)
-{
-    setSelectedBar(position, series);
-
-    // TODO: pass clicked to parent. (QTRD-2517)
-    // TODO: Also hover needed? (QTRD-2131)
-}
-
 void Bars3DController::handleAxisAutoAdjustRangeChangedInOrientation(
         QAbstract3DAxis::AxisOrientation orientation, bool autoAdjust)
 {
@@ -238,6 +228,19 @@ void Bars3DController::handleSeriesVisibilityChangedBySender(QObject *sender)
     setSelectedBar(m_selectedBar, m_selectedBarSeries);
 }
 
+void Bars3DController::handlePendingClick()
+{
+    // This function is called while doing the sync, so it is okay to query from renderer
+    QPoint position = m_renderer->clickedPosition();
+    QBar3DSeries *series = static_cast<QBar3DSeries *>(m_renderer->clickedSeries());
+
+    // TODO: Adjust position according to inserts/removes in the series
+
+    setSelectedBar(position, series);
+
+    m_renderer->resetClickedStatus();
+}
+
 QPoint Bars3DController::invalidSelectionPosition()
 {
     static QPoint invalidSelectionPos(-1, -1);
diff --git a/src/datavisualization/engine/bars3dcontroller_p.h b/src/datavisualization/engine/bars3dcontroller_p.h
index 79d3c3ba..e0d0786f 100644
--- a/src/datavisualization/engine/bars3dcontroller_p.h
+++ b/src/datavisualization/engine/bars3dcontroller_p.h
@@ -96,6 +96,7 @@ public:
 
     virtual void handleAxisAutoAdjustRangeChangedInOrientation(QAbstract3DAxis::AxisOrientation orientation, bool autoAdjust);
     virtual void handleSeriesVisibilityChangedBySender(QObject *sender);
+    virtual void handlePendingClick();
 
     static QPoint invalidSelectionPosition();
 
@@ -121,9 +122,6 @@ public slots:
     void handleDataRowLabelsChanged();
     void handleDataColumnLabelsChanged();
 
-    // Renderer callback handlers
-    void handleBarClicked(const QPoint &position, QBar3DSeries *series);
-
 signals:
     void primarySeriesChanged(QBar3DSeries *series);
 
diff --git a/src/datavisualization/engine/bars3drenderer.cpp b/src/datavisualization/engine/bars3drenderer.cpp
index 81499ac7..96e6a82f 100644
--- a/src/datavisualization/engine/bars3drenderer.cpp
+++ b/src/datavisualization/engine/bars3drenderer.cpp
@@ -91,7 +91,8 @@ Bars3DRenderer::Bars3DRenderer(Bars3DController *controller)
       m_noZeroInRange(false),
       m_seriesScale(0.0f),
       m_seriesStep(0.0f),
-      m_seriesStart(0.0f)
+      m_seriesStart(0.0f),
+      m_clickedPosition(Bars3DController::invalidSelectionPosition())
 {
     initializeOpenGLFunctions();
     initializeOpenGL();
@@ -988,7 +989,8 @@ void Bars3DRenderer::drawScene(GLuint defaultFboHandle)
         // Read color under cursor
         QVector3D clickedColor = Utils::getSelection(m_inputPosition,
                                                      m_viewport.height());
-        emit barClicked(selectionColorToArrayPosition(clickedColor), selectionColorToSeries(clickedColor));
+        m_clickedPosition = selectionColorToArrayPosition(clickedColor);
+        m_clickedSeries = selectionColorToSeries(clickedColor);
 
         // Revert to original render target and viewport
         glBindFramebuffer(GL_FRAMEBUFFER, defaultFboHandle);
@@ -1926,6 +1928,12 @@ void Bars3DRenderer::updateSelectedBar(const QPoint &position, const QBar3DSerie
     }
 }
 
+void Bars3DRenderer::resetClickedStatus()
+{
+    m_clickedPosition = Bars3DController::invalidSelectionPosition();
+    m_clickedSeries = 0;
+}
+
 void Bars3DRenderer::updateShadowQuality(QAbstract3DGraph::ShadowQuality quality)
 {
     m_cachedShadowQuality = quality;
diff --git a/src/datavisualization/engine/bars3drenderer_p.h b/src/datavisualization/engine/bars3drenderer_p.h
index 0eb3e60a..a300aa80 100644
--- a/src/datavisualization/engine/bars3drenderer_p.h
+++ b/src/datavisualization/engine/bars3drenderer_p.h
@@ -105,6 +105,7 @@ private:
     float m_seriesScale;
     float m_seriesStep;
     float m_seriesStart;
+    QPoint m_clickedPosition;
 
 public:
     explicit Bars3DRenderer(Bars3DController *controller);
@@ -123,13 +124,12 @@ public slots:
                         bool relative = true);
     void updateSlicingActive(bool isSlicing);
     void updateSelectedBar(const QPoint &position, const QBar3DSeries *series);
+    inline QPoint clickedPosition() const { return m_clickedPosition; }
+    void resetClickedStatus();
 
     // Overloaded from abstract renderer
     virtual void updateAxisRange(QAbstract3DAxis::AxisOrientation orientation, float min, float max);
 
-signals:
-    void barClicked(QPoint position, QBar3DSeries *series);
-
 private:
     virtual void initShaders(const QString &vertexShader, const QString &fragmentShader);
     virtual void initGradientShaders(const QString &vertexShader, const QString &fragmentShader);
diff --git a/src/datavisualization/engine/scatter3dcontroller.cpp b/src/datavisualization/engine/scatter3dcontroller.cpp
index 3c577fe2..a3df9288 100644
--- a/src/datavisualization/engine/scatter3dcontroller.cpp
+++ b/src/datavisualization/engine/scatter3dcontroller.cpp
@@ -67,8 +67,6 @@ void Scatter3DController::initializeOpenGL()
     setRenderer(m_renderer);
     synchDataToRenderer();
 
-    QObject::connect(m_renderer, &Scatter3DRenderer::itemClicked, this,
-                     &Scatter3DController::handleItemClicked, Qt::QueuedConnection);
     emitNeedRender();
 }
 
@@ -193,14 +191,6 @@ void Scatter3DController::handleItemsInserted(int startIndex, int count)
     emitNeedRender();
 }
 
-void Scatter3DController::handleItemClicked(int index, QScatter3DSeries *series)
-{
-    setSelectedItem(index, series);
-
-    // TODO: pass clicked to parent. (QTRD-2517)
-    // TODO: Also hover needed? (QTRD-2131)
-}
-
 void Scatter3DController::handleAxisAutoAdjustRangeChangedInOrientation(
         QAbstract3DAxis::AxisOrientation orientation, bool autoAdjust)
 {
@@ -224,6 +214,18 @@ void Scatter3DController::handleSeriesVisibilityChangedBySender(QObject *sender)
     adjustValueAxisRange();
 }
 
+void Scatter3DController::handlePendingClick()
+{
+    int index = m_renderer->clickedIndex();
+    QScatter3DSeries *series = static_cast<QScatter3DSeries *>(m_renderer->clickedSeries());
+
+    // TODO: Adjust position according to inserts/removes in the series
+
+    setSelectedItem(index, series);
+
+    m_renderer->resetClickedStatus();
+}
+
 void Scatter3DController::setSelectionMode(QAbstract3DGraph::SelectionFlags mode)
 {
     // We only support single item selection mode and no selection mode
diff --git a/src/datavisualization/engine/scatter3dcontroller_p.h b/src/datavisualization/engine/scatter3dcontroller_p.h
index 2a37e343..6e4d391a 100644
--- a/src/datavisualization/engine/scatter3dcontroller_p.h
+++ b/src/datavisualization/engine/scatter3dcontroller_p.h
@@ -84,6 +84,7 @@ public:
     virtual void handleAxisAutoAdjustRangeChangedInOrientation(QAbstract3DAxis::AxisOrientation orientation, bool autoAdjust);
     virtual void handleAxisRangeChangedBySender(QObject *sender);
     virtual void handleSeriesVisibilityChangedBySender(QObject *sender);
+    virtual void handlePendingClick();
 
 public slots:
     void handleArrayReset();
@@ -92,9 +93,6 @@ public slots:
     void handleItemsRemoved(int startIndex, int count);
     void handleItemsInserted(int startIndex, int count);
 
-    // Renderer callback handlers
-    void handleItemClicked(int index, QScatter3DSeries *series);
-
 private:
     void adjustValueAxisRange();
 
diff --git a/src/datavisualization/engine/scatter3drenderer.cpp b/src/datavisualization/engine/scatter3drenderer.cpp
index e0348f39..2e6680aa 100644
--- a/src/datavisualization/engine/scatter3drenderer.cpp
+++ b/src/datavisualization/engine/scatter3drenderer.cpp
@@ -90,7 +90,8 @@ Scatter3DRenderer::Scatter3DRenderer(Scatter3DController *controller)
       m_dotSizeScale(1.0f),
       m_hasHeightAdjustmentChanged(true),
       m_backgroundMargin(defaultMargin),
-      m_maxItemSize(0.0f)
+      m_maxItemSize(0.0f),
+      m_clickedIndex(Scatter3DController::invalidSelectionIndex())
 {
     initializeOpenGLFunctions();
     initializeOpenGL();
@@ -235,6 +236,12 @@ void Scatter3DRenderer::updateScene(Q3DScene *scene)
     Abstract3DRenderer::updateScene(scene);
 }
 
+void Scatter3DRenderer::resetClickedStatus()
+{
+    m_clickedIndex = Scatter3DController::invalidSelectionIndex();
+    m_clickedSeries = 0;
+}
+
 void Scatter3DRenderer::render(GLuint defaultFboHandle)
 {
     // Handle GL state setup for FBO buffers and clearing of the render surface
@@ -558,10 +565,7 @@ void Scatter3DRenderer::drawScene(const GLuint defaultFboHandle)
         // Read color under cursor
         QVector3D clickedColor = Utils::getSelection(m_inputPosition,
                                                      m_viewport.height());
-        int clickedIndex = 0;
-        QScatter3DSeries *clickedSeries = 0;
-        selectionColorToSeriesAndIndex(clickedColor, clickedIndex, clickedSeries);
-        emit itemClicked(clickedIndex, clickedSeries);
+        selectionColorToSeriesAndIndex(clickedColor, m_clickedIndex, m_clickedSeries);
 
         // Revert to original fbo and viewport
         glBindFramebuffer(GL_FRAMEBUFFER, defaultFboHandle);
@@ -1778,7 +1782,7 @@ QVector3D Scatter3DRenderer::indexToSelectionColor(GLint index)
     return QVector3D(dotIdxRed, dotIdxGreen, dotIdxBlue);
 }
 
-void Scatter3DRenderer::selectionColorToSeriesAndIndex(const QVector3D &color, int &index, QScatter3DSeries *&series)
+void Scatter3DRenderer::selectionColorToSeriesAndIndex(const QVector3D &color, int &index, QAbstract3DSeries *&series)
 {
     if (color != selectionSkipColor) {
         index = int(color.x())
@@ -1787,7 +1791,7 @@ void Scatter3DRenderer::selectionColorToSeriesAndIndex(const QVector3D &color, i
         // Find the series and adjust the index accordingly
         for (int i = 0; i < m_renderingArrays.size(); i++) {
             if (index < m_renderingArrays.at(i).size()) {
-                series = static_cast<QScatter3DSeries *>(m_visibleSeriesList.at(i).series());
+                series = m_visibleSeriesList.at(i).series();
                 return; // Valid found and already set to return parameters, so we can return
             } else {
                 index -= m_renderingArrays.at(i).size();
diff --git a/src/datavisualization/engine/scatter3drenderer_p.h b/src/datavisualization/engine/scatter3drenderer_p.h
index 350aa934..d21f56a0 100644
--- a/src/datavisualization/engine/scatter3drenderer_p.h
+++ b/src/datavisualization/engine/scatter3drenderer_p.h
@@ -93,6 +93,7 @@ private:
     GLfloat m_backgroundMargin;
     GLfloat m_maxItemSize;
     QVector<float> m_cachedItemSize;
+    int m_clickedIndex;
 
 public:
     explicit Scatter3DRenderer(Scatter3DController *controller);
@@ -102,6 +103,9 @@ public:
     void updateData();
     void updateScene(Q3DScene *scene);
 
+    inline int clickedIndex() const { return m_clickedIndex; }
+    void resetClickedStatus();
+
     void render(GLuint defaultFboHandle);
 
 protected:
@@ -142,12 +146,9 @@ public slots:
 
     void updateSelectedItem(int index, const QScatter3DSeries *series);
 
-signals:
-    void itemClicked(int index, QScatter3DSeries *series);
-
 private:
     QVector3D indexToSelectionColor(GLint index);
-    void selectionColorToSeriesAndIndex(const QVector3D &color, int &index, QScatter3DSeries *&series);
+    void selectionColorToSeriesAndIndex(const QVector3D &color, int &index, QAbstract3DSeries *&series);
 };
 
 QT_END_NAMESPACE_DATAVISUALIZATION
diff --git a/src/datavisualization/engine/surface3dcontroller.cpp b/src/datavisualization/engine/surface3dcontroller.cpp
index ca546a74..c69ad5bd 100644
--- a/src/datavisualization/engine/surface3dcontroller.cpp
+++ b/src/datavisualization/engine/surface3dcontroller.cpp
@@ -70,8 +70,7 @@ void Surface3DController::initializeOpenGL()
     m_renderer = new Surface3DRenderer(this);
     setRenderer(m_renderer);
     synchDataToRenderer();
-    QObject::connect(m_renderer, &Surface3DRenderer::pointClicked, this,
-                     &Surface3DController::handlePointClicked, Qt::QueuedConnection);
+
     emitNeedRender();
 }
 
@@ -128,6 +127,19 @@ void Surface3DController::handleSeriesVisibilityChangedBySender(QObject *sender)
     setSelectedPoint(m_selectedPoint, m_selectedSeries);
 }
 
+void Surface3DController::handlePendingClick()
+{
+    // This function is called while doing the sync, so it is okay to query from renderer
+    QPoint position = m_renderer->clickedPosition();
+    QSurface3DSeries *series = static_cast<QSurface3DSeries *>(m_renderer->clickedSeries());
+
+    // TODO: Adjust position according to inserts/removes in the series
+
+    setSelectedPoint(position, series);
+
+    m_renderer->resetClickedStatus();
+}
+
 QPoint Surface3DController::invalidSelectionPosition()
 {
     static QPoint invalidSelectionPoint(-1, -1);
@@ -297,13 +309,6 @@ void Surface3DController::handleArrayReset()
     emitNeedRender();
 }
 
-void Surface3DController::handlePointClicked(const QPoint &position, QSurface3DSeries *series)
-{
-    setSelectedPoint(position, series);
-    // TODO: pass clicked to parent. (QTRD-2517)
-    // TODO: Also hover needed? (QTRD-2131)
-}
-
 void Surface3DController::handleFlatShadingSupportedChange(bool supported)
 {
     // Handle renderer flat surface support indicator signal. This happens exactly once per renderer.
diff --git a/src/datavisualization/engine/surface3dcontroller_p.h b/src/datavisualization/engine/surface3dcontroller_p.h
index d823bd85..22635b73 100644
--- a/src/datavisualization/engine/surface3dcontroller_p.h
+++ b/src/datavisualization/engine/surface3dcontroller_p.h
@@ -84,6 +84,7 @@ public:
             QAbstract3DAxis::AxisOrientation orientation, bool autoAdjust);
     virtual void handleAxisRangeChangedBySender(QObject *sender);
     virtual void handleSeriesVisibilityChangedBySender(QObject *sender);
+    virtual void handlePendingClick();
 
     static QPoint invalidSelectionPosition();
     bool isFlatShadingSupported();
@@ -100,9 +101,6 @@ public slots:
     void handleRowsInserted(int startIndex, int count);
     void handleItemChanged(int rowIndex, int columnIndex);
 
-    // Renderer callback handlers
-    void handlePointClicked(const QPoint &position, QSurface3DSeries *series);
-
     void handleFlatShadingSupportedChange(bool supported);
 
 private:
diff --git a/src/datavisualization/engine/surface3drenderer.cpp b/src/datavisualization/engine/surface3drenderer.cpp
index b0bb4834..45eb3391 100644
--- a/src/datavisualization/engine/surface3drenderer.cpp
+++ b/src/datavisualization/engine/surface3drenderer.cpp
@@ -110,7 +110,8 @@ Surface3DRenderer::Surface3DRenderer(Surface3DController *controller)
       m_hasHeightAdjustmentChanged(true),
       m_selectedPoint(Surface3DController::invalidSelectionPosition()),
       m_selectedSeries(0),
-      m_uniformGradientTexture(0)
+      m_uniformGradientTexture(0),
+      m_clickedPosition(Surface3DController::invalidSelectionPosition())
 {
     // Check if flat feature is supported
     ShaderHelper tester(this, QStringLiteral(":/shaders/vertexSurfaceFlat"),
@@ -1038,8 +1039,8 @@ void Surface3DRenderer::drawScene(GLuint defaultFboHandle)
         uint selectionId = pixel[0] + pixel[1] * 256 + pixel[2] * 65536;
 #endif
 
-        emit pointClicked(QPoint(selectionIdToSurfacePoint(selectionId)),
-                          static_cast<QSurface3DSeries *>(m_visibleSeriesList.at(0).series()));
+        m_clickedPosition = selectionIdToSurfacePoint(selectionId);
+        m_clickedSeries = m_visibleSeriesList.at(0).series();
 
         // Revert to original viewport
         glViewport(m_primarySubViewport.x(),
@@ -1862,6 +1863,12 @@ void Surface3DRenderer::updateSelectedPoint(const QPoint &position, const QSurfa
     m_selectionDirty = true;
 }
 
+void Surface3DRenderer::resetClickedStatus()
+{
+    m_clickedPosition = Surface3DController::invalidSelectionPosition();
+    m_clickedSeries = 0;
+}
+
 void Surface3DRenderer::updateSurfaceGridStatus(bool enable)
 {
     m_cachedSurfaceGridOn = enable;
diff --git a/src/datavisualization/engine/surface3drenderer_p.h b/src/datavisualization/engine/surface3drenderer_p.h
index 13fd11d8..3da50d53 100644
--- a/src/datavisualization/engine/surface3drenderer_p.h
+++ b/src/datavisualization/engine/surface3drenderer_p.h
@@ -125,6 +125,7 @@ private:
     const QSurface3DSeries *m_selectedSeries;
     GLuint m_uniformGradientTexture;
     QVector3D m_uniformGradientTextureColor;
+    QPoint m_clickedPosition;
 
 public:
     explicit Surface3DRenderer(Surface3DController *controller);
@@ -139,6 +140,8 @@ public:
     void updateSurfaceGridStatus(bool enable);
     void updateSlicingActive(bool isSlicing);
     void updateSelectedPoint(const QPoint &position, const QSurface3DSeries *series);
+    inline QPoint clickedPosition() const { return m_clickedPosition; }
+    void resetClickedStatus();
 
     void drawSlicedScene();
     void render(GLuint defaultFboHandle = 0);
@@ -147,7 +150,6 @@ protected:
     void initializeOpenGL();
 
 signals:
-    void pointClicked(QPoint position, QSurface3DSeries *series);
     void flatShadingSupportedChanged(bool supported);
 
 private: