##// END OF EJS Templates
Fix crash when adding/removing points during animation...
Miikka Heikkinen -
r2489:c4f9629c130d
parent child
Show More
@@ -59,15 +59,21 void XYAnimation::setup(const QVector<QPointF> &oldPoints, const QVector<QPointF
59 59
60 60 int x = m_oldPoints.count();
61 61 int y = m_newPoints.count();
62
63 if (x - y == 1 && index >= 0 && y > 0) {
62 int diff = x - y;
63 int requestedDiff = oldPoints.count() - y;
64
65 // m_oldPoints can be whatever between 0 and actual points count if new animation setup
66 // interrupts a previous animation, so only do remove and add animations if both
67 // stored diff and requested diff indicate add or remove. Also ensure that index is not
68 // invalid.
69 if (diff == 1 && requestedDiff == 1 && index >= 0 && y > 0 && index <= y) {
64 70 //remove point
65 71 m_newPoints.insert(index, index > 0 ? newPoints[index - 1] : newPoints[index]);
66 72 m_index = index;
67 73 m_type = RemovePointAnimation;
68 74 }
69 75
70 if (x - y == -1 && index >= 0) {
76 if (diff == -1 && requestedDiff == -1 && index >= 0 && index <= x) {
71 77 //add point
72 78 m_oldPoints.insert(index, index > 0 ? newPoints[index - 1] : newPoints[index]);
73 79 m_index = index;
@@ -113,6 +113,9 void LineChartItem::updateGeometry()
113 113 qreal rightMarginLine = centerPoint.x() + margin;
114 114 qreal horizontal = centerPoint.y();
115 115
116 // See ScatterChartItem::updateGeometry() for explanation why seriesLastIndex is needed
117 const int seriesLastIndex = m_series->count() - 1;
118
116 119 for (int i = 1; i < points.size(); i++) {
117 120 // Interpolating line fragments would be ugly when thick pen is used,
118 121 // so we work around it by utilizing three separate
@@ -126,7 +129,7 void LineChartItem::updateGeometry()
126 129 // degrees and both of the points are within the margin, one in the top half and one in the
127 130 // bottom half of the chart, the bottom one gets clipped incorrectly.
128 131 // However, this should be rare occurrence in any sensible chart.
129 currentSeriesPoint = m_series->pointAt(i);
132 currentSeriesPoint = m_series->pointAt(qMin(seriesLastIndex, i));
130 133 currentGeometryPoint = points.at(i);
131 134 pointOffGrid = (currentSeriesPoint.x() < minX || currentSeriesPoint.x() > maxX);
132 135
@@ -127,12 +127,20 void ScatterChartItem::updateGeometry()
127 127 QRectF clipRect(QPointF(0,0),domain()->size());
128 128
129 129 QVector<bool> offGridStatus = offGridStatusVector();
130 const int seriesLastIndex = m_series->count() - 1;
130 131
131 132 for (int i = 0; i < points.size(); i++) {
132 133 QGraphicsItem *item = items.at(i);
133 134 const QPointF &point = points.at(i);
134 135 const QRectF &rect = item->boundingRect();
135 m_markerMap[item] = m_series->pointAt(i);
136 // During remove/append animation series may have different number of points,
137 // so ensure we don't go over the index. Animation handling itself ensures that
138 // if there is actually no points in the series, then it won't generate a fake point,
139 // so we can be assured there is always at least one point in m_series here.
140 // Note that marker map values can be technically incorrect during the animation,
141 // if it was caused by an insert, but this shouldn't be a problem as the points are
142 // fake anyway.
143 m_markerMap[item] = m_series->pointAt(qMin(seriesLastIndex, i));
136 144 item->setPos(point.x() - rect.width() / 2, point.y() - rect.height() / 2);
137 145
138 146 if (!m_visible || offGridStatus.at(i))
@@ -141,6 +141,9 void SplineChartItem::updateGeometry()
141 141 qreal rightMarginLine = centerPoint.x() + margin;
142 142 qreal horizontal = centerPoint.y();
143 143
144 // See ScatterChartItem::updateGeometry() for explanation why seriesLastIndex is needed
145 const int seriesLastIndex = m_series->count() - 1;
146
144 147 for (int i = 1; i < points.size(); i++) {
145 148 // Interpolating spline fragments accurately is not trivial, and would anyway be ugly
146 149 // when thick pen is used, so we work around it by utilizing three separate
@@ -154,7 +157,7 void SplineChartItem::updateGeometry()
154 157 // degrees and both of the points are within the margin, one in the top half and one in the
155 158 // bottom half of the chart, the bottom one gets clipped incorrectly.
156 159 // However, this should be rare occurrence in any sensible chart.
157 currentSeriesPoint = m_series->pointAt(i);
160 currentSeriesPoint = m_series->pointAt(qMin(seriesLastIndex, i));
158 161 currentGeometryPoint = points.at(i);
159 162 pointOffGrid = (currentSeriesPoint.x() < minX || currentSeriesPoint.x() > maxX);
160 163
@@ -73,9 +73,13 QVector<bool> XYChart::offGridStatusVector()
73 73
74 74 QVector<bool> returnVector;
75 75 returnVector.resize(m_points.size());
76 // During remove/append animation series may have different number of points,
77 // so ensure we don't go over the index. No need to check for zero points, this
78 // will not be called in such a situation.
79 const int seriesLastIndex = m_series->count() - 1;
76 80
77 81 for (int i = 0; i < m_points.size(); i++) {
78 const QPointF &seriesPoint = m_series->pointAt(i);
82 const QPointF &seriesPoint = m_series->pointAt(qMin(seriesLastIndex, i));
79 83 if (seriesPoint.x() < minX
80 84 || seriesPoint.x() > maxX
81 85 || seriesPoint.y() < minY
@@ -84,8 +84,13 void tst_QXYSeries::seriesOpacity()
84 84 void tst_QXYSeries::append_data()
85 85 {
86 86 QTest::addColumn< QList<QPointF> >("points");
87 QTest::newRow("0,0 1,1 2,2 3,3") << (QList<QPointF>() << QPointF(0,0) << QPointF(1,1) << QPointF(2,2) << QPointF(3,3));
88 QTest::newRow("0,0 -1,-1 -2,-2 -3,-3") << (QList<QPointF>() << QPointF(0,0) << QPointF(-1,-1) << QPointF(-2,-2) << QPointF(-3,-3));
87 QTest::addColumn< QList<QPointF> >("otherPoints");
88 QTest::newRow("0,0 1,1 2,2 3,3")
89 << (QList<QPointF>() << QPointF(0,0) << QPointF(1,1) << QPointF(2,2) << QPointF(3,3))
90 << (QList<QPointF>() << QPointF(4,4) << QPointF(5,5) << QPointF(6,6) << QPointF(7,7));
91 QTest::newRow("0,0 -1,-1 -2,-2 -3,-3")
92 << (QList<QPointF>() << QPointF(0,0) << QPointF(-1,-1) << QPointF(-2,-2) << QPointF(-3,-3))
93 << (QList<QPointF>() << QPointF(-4,-4) << QPointF(-5,-5) << QPointF(-6,-6) << QPointF(-7,-7));
89 94 }
90 95
91 96
@@ -97,12 +102,19 void tst_QXYSeries::append_raw_data()
97 102 void tst_QXYSeries::append_raw()
98 103 {
99 104 QFETCH(QList<QPointF>, points);
105 QFETCH(QList<QPointF>, otherPoints);
100 106 QSignalSpy spy0(m_series, SIGNAL(clicked(QPointF)));
101 107 QSignalSpy addedSpy(m_series, SIGNAL(pointAdded(int)));
102 108 m_series->append(points);
103 109 TRY_COMPARE(spy0.count(), 0);
104 110 TRY_COMPARE(addedSpy.count(), points.count());
105 111 QCOMPARE(m_series->points(), points);
112
113 // Process events between appends
114 foreach (const QPointF &point, otherPoints) {
115 m_series->append(point);
116 QApplication::processEvents();
117 }
106 118 }
107 119
108 120 void tst_QXYSeries::chart_append_data()
@@ -196,6 +208,29 void tst_QXYSeries::remove_raw()
196 208 m_series->remove(points[i]);
197 209 }
198 210 QCOMPARE(m_series->points().count(), 0);
211
212 QApplication::processEvents();
213
214 // Process events between removes
215 m_series->append(points);
216 QCOMPARE(m_series->points(), points);
217 foreach (const QPointF &point, points) {
218 m_series->remove(point);
219 QApplication::processEvents();
220 }
221
222 // Actual meaningful delay between removes, but still shorter than animation duration
223 // (simulate e.g. spamming a hypothetical "remove last point"-button)
224 QList<QPointF> bunchOfPoints;
225 for (int i = 0; i < 10; i++)
226 bunchOfPoints.append(QPointF(i, (qreal) rand() / (qreal) RAND_MAX));
227 m_series->replace(bunchOfPoints);
228 QCOMPARE(m_series->points(), bunchOfPoints);
229 QTest::qWait(1500); // Wait for append animations to be over
230 for (int i = bunchOfPoints.count() - 1; i >= 0; i--) {
231 m_series->remove(bunchOfPoints.at(i));
232 QTest::qWait(50);
233 }
199 234 }
200 235
201 236 void tst_QXYSeries::remove_chart_data()
@@ -238,6 +273,8 void tst_QXYSeries::clear_raw()
238 273 m_series->clear();
239 274 TRY_COMPARE(spy0.count(), 0);
240 275 QCOMPARE(m_series->points().count(), 0);
276
277 QApplication::processEvents();
241 278 }
242 279
243 280 void tst_QXYSeries::clear_chart_data()
@@ -272,6 +309,7 void tst_QXYSeries::replace_raw_data()
272 309 void tst_QXYSeries::replace_raw()
273 310 {
274 311 QFETCH(QList<QPointF>, points);
312 QFETCH(QList<QPointF>, otherPoints);
275 313 QSignalSpy pointReplacedSpy(m_series, SIGNAL(pointReplaced(int)));
276 314 QSignalSpy pointsReplacedSpy(m_series, SIGNAL(pointsReplaced()));
277 315 m_series->append(points);
@@ -303,6 +341,31 void tst_QXYSeries::replace_raw()
303 341 m_series->replace(allPoints);
304 342 TRY_COMPARE(pointReplacedSpy.count(), points.count());
305 343 TRY_COMPARE(pointsReplacedSpy.count(), 1);
344
345 m_series->replace(points);
346 QApplication::processEvents();
347
348 // Process events between replaces
349 for (int i = 0; i < points.count(); ++i) {
350 m_series->replace(points.at(i), otherPoints.at(i));
351 QApplication::processEvents();
352 }
353
354 newPoints = m_series->points();
355 QCOMPARE(newPoints.count(), points.count());
356 for (int i = 0; i < points.count(); ++i) {
357 QCOMPARE(otherPoints.at(i).x(), newPoints.at(i).x());
358 QCOMPARE(otherPoints.at(i).y(), newPoints.at(i).y());
359 }
360
361 // Append followed by a replace shouldn't crash
362 m_series->clear();
363 m_series->append(QPointF(22,22));
364 m_series->append(QPointF(23,23));
365 QApplication::processEvents();
366 m_series->replace(QPointF(23,23), otherPoints.at(1));
367 QCOMPARE(m_series->points().at(1).x(), otherPoints.at(1).x());
368 QCOMPARE(m_series->points().at(1).y(), otherPoints.at(1).y());
306 369 }
307 370
308 371
General Comments 0
You need to be logged in to leave comments. Login now