diff --git a/src/animations/xyanimation.cpp b/src/animations/xyanimation.cpp index 9f66b4a..e8ad280 100644 --- a/src/animations/xyanimation.cpp +++ b/src/animations/xyanimation.cpp @@ -59,15 +59,21 @@ void XYAnimation::setup(const QVector &oldPoints, const QVector= 0 && y > 0) { + int diff = x - y; + int requestedDiff = oldPoints.count() - y; + + // m_oldPoints can be whatever between 0 and actual points count if new animation setup + // interrupts a previous animation, so only do remove and add animations if both + // stored diff and requested diff indicate add or remove. Also ensure that index is not + // invalid. + if (diff == 1 && requestedDiff == 1 && index >= 0 && y > 0 && index <= y) { //remove point m_newPoints.insert(index, index > 0 ? newPoints[index - 1] : newPoints[index]); m_index = index; m_type = RemovePointAnimation; } - if (x - y == -1 && index >= 0) { + if (diff == -1 && requestedDiff == -1 && index >= 0 && index <= x) { //add point m_oldPoints.insert(index, index > 0 ? newPoints[index - 1] : newPoints[index]); m_index = index; diff --git a/src/linechart/linechartitem.cpp b/src/linechart/linechartitem.cpp index dd5f540..ef2a350 100644 --- a/src/linechart/linechartitem.cpp +++ b/src/linechart/linechartitem.cpp @@ -113,6 +113,9 @@ void LineChartItem::updateGeometry() qreal rightMarginLine = centerPoint.x() + margin; qreal horizontal = centerPoint.y(); + // See ScatterChartItem::updateGeometry() for explanation why seriesLastIndex is needed + const int seriesLastIndex = m_series->count() - 1; + for (int i = 1; i < points.size(); i++) { // Interpolating line fragments would be ugly when thick pen is used, // so we work around it by utilizing three separate @@ -126,7 +129,7 @@ void LineChartItem::updateGeometry() // degrees and both of the points are within the margin, one in the top half and one in the // bottom half of the chart, the bottom one gets clipped incorrectly. // However, this should be rare occurrence in any sensible chart. - currentSeriesPoint = m_series->pointAt(i); + currentSeriesPoint = m_series->pointAt(qMin(seriesLastIndex, i)); currentGeometryPoint = points.at(i); pointOffGrid = (currentSeriesPoint.x() < minX || currentSeriesPoint.x() > maxX); diff --git a/src/scatterchart/scatterchartitem.cpp b/src/scatterchart/scatterchartitem.cpp index fecbeb1..6889fdc 100644 --- a/src/scatterchart/scatterchartitem.cpp +++ b/src/scatterchart/scatterchartitem.cpp @@ -127,12 +127,20 @@ void ScatterChartItem::updateGeometry() QRectF clipRect(QPointF(0,0),domain()->size()); QVector offGridStatus = offGridStatusVector(); + const int seriesLastIndex = m_series->count() - 1; for (int i = 0; i < points.size(); i++) { QGraphicsItem *item = items.at(i); const QPointF &point = points.at(i); const QRectF &rect = item->boundingRect(); - m_markerMap[item] = m_series->pointAt(i); + // During remove/append animation series may have different number of points, + // so ensure we don't go over the index. Animation handling itself ensures that + // if there is actually no points in the series, then it won't generate a fake point, + // so we can be assured there is always at least one point in m_series here. + // Note that marker map values can be technically incorrect during the animation, + // if it was caused by an insert, but this shouldn't be a problem as the points are + // fake anyway. + m_markerMap[item] = m_series->pointAt(qMin(seriesLastIndex, i)); item->setPos(point.x() - rect.width() / 2, point.y() - rect.height() / 2); if (!m_visible || offGridStatus.at(i)) diff --git a/src/splinechart/splinechartitem.cpp b/src/splinechart/splinechartitem.cpp index 4a46a72..4f1162c 100644 --- a/src/splinechart/splinechartitem.cpp +++ b/src/splinechart/splinechartitem.cpp @@ -141,6 +141,9 @@ void SplineChartItem::updateGeometry() qreal rightMarginLine = centerPoint.x() + margin; qreal horizontal = centerPoint.y(); + // See ScatterChartItem::updateGeometry() for explanation why seriesLastIndex is needed + const int seriesLastIndex = m_series->count() - 1; + for (int i = 1; i < points.size(); i++) { // Interpolating spline fragments accurately is not trivial, and would anyway be ugly // when thick pen is used, so we work around it by utilizing three separate @@ -154,7 +157,7 @@ void SplineChartItem::updateGeometry() // degrees and both of the points are within the margin, one in the top half and one in the // bottom half of the chart, the bottom one gets clipped incorrectly. // However, this should be rare occurrence in any sensible chart. - currentSeriesPoint = m_series->pointAt(i); + currentSeriesPoint = m_series->pointAt(qMin(seriesLastIndex, i)); currentGeometryPoint = points.at(i); pointOffGrid = (currentSeriesPoint.x() < minX || currentSeriesPoint.x() > maxX); diff --git a/src/xychart/xychart.cpp b/src/xychart/xychart.cpp index 3d442c5..a018119 100644 --- a/src/xychart/xychart.cpp +++ b/src/xychart/xychart.cpp @@ -73,9 +73,13 @@ QVector XYChart::offGridStatusVector() QVector returnVector; returnVector.resize(m_points.size()); + // During remove/append animation series may have different number of points, + // so ensure we don't go over the index. No need to check for zero points, this + // will not be called in such a situation. + const int seriesLastIndex = m_series->count() - 1; for (int i = 0; i < m_points.size(); i++) { - const QPointF &seriesPoint = m_series->pointAt(i); + const QPointF &seriesPoint = m_series->pointAt(qMin(seriesLastIndex, i)); if (seriesPoint.x() < minX || seriesPoint.x() > maxX || seriesPoint.y() < minY diff --git a/tests/auto/qxyseries/tst_qxyseries.cpp b/tests/auto/qxyseries/tst_qxyseries.cpp index 7988e9b..d3c15a1 100644 --- a/tests/auto/qxyseries/tst_qxyseries.cpp +++ b/tests/auto/qxyseries/tst_qxyseries.cpp @@ -84,8 +84,13 @@ void tst_QXYSeries::seriesOpacity() void tst_QXYSeries::append_data() { QTest::addColumn< QList >("points"); - QTest::newRow("0,0 1,1 2,2 3,3") << (QList() << QPointF(0,0) << QPointF(1,1) << QPointF(2,2) << QPointF(3,3)); - QTest::newRow("0,0 -1,-1 -2,-2 -3,-3") << (QList() << QPointF(0,0) << QPointF(-1,-1) << QPointF(-2,-2) << QPointF(-3,-3)); + QTest::addColumn< QList >("otherPoints"); + QTest::newRow("0,0 1,1 2,2 3,3") + << (QList() << QPointF(0,0) << QPointF(1,1) << QPointF(2,2) << QPointF(3,3)) + << (QList() << QPointF(4,4) << QPointF(5,5) << QPointF(6,6) << QPointF(7,7)); + QTest::newRow("0,0 -1,-1 -2,-2 -3,-3") + << (QList() << QPointF(0,0) << QPointF(-1,-1) << QPointF(-2,-2) << QPointF(-3,-3)) + << (QList() << QPointF(-4,-4) << QPointF(-5,-5) << QPointF(-6,-6) << QPointF(-7,-7)); } @@ -97,12 +102,19 @@ void tst_QXYSeries::append_raw_data() void tst_QXYSeries::append_raw() { QFETCH(QList, points); + QFETCH(QList, otherPoints); QSignalSpy spy0(m_series, SIGNAL(clicked(QPointF))); QSignalSpy addedSpy(m_series, SIGNAL(pointAdded(int))); m_series->append(points); TRY_COMPARE(spy0.count(), 0); TRY_COMPARE(addedSpy.count(), points.count()); QCOMPARE(m_series->points(), points); + + // Process events between appends + foreach (const QPointF &point, otherPoints) { + m_series->append(point); + QApplication::processEvents(); + } } void tst_QXYSeries::chart_append_data() @@ -196,6 +208,29 @@ void tst_QXYSeries::remove_raw() m_series->remove(points[i]); } QCOMPARE(m_series->points().count(), 0); + + QApplication::processEvents(); + + // Process events between removes + m_series->append(points); + QCOMPARE(m_series->points(), points); + foreach (const QPointF &point, points) { + m_series->remove(point); + QApplication::processEvents(); + } + + // Actual meaningful delay between removes, but still shorter than animation duration + // (simulate e.g. spamming a hypothetical "remove last point"-button) + QList bunchOfPoints; + for (int i = 0; i < 10; i++) + bunchOfPoints.append(QPointF(i, (qreal) rand() / (qreal) RAND_MAX)); + m_series->replace(bunchOfPoints); + QCOMPARE(m_series->points(), bunchOfPoints); + QTest::qWait(1500); // Wait for append animations to be over + for (int i = bunchOfPoints.count() - 1; i >= 0; i--) { + m_series->remove(bunchOfPoints.at(i)); + QTest::qWait(50); + } } void tst_QXYSeries::remove_chart_data() @@ -238,6 +273,8 @@ void tst_QXYSeries::clear_raw() m_series->clear(); TRY_COMPARE(spy0.count(), 0); QCOMPARE(m_series->points().count(), 0); + + QApplication::processEvents(); } void tst_QXYSeries::clear_chart_data() @@ -272,6 +309,7 @@ void tst_QXYSeries::replace_raw_data() void tst_QXYSeries::replace_raw() { QFETCH(QList, points); + QFETCH(QList, otherPoints); QSignalSpy pointReplacedSpy(m_series, SIGNAL(pointReplaced(int))); QSignalSpy pointsReplacedSpy(m_series, SIGNAL(pointsReplaced())); m_series->append(points); @@ -303,6 +341,31 @@ void tst_QXYSeries::replace_raw() m_series->replace(allPoints); TRY_COMPARE(pointReplacedSpy.count(), points.count()); TRY_COMPARE(pointsReplacedSpy.count(), 1); + + m_series->replace(points); + QApplication::processEvents(); + + // Process events between replaces + for (int i = 0; i < points.count(); ++i) { + m_series->replace(points.at(i), otherPoints.at(i)); + QApplication::processEvents(); + } + + newPoints = m_series->points(); + QCOMPARE(newPoints.count(), points.count()); + for (int i = 0; i < points.count(); ++i) { + QCOMPARE(otherPoints.at(i).x(), newPoints.at(i).x()); + QCOMPARE(otherPoints.at(i).y(), newPoints.at(i).y()); + } + + // Append followed by a replace shouldn't crash + m_series->clear(); + m_series->append(QPointF(22,22)); + m_series->append(QPointF(23,23)); + QApplication::processEvents(); + m_series->replace(QPointF(23,23), otherPoints.at(1)); + QCOMPARE(m_series->points().at(1).x(), otherPoints.at(1).x()); + QCOMPARE(m_series->points().at(1).y(), otherPoints.at(1).y()); }