Scatter API review: changed signal now private etc.

src/chartpresenter.cpp

@@ -217,8 +217,6 @@ void ChartPresenter::handleSeriesAdded(QSeries* series,Domain* domain)
     case QSeries::SeriesTypeScatter: {
         QScatterSeries *scatterSeries = qobject_cast<QScatterSeries *>(series);
         ScatterPresenter *scatterPresenter = new ScatterPresenter(scatterSeries, m_chart);
-        QObject::connect(scatterPresenter, SIGNAL(clicked(QPointF)),
-                         scatterSeries, SIGNAL(clicked(QPointF)));
         QObject::connect(this, SIGNAL(geometryChanged(const QRectF&)),
                          scatterPresenter, SLOT(handleGeometryChanged(const QRectF&)));
         QObject::connect(domain, SIGNAL(domainChanged(qreal,qreal,qreal,qreal)),

src/scatterseries/qscatterseries.cpp

@@ -33,6 +33,7 @@
     \value MarkerShapeDefault
     \value MarkerShapeX
     \value MarkerShapeRectangle
+    \value MarkerShapeRoundedRectangle
     \value MarkerShapeTiltedRectangle
     \value MarkerShapeTriangle
     \value MarkerShapeCircle
@@ -50,14 +51,10 @@
     data point you can use closestPoint().
 */
 
-/*!
-    \fn void QScatterSeries::changed()
-    \brief TODO
-*/
-
 QTCOMMERCIALCHART_BEGIN_NAMESPACE
 
-QScatterSeriesPrivate::QScatterSeriesPrivate() :
+QScatterSeriesPrivate::QScatterSeriesPrivate(QObject *parent) :
+    QObject(parent),
     m_data(QList<QPointF>()),
     m_markerPen(QPen(QColor::Invalid)),
     m_markerBrush(QBrush(QColor::Invalid)),
@@ -66,12 +63,19 @@ QScatterSeriesPrivate::QScatterSeriesPrivate() :
 {
 }
 
+void QScatterSeriesPrivate::emitChanged()
+{
+    emit changed();
+}
+
+#include "moc_scatterseries_p.cpp"
+
 /*!
     Constructs a series object which is a child of \a parent.
 */
 QScatterSeries::QScatterSeries(QObject *parent) :
     QSeries(parent),
-    d(new QScatterSeriesPrivate())
+    d(new QScatterSeriesPrivate(this))
 {
 }
 
@@ -89,7 +93,7 @@ QScatterSeries::~QScatterSeries()
 void QScatterSeries::add(qreal x, qreal y)
 {
     d->m_data.append(QPointF(x, y));
-    emit changed();
+    d->emitChanged();
 }
 
 /*!
@@ -98,7 +102,7 @@ void QScatterSeries::add(qreal x, qreal y)
 void QScatterSeries::add(QPointF value)
 {
     d->m_data.append(value);
-    emit changed();
+    d->emitChanged();
 }
 
 /*!
@@ -107,7 +111,7 @@ void QScatterSeries::add(QPointF value)
 void QScatterSeries::add(QList<QPointF> points)
 {
     d->m_data.append(points);
-    emit changed();
+    d->emitChanged();
 }
 
 /*!
@@ -120,7 +124,7 @@ void QScatterSeries::add(QList<QPointF> points)
 QScatterSeries& QScatterSeries::operator << (const QPointF &value)
 {
     d->m_data.append(value);
-    emit changed();
+    d->emitChanged();
     return *this;
 }
 
@@ -131,7 +135,7 @@ QScatterSeries& QScatterSeries::operator << (const QPointF &value)
 QScatterSeries& QScatterSeries::operator << (QList<QPointF> value)
 {
     d->m_data.append(value);
-    emit changed();
+    d->emitChanged();
     return *this;
 }
 
@@ -141,7 +145,7 @@ QScatterSeries& QScatterSeries::operator << (QList<QPointF> value)
 void QScatterSeries::setData(QList<QPointF> points)
 {
     d->m_data = points;
-    emit changed();
+    d->emitChanged();
 }
 
 /*!
@@ -160,7 +164,7 @@ bool QScatterSeries::replace(int index, QPointF newPoint)
 {
     if (index >= 0 && index < d->m_data.count()) {
         d->m_data.replace(index, newPoint);
-        emit changed();
+        d->emitChanged();
         return true;
     }
     return false;
@@ -174,7 +178,7 @@ bool QScatterSeries::removeAt(int index)
 {
     if (index >=0 && index < d->m_data.count()) {
         d->m_data.removeAt(index);
-        emit changed();
+        d->emitChanged();
         return true;
     }
     return false;
@@ -186,7 +190,7 @@ bool QScatterSeries::removeAt(int index)
 int QScatterSeries::removeAll(QPointF point)
 {
     int count = d->m_data.removeAll(point);
-    emit changed();
+    d->emitChanged();
     return count;
 }
 
@@ -196,7 +200,7 @@ int QScatterSeries::removeAll(QPointF point)
 void QScatterSeries::clear()
 {
     d->m_data.clear();
-    emit changed();
+    d->emitChanged();
 }
 
 /*!
@@ -219,6 +223,14 @@ int QScatterSeries::closestPoint(QPointF coordinate)
     return pointIndex;
 }
 
+/*!
+    Returns the pen used for drawing markers.
+*/
+QPen QScatterSeries::pen() const
+{
+    return d->m_markerPen;
+}
+
 /*!
     Overrides the default pen used for drawing a marker item with a user defined \a pen. The
     default pen is defined by chart theme setting.
@@ -226,17 +238,18 @@ int QScatterSeries::closestPoint(QPointF coordinate)
     \sa setBrush()
     \sa QChart::setChartTheme()
 */
-void QScatterSeries::setPen(QPen pen)
+void QScatterSeries::setPen(const QPen &pen)
 {
     d->m_markerPen = pen;
+    d->emitChanged();
 }
 
 /*!
-    Returns the pen used for drawing markers.
+    Returns the brush used for drawing markers.
 */
-QPen QScatterSeries::pen()
+QBrush QScatterSeries::brush() const
 {
-    return d->m_markerPen;
+    return d->m_markerBrush;
 }
 
 /*!
@@ -246,17 +259,18 @@ QPen QScatterSeries::pen()
     \sa setPen()
     \sa QChart::setChartTheme()
 */
-void QScatterSeries::setBrush(QBrush brush)
+void QScatterSeries::setBrush(const QBrush &brush)
 {
     d->m_markerBrush = brush;
+    d->emitChanged();
 }
 
 /*!
-    Returns the brush used for drawing markers.
+    Returns the shape used for drawing markers.
 */
-QBrush QScatterSeries::brush()
+QScatterSeries::MarkerShape QScatterSeries::shape() const
 {
-    return d->m_markerBrush;
+    return (QScatterSeries::MarkerShape) d->m_markerShape;
 }
 
 /*!
@@ -266,20 +280,13 @@ QBrush QScatterSeries::brush()
 void QScatterSeries::setShape(MarkerShape shape)
 {
     d->m_markerShape = shape;
-}
-
-/*!
-    Returns the shape used for drawing markers.
-*/
-QScatterSeries::MarkerShape QScatterSeries::shape()
-{
-    return (QScatterSeries::MarkerShape) d->m_markerShape;
+    d->emitChanged();
 }
 
 /*!
     Returns the size of the marker items.
 */
-qreal QScatterSeries::size()
+qreal QScatterSeries::size() const
 {
     return d->m_markerSize;
 }
@@ -290,7 +297,7 @@ qreal QScatterSeries::size()
 void QScatterSeries::setSize(qreal size)
 {
     d->m_markerSize = size;
-    emit changed();
+    d->emitChanged();
 }
 
 #include "moc_qscatterseries.cpp"

src/scatterseries/qscatterseries.h

@@ -19,6 +19,7 @@ public:
         MarkerShapeDefault = 0,
         MarkerShapeX,
         MarkerShapeRectangle,
+        MarkerShapeRoundedRectangle,
         MarkerShapeTiltedRectangle,
         MarkerShapeTriangle,
         MarkerShapeCircle
@@ -46,26 +47,23 @@ public:
     int closestPoint(QPointF coordinate);
     //TODO: insert, replace...?
 
-    QPen pen();
-    void setPen(QPen pen);
-    QBrush brush();
-    void setBrush(QBrush brush);
-    MarkerShape shape();
+    QPen pen() const;
+    void setPen(const QPen &pen);
+    QBrush brush() const;
+    void setBrush(const QBrush &brush);
+    MarkerShape shape() const;
     void setShape(MarkerShape shape);
-    qreal size();
+    qreal size() const;
     void setSize(qreal size);
 
 Q_SIGNALS:
     void clicked(QPointF coordinate);
-    // TODO: move to PIMPL for simplicity or does the user ever need changed signals?
-    // TODO: more finegrained signaling for performance reasons
-    // (check QPieSeries implementation with change sets)
-    void changed();
 
 private:
     Q_DECLARE_PRIVATE(QScatterSeries)
     Q_DISABLE_COPY(QScatterSeries)
-    QScatterSeriesPrivate *const d;
+    friend class ScatterPresenter;
+    QScatterSeriesPrivate *d;
 };
 
 QTCOMMERCIALCHART_END_NAMESPACE

src/scatterseries/scatterpresenter.cpp

@@ -1,5 +1,6 @@
 #include "scatterpresenter_p.h"
 #include "qscatterseries.h"
+#include "scatterseries_p.h"
 #include "chartpresenter_p.h"
 #include <QPen>
 #include <QPainter>
@@ -20,13 +21,12 @@ ScatterPresenter::ScatterPresenter(QScatterSeries *series, QGraphicsObject *pare
     m_series(series),
     m_clippingRect()
 {
-    if (parent)
-        m_clippingRect = parent->boundingRect();
-
-    if (series) {
-        connect(series, SIGNAL(changed()), this, SLOT(handleModelChanged()));
-    }
+    Q_ASSERT(parent);
+    Q_ASSERT(series);
 
+    m_clippingRect = parent->boundingRect();
+    connect(series->d, SIGNAL(changed()), this, SLOT(handleModelChanged()));
+    connect(this, SIGNAL(clicked(QPointF)), series, SIGNAL(clicked(QPointF)));
     setZValue(ChartPresenter::ScatterSeriesZValue);
 
     // TODO: how to draw a drop shadow?
@@ -136,11 +136,21 @@ void ScatterPresenter::changeGeometry()
                 case QScatterSeries::MarkerShapeRectangle:
                     m_path.addRect(x, y, size, size);
                     break;
+                case QScatterSeries::MarkerShapeRoundedRectangle:
+                    m_path.addRoundedRect(x, y, size, size, size / 4.0, size / 4.0);
+                    break;
                 case QScatterSeries::MarkerShapeTiltedRectangle: {
                     // TODO: tilt the rectangle
                     m_path.addRect(x, y, size, size);
                     break;
                 }
+                case QScatterSeries::MarkerShapeTriangle: {
+                    QPolygonF polygon;
+                    polygon << QPointF(0.0, -size) << QPointF(size / 2.0, 0.0) << QPointF(-size / 2, 0.0);
+                    // TODO: the position is not exactly right...
+                    m_path.addPolygon(polygon.translated(x + size / 2.0, y + size));
+                    break;
+                }
                 default:
                     // TODO: implement the rest of the shapes
                     Q_ASSERT(false);

src/scatterseries/scatterseries_p.h

@@ -3,6 +3,7 @@
 
 #include "qchartglobal.h"
 #include "qseries.h"
+#include <QObject>
 #include <QPen>
 
 QTCOMMERCIALCHART_BEGIN_NAMESPACE
@@ -10,10 +11,18 @@ QTCOMMERCIALCHART_BEGIN_NAMESPACE
 /*!
  * The PIMPL class of QScatterSeries.
  */
-class QScatterSeriesPrivate
+class QScatterSeriesPrivate : public QObject
 {
+    Q_OBJECT
+
 public:
-    QScatterSeriesPrivate();
+    QScatterSeriesPrivate(QObject *parent);
+    void emitChanged();
+
+Q_SIGNALS:
+    // TODO: more finegrained signaling for performance reasons
+    // (see for example QPieSeries implementation with change sets)
+    void changed();
 
 public:
     QList<QPointF> m_data;