From d850de2890197f625d7f73ef5c579e173a77e9a1 2012-04-02 13:26:14 From: Tero Ahola Date: 2012-04-02 13:26:14 Subject: [PATCH] Reserve ordering of chart series internally. This fixes for example issues with series colors being randomly messed up; an issue was most frequently seen in QML test app and on OSX after adding multiple series. --- diff --git a/src/chartdataset.cpp b/src/chartdataset.cpp index c8a029d..09f019b 100644 --- a/src/chartdataset.cpp +++ b/src/chartdataset.cpp @@ -48,9 +48,7 @@ void ChartDataSet::addSeries(QSeries* series, QChartAxis *axisY) { if(axisY==0) axisY = m_axisY; - QChartAxis* axis = m_seriesAxisMap.value(series); - - if(axis) { + if (seriesIndex(series) > -1) { qWarning() << "Can not add series. Series already on the chart"; return; } @@ -64,8 +62,7 @@ void ChartDataSet::addSeries(QSeries* series, QChartAxis *axisY) } Domain* domain = m_axisDomainMap.value(axisY); - - if(!domain) { + if (!domain) { domain = new Domain(axisY); QObject::connect(axisY,SIGNAL(changed(qreal,qreal,int,bool)),domain,SLOT(handleAxisYChanged(qreal,qreal,int,bool))); QObject::connect(axisX(),SIGNAL(changed(qreal,qreal,int,bool)),domain,SLOT(handleAxisXChanged(qreal,qreal,int,bool))); @@ -76,70 +73,75 @@ void ChartDataSet::addSeries(QSeries* series, QChartAxis *axisY) emit axisAdded(axisY,domain); } - if(!m_axisXInitialized){ - emit axisAdded(axisX(),domain); - m_axisXInitialized=true; + if (!m_axisXInitialized) { + emit axisAdded(axisX(), domain); + m_axisXInitialized = true; } - calculateDomain(series,domain); + calculateDomain(series, domain); - m_seriesAxisMap.insert(series,axisY); - emit seriesAdded(series,domain); + m_seriesAxisList.append(QPair(series, axisY)); + emit seriesAdded(series, domain); } void ChartDataSet::removeSeries(QSeries* series) { - - QChartAxis* axis = m_seriesAxisMap.value(series); - - if(!axis){ - qWarning()<<"Can not remove series. Series not found on the chart."; + int index = seriesIndex(series); + if (!index < 0) { + qWarning() << "Can not remove series. Series not found on the chart."; return; } - emit seriesRemoved(series); - m_seriesAxisMap.remove(series); - if(series->parent()==this){ + // Remove the series and the axis from the container + QChartAxis* axis = m_seriesAxisList.at(index).second; + m_seriesAxisList.removeAt(index); + + // Delete the series + emit seriesRemoved(series); + if (series->parent() == this) { delete series; - series=0; + series = 0; } - QList axes = m_seriesAxisMap.values(); - - int i = axes.indexOf(axis); + // Check if the Y axis is still in use + bool yAxisInUse(false); + for (int i(0); i < m_seriesAxisList.count(); i++) { + QPair pair = m_seriesAxisList.at(i); + if (pair.second == axis) + yAxisInUse = true; + } - if(i==-1){ + // Remove the Y axis if not in use + if (!yAxisInUse) { Domain* domain = m_axisDomainMap.take(axis); emit axisRemoved(axis); - if(axis!=axisY()){ - if(axis->parent()==this){ + if (axis != axisY()) { + // Delete the Y axis unless it is the default one + if (axis->parent() == this) { delete axis; - axis=0; + axis = 0; } } delete domain; } - if(m_seriesAxisMap.values().size()==0) - { - m_axisXInitialized=false; + // Remove the x axis in case there are no y-axes left + if (m_seriesAxisList.count() == 0) { + m_axisXInitialized = false; emit axisRemoved(axisX()); } } void ChartDataSet::removeAllSeries() { - - QList series = m_seriesAxisMap.keys(); - - foreach(QSeries* s , series) { - removeSeries(s); + while (m_seriesAxisList.count()) { + QPair pair = m_seriesAxisList.last(); + removeSeries(pair.first); } - Q_ASSERT(m_seriesAxisMap.count()==0); - Q_ASSERT(m_axisDomainMap.count()==0); - + Q_ASSERT(m_seriesAxisList.count() == 0); + Q_ASSERT(m_axisDomainMap.count() == 0); } //to be removed with PIMPL @@ -282,43 +284,30 @@ void ChartDataSet::zoomOutDomain(const QRectF& rect, const QSizeF& size) } } -int ChartDataSet::seriesCount(QSeries::QSeriesType type) -{ - int count=0; - QMapIterator i(m_seriesAxisMap); - while (i.hasNext()) { - i.next(); - if(i.key()->type()==type) count++; - } - return count; -} - -int ChartDataSet::seriesIndex(QSeries *series) +int ChartDataSet::seriesIndex(QSeries *series) const { - int count=-1; - QMapIterator i(m_seriesAxisMap); - while (i.hasNext()) { - i.next(); - count++; - if (i.key() == series) - return count; + for (int i(0); i < m_seriesAxisList.count(); i++) { + QPair pair = m_seriesAxisList.at(i); + if (pair.first == series) + return i; } - return count; + return -1; } QChartAxis* ChartDataSet::axisY(QSeries* series) const { - if(series == 0) return m_axisY; - return m_seriesAxisMap.value(series); + if (series == 0) + return m_axisY; + + return m_seriesAxisList.at(seriesIndex(series)).second; } Domain* ChartDataSet::domain(QSeries* series) const { - QChartAxis* axis = m_seriesAxisMap.value(series); - if(axis){ + QChartAxis* axis = m_seriesAxisList.at(seriesIndex(series)).second; + if (axis) return m_axisDomainMap.value(axis); - }else - return 0; + return 0; } Domain* ChartDataSet::domain(QChartAxis* axis) const diff --git a/src/chartdataset_p.h b/src/chartdataset_p.h index 8a915e4..7ef2f3d 100644 --- a/src/chartdataset_p.h +++ b/src/chartdataset_p.h @@ -54,8 +54,7 @@ public: void zoomOutDomain(const QRectF& rect, const QSizeF& size); void scrollDomain(int dx,int dy,const QSizeF& size); - int seriesCount(QSeries::QSeriesType type); - int seriesIndex(QSeries *series); + int seriesIndex(QSeries *series) const; Domain* domain(QSeries* series) const; Domain* domain(QChartAxis* axis) const; @@ -75,7 +74,7 @@ private: void setupCategories(QBarSeries* series); private: - QMap m_seriesAxisMap; + QList > m_seriesAxisList; QMap m_axisDomainMap; QChartAxis* m_axisX; QChartAxis* m_axisY; diff --git a/test/auto/chartdataset/tst_chartdataset.cpp b/test/auto/chartdataset/tst_chartdataset.cpp index 15636d8..3857379 100644 --- a/test/auto/chartdataset/tst_chartdataset.cpp +++ b/test/auto/chartdataset/tst_chartdataset.cpp @@ -52,8 +52,6 @@ private Q_SLOTS: void removeAllSeries(); void axisY_data(); void axisY(); - void seriesCount_data(); - void seriesCount(); void seriesIndex_data(); void seriesIndex(); void domain_data(); @@ -261,40 +259,6 @@ void tst_ChartDataSet::axisY() } -void tst_ChartDataSet::seriesCount_data() -{ - addSeries_data(); -} - -void tst_ChartDataSet::seriesCount() -{ - QFETCH(QLineSeries*, series0); - QFETCH(QChartAxis*, axis0); - QFETCH(QLineSeries*, series1); - QFETCH(QChartAxis*, axis1); - QFETCH(QLineSeries*, series2); - QFETCH(QChartAxis*, axis2); - QFETCH(int, axisCount); - Q_UNUSED(axisCount); - - ChartDataSet dataSet; - - dataSet.addSeries(series0, axis0); - dataSet.addSeries(series1, axis1); - dataSet.addSeries(series2, axis2); - - QSignalSpy spy0(&dataSet, SIGNAL(axisAdded(QChartAxis*,Domain*))); - QSignalSpy spy1(&dataSet, SIGNAL(axisRemoved(QChartAxis*))); - QSignalSpy spy2(&dataSet, SIGNAL(seriesAdded(QSeries*,Domain*))); - QSignalSpy spy3(&dataSet, SIGNAL(seriesRemoved(QSeries*))); - - QCOMPARE(dataSet.seriesCount(series0->type()),3); - QCOMPARE(spy0.count(), 0); - QCOMPARE(spy1.count(), 0); - QCOMPARE(spy2.count(), 0); - QCOMPARE(spy3.count(), 0); -} - void tst_ChartDataSet::seriesIndex_data() { addSeries_data(); diff --git a/test/chartwidgettest/mainwidget.cpp b/test/chartwidgettest/mainwidget.cpp index 5ea101f..479f54e 100644 --- a/test/chartwidgettest/mainwidget.cpp +++ b/test/chartwidgettest/mainwidget.cpp @@ -210,6 +210,7 @@ void MainWidget::addSeries(QString seriesName, int columnCount, int rowCount, QS for (int j(0); j < data.count(); j ++) { QList column = data.at(j); QLineSeries *series = new QLineSeries(); + series->setName("line" + QString::number(j)); for (int i(0); i < column.count(); i++) series->append(i, column.at(i)); m_chart->addSeries(series); @@ -222,12 +223,14 @@ void MainWidget::addSeries(QString seriesName, int columnCount, int rowCount, QS for (int i(0); i < column.count(); i++) lineSeries->append(i, column.at(i)); QAreaSeries *areaSeries = new QAreaSeries(lineSeries); + areaSeries->setName("area" + QString::number(j)); m_chart->addSeries(areaSeries); } } else if (seriesName == "Scatter") { for (int j(0); j < data.count(); j++) { QList column = data.at(j); QScatterSeries *series = new QScatterSeries(); + series->setName("scatter" + QString::number(j)); for (int i(0); i < column.count(); i++) series->append(i, column.at(i)); m_chart->addSeries(series);