Display selected points regardless of QXYSeries::lightMarker()

Selected points needs to be displayed regardless of the visibility
of other points. It was correctly implemented when not using light
markers concept, but it didn't work correctly with
QXYSeries::lightMarker() set to null image and valid
QXYSeries::selectedLightMarker().

The fix was to change some of the conditions in linechartitem.cpp,
splinechartitem.cpp and scatterchartitem.cpp. There was also a change
added to scatterchartitem to use QXYSeries::pointsVisible() setting.

Task-number: QTBUG-89445
Change-Id: I6010bbf6b6f670f6b0694a7612d22e353d320154
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
(cherry picked from commit 1ba0ef016e9fef13fe35235cac208d0a762b324e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

7 files changed, 121 insertions, 27 deletions

diff --git a/src/charts/linechart/linechartitem.cpp b/src/charts/linechart/linechartitem.cpp
index c2c3c694..1da16e07 100644
--- a/src/charts/linechart/linechartitem.cpp
+++ b/src/charts/linechart/linechartitem.cpp
@@ -58,6 +58,7 @@ LineChartItem::LineChartItem(QLineSeries *series, QGraphicsItem *item)
     connect(series->d_func(), &QXYSeriesPrivate::seriesUpdated,
             this, &LineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::lightMarkerChanged, this, &LineChartItem::handleSeriesUpdated);
+    connect(series, &QXYSeries::selectedLightMarkerChanged, this, &LineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::markerSizeChanged, this, &LineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::visibleChanged, this, &LineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::opacityChanged, this, &LineChartItem::handleSeriesUpdated);
@@ -322,7 +323,9 @@ void LineChartItem::updateGeometry()
 
     // For mouse interactivity, we have to add the rects *after* the 'createStroke',
     // as we don't need the outline - we need it filled up.
-    if (!m_series->lightMarker().isNull()) {
+    if (!m_series->lightMarker().isNull()
+            || (!m_series->selectedLightMarker().isNull()
+                && !m_series->selectedPoints().isEmpty())) {
         // +1, +2: a margin to guarantee we cover all of the pixmap
         qreal markerHalfSize = (m_markerSize / 2.0) + 1;
         qreal markerSize = m_markerSize + 2;
@@ -449,8 +452,10 @@ void LineChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *opt
 
     int pointLabelsOffset = m_linePen.width() / 2;
 
-    // Draw markers if a marker has been set (set to QImage() to disable)
-    if (!m_series->lightMarker().isNull()) {
+    // Draw markers if a marker or marker for selected points only has been
+    // set (set to QImage() to disable)
+    if (!m_series->lightMarker().isNull() || (!m_series->selectedLightMarker().isNull()
+                                              && !m_series->selectedPoints().isEmpty())) {
         const QImage &marker = m_series->lightMarker();
         const QImage &selectedMarker = m_series->selectedLightMarker();
         qreal markerHalfSize = m_markerSize / 2.0;
@@ -461,7 +466,7 @@ void LineChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *opt
             // light markers are independent features. Therefore m_pointsVisible
             // is not used here as light markers are drawn if lightMarker is not null.
             // However points visibility configuration can be still used here.
-            bool drawPoint = true;
+            bool drawPoint = !m_series->lightMarker().isNull();
             if (m_pointsConfiguration.contains(i)) {
                 const auto &conf = m_pointsConfiguration[i];
 
diff --git a/src/charts/scatterchart/qscatterseries.cpp b/src/charts/scatterchart/qscatterseries.cpp
index 1467a839..44ab2a49 100644
--- a/src/charts/scatterchart/qscatterseries.cpp
+++ b/src/charts/scatterchart/qscatterseries.cpp
@@ -206,6 +206,8 @@ QT_BEGIN_NAMESPACE
 QScatterSeries::QScatterSeries(QObject *parent)
     : QXYSeries(*new QScatterSeriesPrivate(this), parent)
 {
+    setPointsVisible(true);
+
     // Emit QScatterSeries' markerSizeChanged signal as it's not the same as
     // QXYSeries' markerSizeChanged
     connect(this, &QXYSeries::markerSizeChanged, this, &QScatterSeries::markerSizeChanged);
diff --git a/src/charts/scatterchart/scatterchartitem.cpp b/src/charts/scatterchart/scatterchartitem.cpp
index 3c9c8cfb..15201e7e 100644
--- a/src/charts/scatterchart/scatterchartitem.cpp
+++ b/src/charts/scatterchart/scatterchartitem.cpp
@@ -50,17 +50,20 @@ ScatterChartItem::ScatterChartItem(QScatterSeries *series, QGraphicsItem *item)
       m_items(this),
       m_visible(true),
       m_markerShape(QScatterSeries::MarkerShapeRectangle),
+      m_pointsVisible(true),
       m_pointLabelsVisible(false),
       m_markerSize(series->markerSize()),
       m_pointLabelsFormat(series->pointLabelsFormat()),
       m_pointLabelsFont(series->pointLabelsFont()),
       m_pointLabelsColor(series->pointLabelsColor()),
       m_pointLabelsClipping(true),
+      m_lastHoveredPoint(QPointF(qQNaN(), qQNaN())),
       m_mousePressed(false)
 {
     connect(series->d_func(), &QXYSeriesPrivate::seriesUpdated,
             this, &ScatterChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::lightMarkerChanged, this, &ScatterChartItem::handleSeriesUpdated);
+    connect(series, &QXYSeries::selectedLightMarkerChanged, this, &ScatterChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::markerSizeChanged, this, &ScatterChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::visibleChanged, this, &ScatterChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::opacityChanged, this, &ScatterChartItem::handleSeriesUpdated);
@@ -81,8 +84,9 @@ ScatterChartItem::ScatterChartItem(QScatterSeries *series, QGraphicsItem *item)
     connect(series, &QScatterSeries::pointsConfigurationChanged,
             this, &ScatterChartItem::handleSeriesUpdated);
 
+    setAcceptHoverEvents(true);
     setZValue(ChartPresenter::ScatterSeriesZValue);
-    setFlags(QGraphicsItem::ItemClipsChildrenToShape);
+    setFlags(QGraphicsItem::ItemClipsChildrenToShape | QGraphicsItem::ItemIsSelectable);
 
     handleSeriesUpdated();
 
@@ -281,7 +285,7 @@ void ScatterChartItem::updateGeometry()
             if (!m_visible || offGridStatus.at(i)) {
                 item->setVisible(false);
             } else {
-                bool drawPoint = true;
+                bool drawPoint = m_pointsVisible;
                 if (m_pointsConfiguration.contains(i)) {
                     const auto &conf = m_pointsConfiguration[i];
 
@@ -298,6 +302,9 @@ void ScatterChartItem::updateGeometry()
                     }
                 }
 
+                if (m_series->isPointSelected(i))
+                    drawPoint = m_series->selectedLightMarker().isNull();
+
                 item->setVisible(drawPoint);
             }
         }
@@ -307,6 +314,62 @@ void ScatterChartItem::updateGeometry()
     }
 }
 
+void ScatterChartItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
+{
+    QPointF matchedP = matchForLightMarker(event->pos());
+    if (!qIsNaN(matchedP.x())) {
+        emit XYChart::pressed(matchedP);
+        m_lastMousePos = event->pos();
+        m_mousePressed = true;
+    }
+
+    QGraphicsItem::mousePressEvent(event);
+}
+
+void ScatterChartItem::hoverMoveEvent(QGraphicsSceneHoverEvent *event)
+{
+    QPointF matchedP = matchForLightMarker(event->pos());
+    if (!qIsNaN(matchedP.x())) {
+        if (matchedP != m_lastHoveredPoint) {
+            if (!qIsNaN(m_lastHoveredPoint.x()))
+                emit XYChart::hovered(m_lastHoveredPoint, false);
+
+            m_lastHoveredPoint = matchedP;
+            emit XYChart::hovered(matchedP, true);
+        }
+    } else if (!qIsNaN(m_lastHoveredPoint.x())) {
+        emit XYChart::hovered(m_lastHoveredPoint, false);
+        m_lastHoveredPoint = QPointF(qQNaN(), qQNaN());
+    }
+
+    QGraphicsItem::hoverMoveEvent(event);
+}
+
+void ScatterChartItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
+{
+    QPointF result;
+    QPointF matchedP = matchForLightMarker(m_lastMousePos);
+    if (!qIsNaN(matchedP.x()) && m_mousePressed) {
+        result = matchedP;
+        emit XYChart::released(result);
+        emit XYChart::clicked(result);
+    }
+
+    m_mousePressed = false;
+    QGraphicsItem::mouseReleaseEvent(event);
+}
+
+void ScatterChartItem::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event)
+{
+    QPointF matchedP = matchForLightMarker(event->pos());
+    if (!qIsNaN(matchedP.x()))
+        emit XYChart::doubleClicked(matchedP);
+    else
+        emit XYChart::doubleClicked(domain()->calculateDomainPoint(m_lastMousePos));
+
+    QGraphicsItem::mouseDoubleClickEvent(event);
+}
+
 void ScatterChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
 {
     Q_UNUSED(option);
@@ -315,8 +378,14 @@ void ScatterChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *
     if (m_series->useOpenGL())
         return;
 
-    // Draw markers if a marker has been set (set to QImage() to disable)
-    if (!m_series->lightMarker().isNull()) {
+    QRectF clipRect = QRectF(QPointF(0, 0), domain()->size());
+
+    painter->save();
+    painter->setClipRect(clipRect);
+
+    // Draw markers if a marker or marker for selected points only has been
+    // set (set to QImage() to disable)
+    if (!m_series->lightMarker().isNull() || !m_series->selectedLightMarker().isNull()) {
         const QImage &marker = m_series->lightMarker();
         const QImage &selectedMarker = m_series->selectedLightMarker();
         qreal markerHalfSize = m_markerSize / 2.0;
@@ -326,7 +395,7 @@ void ScatterChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *
             // light markers are independent features. Therefore m_pointsVisible
             // is not used here as light markers are drawn if lightMarker is not null.
             // However points visibility configuration can be still used here.
-            bool drawPoint = true;
+            bool drawPoint = !m_series->lightMarker().isNull();
             if (m_pointsConfiguration.contains(i)) {
                 const auto &conf = m_pointsConfiguration[i];
 
@@ -350,11 +419,6 @@ void ScatterChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *
         }
     }
 
-    QRectF clipRect = QRectF(QPointF(0, 0), domain()->size());
-
-    painter->save();
-    painter->setClipRect(clipRect);
-
     if (m_series->bestFitLineVisible())
         m_series->d_func()->drawBestFitLine(painter, clipRect);
 
@@ -422,6 +486,7 @@ void ScatterChartItem::handleSeriesUpdated()
     m_pointsConfigurationDirty = m_series->pointsConfiguration() != m_pointsConfiguration;
 
     bool recreate = m_visible != m_series->isVisible()
+                    || m_pointsVisible != m_series->pointsVisible()
                     || m_markerSize != m_series->markerSize()
                     || m_markerShape != m_series->markerShape()
                     || m_selectedColor != m_series->selectedColor()
@@ -432,6 +497,7 @@ void ScatterChartItem::handleSeriesUpdated()
     m_markerShape = m_series->markerShape();
     setVisible(m_visible);
     setOpacity(m_series->opacity());
+    m_pointsVisible = m_series->pointsVisible();
     m_pointLabelsFormat = m_series->pointLabelsFormat();
     m_pointLabelsVisible = m_series->pointLabelsVisible();
     m_pointLabelsFont = m_series->pointLabelsFont();
diff --git a/src/charts/scatterchart/scatterchartitem_p.h b/src/charts/scatterchart/scatterchartitem_p.h
index 1770d043..93969173 100644
--- a/src/charts/scatterchart/scatterchartitem_p.h
+++ b/src/charts/scatterchart/scatterchartitem_p.h
@@ -85,6 +85,10 @@ private:
 
 protected:
     void updateGeometry() override;
+    void mousePressEvent(QGraphicsSceneMouseEvent *event) override;
+    void hoverMoveEvent(QGraphicsSceneHoverEvent *event) override;
+    void mouseReleaseEvent(QGraphicsSceneMouseEvent *event) override;
+    void mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event) override;
 
 private:
     QScatterSeries *m_series;
@@ -94,6 +98,7 @@ private:
     QRectF m_rect;
     QMap<QGraphicsItem *, QPointF> m_markerMap;
 
+    bool m_pointsVisible;
     bool m_pointLabelsVisible;
     qreal m_markerSize;
     QString m_pointLabelsFormat;
@@ -101,6 +106,8 @@ private:
     QColor m_pointLabelsColor;
     bool m_pointLabelsClipping;
 
+    QPointF m_lastHoveredPoint;
+    QPointF m_lastMousePos;
     bool m_mousePressed;
 };
 
diff --git a/src/charts/splinechart/splinechartitem.cpp b/src/charts/splinechart/splinechartitem.cpp
index 7e0601e4..8f39e211 100644
--- a/src/charts/splinechart/splinechartitem.cpp
+++ b/src/charts/splinechart/splinechartitem.cpp
@@ -56,6 +56,7 @@ SplineChartItem::SplineChartItem(QSplineSeries *series, QGraphicsItem *item)
     connect(m_series->d_func(), &QXYSeriesPrivate::seriesUpdated,
             this, &SplineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::lightMarkerChanged, this, &SplineChartItem::handleSeriesUpdated);
+    connect(series, &QXYSeries::selectedLightMarkerChanged, this, &SplineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::markerSizeChanged, this, &SplineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::visibleChanged, this, &SplineChartItem::handleSeriesUpdated);
     connect(series, &QXYSeries::opacityChanged, this, &SplineChartItem::handleSeriesUpdated);
@@ -316,7 +317,8 @@ void SplineChartItem::updateGeometry()
 
     // For mouse interactivity, we have to add the rects *after* the 'createStroke',
     // as we don't need the outline - we need it filled up.
-    if (!m_series->lightMarker().isNull()) {
+    if (!m_series->lightMarker().isNull() || (!m_series->selectedLightMarker().isNull()
+                                                 && !m_series->selectedPoints().isEmpty())) {
         // +1, +2: a margin to guarantee we cover all of the pixmap
         qreal markerHalfSize = (m_series->markerSize() / 2.0) + 1;
         qreal markerSize = m_series->markerSize() + 2;
@@ -505,8 +507,10 @@ void SplineChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *o
 
     int pointLabelsOffset = m_linePen.width() / 2;
 
-    // Draw markers if a marker has been set (set to QImage() to disable)
-    if (!m_series->lightMarker().isNull()) {
+    // Draw markers if a marker or marker for selected points only has been
+    // set (set to QImage() to disable)
+    if (!m_series->lightMarker().isNull() || (!m_series->selectedLightMarker().isNull()
+                                           && !m_series->selectedPoints().isEmpty())) {
         const QImage &marker = m_series->lightMarker();
         const QImage &selectedMarker = m_series->selectedLightMarker();
         qreal markerHalfSize = m_markerSize / 2.0;
@@ -517,7 +521,7 @@ void SplineChartItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *o
             // light markers are independent features. Therefore m_pointsVisible
             // is not used here as light markers are drawn if lightMarker is not null.
             // However points visibility configuration can be still used here.
-            bool drawPoint = true;
+            bool drawPoint = !m_series->lightMarker().isNull();
             if (m_pointsConfiguration.contains(i)) {
                 const auto &conf = m_pointsConfiguration[i];
 
diff --git a/src/charts/xychart/xychart.cpp b/src/charts/xychart/xychart.cpp
index 7eda3819..f09c2d66 100644
--- a/src/charts/xychart/xychart.cpp
+++ b/src/charts/xychart/xychart.cpp
@@ -258,13 +258,23 @@ bool XYChart::isEmpty()
 
 QPointF XYChart::matchForLightMarker(const QPointF &eventPos)
 {
-    if (m_series->lightMarker().isNull())
+    if (m_series->lightMarker().isNull()
+            && (m_series->selectedLightMarker().isNull()
+                || m_series->selectedPoints().isEmpty()))
         return QPointF(qQNaN(), qQNaN()); // 0,0 could actually be in points()
 
-    int markerWidth =  m_series->lightMarker().width();
-    int markerHeight =  m_series->lightMarker().height();
+    const bool useSelectedMarker = m_series->lightMarker().isNull();
 
-    for (const QPointF &dp : m_series->points()) {
+    QList<QPointF> points;
+    if (useSelectedMarker) {
+        const auto selectedPoints = m_series->selectedPoints();
+        for (const int &selectedPointIndex : selectedPoints)
+            points << m_series->at(selectedPointIndex);
+    } else {
+        points = m_series->points();
+    }
+
+    for (const QPointF &dp : points) {
         bool ok;
         const QPointF gp = domain()->calculateGeometryPoint(dp, ok);
         if (ok) {
@@ -274,9 +284,9 @@ QPointF XYChart::matchForLightMarker(const QPointF &eventPos)
             // but as there is a bunch of 'translations' and therefore inaccuracies,
             // so it is necessary to increase that margin to 2
             // (otherwise you can click next to an icon, get a click event but not match it)
-            QRectF r(gp.x() - (markerWidth / 2 + 2),
-                     gp.y() - (markerHeight / 2 + 2),
-                     markerWidth + 4, markerHeight + 4);
+            QRectF r(gp.x() - (m_series->markerSize() / 2 + 2),
+                     gp.y() - (m_series->markerSize() / 2 + 2),
+                     m_series->markerSize() + 4, m_series->markerSize() + 4);
 
             if (r.contains(eventPos))
                 return dp;
diff --git a/tests/auto/qscatterseries/tst_qscatterseries.cpp b/tests/auto/qscatterseries/tst_qscatterseries.cpp
index e91a881b..8d369660 100644
--- a/tests/auto/qscatterseries/tst_qscatterseries.cpp
+++ b/tests/auto/qscatterseries/tst_qscatterseries.cpp
@@ -87,7 +87,7 @@ void tst_QScatterSeries::qscatterseries()
     QCOMPARE(series.brush(), QBrush());
     QCOMPARE(series.points(), QList<QPointF>());
     QCOMPARE(series.pen(), QPen());
-    QCOMPARE(series.pointsVisible(), false);
+    QCOMPARE(series.pointsVisible(), true);
 
     series.append(QList<QPointF>());
     series.append(0.0,0.0);
@@ -102,7 +102,7 @@ void tst_QScatterSeries::qscatterseries()
     series.setBrush(QBrush());
 
     series.setPen(QPen());
-    series.setPointsVisible(false);
+    series.setPointsVisible(true);
 
     m_chart->addSeries(&series);
     m_view->show();