HG_REPOSITORIES/LPP/INSTRUMENTATION/USERS/JEANDET/qtcharts Commit - r2489:c4f9629c130d

Fix crash when adding/removing points during animation...

Miikka Heikkinen -

r2489:c4f9629c130d

parent child

Context file:

r2489:c4f9629c130d

Collapse all files

src/animations/xyanimation.cpp +9 -3

                 int x = m_oldPoints.count();
                 int y = m_newPoints.count();
+                int diff = x - y;
-                if (x - y == 1  && index >= 0 && y > 0) {
+                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;

src/linechart/linechartitem.cpp +4 -1

                     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
                         // 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);

src/scatterchart/scatterchartitem.cpp +9 -1

                 QRectF clipRect(QPointF(0,0),domain()->size());
                 QVector<bool> 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))

src/splinechart/splinechartitem.cpp +4 -1

                     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
                         // 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);

src/xychart/xychart.cpp +5 -1

                 QVector<bool> 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

tests/auto/qxyseries/tst_qxyseries.cpp +65 -2

@@ -84,8 +84,13 void tst_QXYSeries::seriesOpacity()
84	void tst_QXYSeries::append_data()	84	void tst_QXYSeries::append_data()
85	{	85	{
86	QTest::addColumn< QList<QPointF> >("points");	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));	87	QTest::addColumn< QList<QPointF> >("otherPoints");
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));	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	void tst_QXYSeries::append_raw()	102	void tst_QXYSeries::append_raw()
98	{	103	{
99	QFETCH(QList<QPointF>, points);	104	QFETCH(QList<QPointF>, points);
		105	QFETCH(QList<QPointF>, otherPoints);
100	QSignalSpy spy0(m_series, SIGNAL(clicked(QPointF)));	106	QSignalSpy spy0(m_series, SIGNAL(clicked(QPointF)));
101	QSignalSpy addedSpy(m_series, SIGNAL(pointAdded(int)));	107	QSignalSpy addedSpy(m_series, SIGNAL(pointAdded(int)));
102	m_series->append(points);	108	m_series->append(points);
103	TRY_COMPARE(spy0.count(), 0);	109	TRY_COMPARE(spy0.count(), 0);
104	TRY_COMPARE(addedSpy.count(), points.count());	110	TRY_COMPARE(addedSpy.count(), points.count());
105	QCOMPARE(m_series->points(), points);	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	void tst_QXYSeries::chart_append_data()	120	void tst_QXYSeries::chart_append_data()
@@ -196,6 +208,29 void tst_QXYSeries::remove_raw()
196	m_series->remove(points[i]);	208	m_series->remove(points[i]);
197	}	209	}
198	QCOMPARE(m_series->points().count(), 0);	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	void tst_QXYSeries::remove_chart_data()	236	void tst_QXYSeries::remove_chart_data()
@@ -238,6 +273,8 void tst_QXYSeries::clear_raw()
238	m_series->clear();	273	m_series->clear();
239	TRY_COMPARE(spy0.count(), 0);	274	TRY_COMPARE(spy0.count(), 0);
240	QCOMPARE(m_series->points().count(), 0);	275	QCOMPARE(m_series->points().count(), 0);
		276
		277	QApplication::processEvents();
241	}	278	}
242		279
243	void tst_QXYSeries::clear_chart_data()	280	void tst_QXYSeries::clear_chart_data()
@@ -272,6 +309,7 void tst_QXYSeries::replace_raw_data()
272	void tst_QXYSeries::replace_raw()	309	void tst_QXYSeries::replace_raw()
273	{	310	{
274	QFETCH(QList<QPointF>, points);	311	QFETCH(QList<QPointF>, points);
		312	QFETCH(QList<QPointF>, otherPoints);
275	QSignalSpy pointReplacedSpy(m_series, SIGNAL(pointReplaced(int)));	313	QSignalSpy pointReplacedSpy(m_series, SIGNAL(pointReplaced(int)));
276	QSignalSpy pointsReplacedSpy(m_series, SIGNAL(pointsReplaced()));	314	QSignalSpy pointsReplacedSpy(m_series, SIGNAL(pointsReplaced()));
277	m_series->append(points);	315	m_series->append(points);
@@ -303,6 +341,31 void tst_QXYSeries::replace_raw()
303	m_series->replace(allPoints);	341	m_series->replace(allPoints);
304	TRY_COMPARE(pointReplacedSpy.count(), points.count());	342	TRY_COMPARE(pointReplacedSpy.count(), points.count());
305	TRY_COMPARE(pointsReplacedSpy.count(), 1);	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

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages