statistics: refactor QSG memory management

The code was wrong, because it deleted the ChartItems in the
main UI thread, not the render thread. This would delete the
QSG nodes in the UI thread and then crash on mobile.

Therefore refactor this part of the code by adding the
items to be deleted to a list that will be deleted by the
render thread.

As a drop in replacement of std::unique_ptr, implement
a silly ChartItemPtr class, which auto-initializes to null.

This turns the deterministic and easily controlled memory
management into a steaming pile of insanity. Obviously,
this can be made much more elegant, but this has to do for now.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

stats/barseries.cpp

@@ -382,8 +382,9 @@ bool BarSeries::hover(QPointF pos)
 		if (!information)
 			information = view.createChartItem<InformationBox>();
 		information->setText(makeInfo(item, highlighted.subitem), pos);
+		information->setVisible(true);
 	} else {
-		information.reset();
+		information->setVisible(false);
 	}
 
 	return highlighted.bar >= 0;

stats/barseries.h

@@ -5,6 +5,7 @@
 #ifndef BAR_SERIES_H
 #define BAR_SERIES_H
 
+#include "statshelper.h"
 #include "statsseries.h"
 #include "statsvariables.h"
 
@@ -81,7 +82,7 @@ private:
 
 	// A label that is composed of multiple lines
 	struct BarLabel {
-		std::unique_ptr<ChartTextItem> item;
+		ChartItemPtr<ChartTextItem> item;
 		bool isOutside; // Is shown outside of bar
 		BarLabel(StatsView &view, const std::vector<QString> &labels, int bin_nr, int binCount);
 		void setVisible(bool visible);
@@ -90,7 +91,7 @@ private:
 	};
 
 	struct SubItem {
-		std::unique_ptr<ChartBarItem> item;
+		ChartItemPtr<ChartBarItem> item;
 		std::unique_ptr<BarLabel> label;
 		double value_from;
 		double value_to;
@@ -116,7 +117,7 @@ private:
 		int getSubItemUnderMouse(const QPointF &f, bool horizontal, bool stacked) const;
 	};
 
-	std::unique_ptr<InformationBox> information;
+	ChartItemPtr<InformationBox> information;
 	std::vector<Item> items;
 	bool horizontal;
 	bool stacked;

stats/boxseries.cpp

@@ -128,8 +128,9 @@ bool BoxSeries::hover(QPointF pos)
 		if (!information)
 			information = view.createChartItem<InformationBox>();
 		information->setText(formatInformation(item), pos);
+		information->setVisible(true);
 	} else {
-		information.reset();
+		information->setVisible(false);
 	}
 	return highlighted >= 0;
 }

stats/boxseries.h

@@ -34,7 +34,7 @@ private:
 	int getItemUnderMouse(const QPointF &f);
 
 	struct Item {
-		std::unique_ptr<ChartBoxItem> item;
+		ChartItemPtr<ChartBoxItem> item;
 		double lowerBound, upperBound;
 		StatsQuartiles q;
 		QString binName;
@@ -48,7 +48,7 @@ private:
 	int decimals;
 
 	std::vector<QString> formatInformation(const Item &item) const;
-	std::unique_ptr<InformationBox> information;
+	ChartItemPtr<InformationBox> information;
 	std::vector<std::unique_ptr<Item>> items;
 	int highlighted; // -1: no item highlighted
 };

stats/chartitem.cpp

@@ -16,15 +16,16 @@ static int round_up(double f)
 }
 
 ChartItem::ChartItem(StatsView &v, ChartZValue z) :
-	dirty(false), dirtyPrev(nullptr), dirtyNext(nullptr),
+	dirty(false), prev(nullptr), next(nullptr),
 	zValue(z), view(v)
 {
+	// Register before the derived constructors run, so that the
+	// derived classes can mark the item as dirty in the constructor.
+	v.registerChartItem(*this);
 }
 
 ChartItem::~ChartItem()
 {
-	if (dirty)
-		view.unregisterDirtyChartItem(*this);
 }
 
 QSizeF ChartItem::sceneSize() const
@@ -32,6 +33,11 @@ QSizeF ChartItem::sceneSize() const
 	return view.size();
 }
 
+void ChartItem::markDirty()
+{
+	view.registerDirtyChartItem(*this);
+}
+
 ChartPixmapItem::ChartPixmapItem(StatsView &v, ChartZValue z) : HideableChartItem(v, z),
 	positionDirty(false), textureDirty(false)
 {
@@ -45,13 +51,13 @@ ChartPixmapItem::~ChartPixmapItem()
 void ChartPixmapItem::setTextureDirty()
 {
 	textureDirty = true;
-	view.registerDirtyChartItem(*this);
+	markDirty();
 }
 
 void ChartPixmapItem::setPositionDirty()
 {
 	positionDirty = true;
-	view.registerDirtyChartItem(*this);
+	markDirty();
 }
 
 void ChartPixmapItem::render()
@@ -60,6 +66,8 @@ void ChartPixmapItem::render()
 		createNode(view.w()->createImageNode());
 		view.addQSGNode(node.get(), zValue);
 	}
+	updateVisible();
+
 	if (!img) {
 		resize(QSizeF(1,1));
 		img->fill(Qt::transparent);
@@ -141,6 +149,7 @@ void ChartScatterItem::render()
 		view.addQSGNode(node.get(), zValue);
 		textureDirty = positionDirty = true;
 	}
+	updateVisible();
 	if (textureDirty) {
 		node->node->setTexture(highlighted ? scatterItemHighlightedTexture.get() : scatterItemTexture.get());
 		textureDirty = false;
@@ -156,7 +165,7 @@ void ChartScatterItem::setPos(QPointF pos)
 	pos -= QPointF(scatterItemDiameter / 2.0, scatterItemDiameter / 2.0);
 	rect.moveTopLeft(pos);
 	positionDirty = true;
-	view.registerDirtyChartItem(*this);
+	markDirty();
 }
 
 static double squareDist(const QPointF &p1, const QPointF &p2)
@@ -176,7 +185,7 @@ void ChartScatterItem::setHighlight(bool highlightedIn)
 		return;
 	highlighted = highlightedIn;
 	textureDirty = true;
-	view.registerDirtyChartItem(*this);
+	markDirty();
 }
 
 QRectF ChartScatterItem::getRect() const
@@ -297,6 +306,7 @@ void ChartLineItem::render()
 		view.addQSGNode(node.get(), zValue);
 		positionDirty = materialDirty = true;
 	}
+	updateVisible();
 
 	if (positionDirty) {
 		// Attention: width is a geometry property and therefore handled by position dirty!
@@ -320,7 +330,7 @@ void ChartLineItem::setLine(QPointF fromIn, QPointF toIn)
 	from = fromIn;
 	to = toIn;
 	positionDirty = true;
-	view.registerDirtyChartItem(*this);
+	markDirty();
 }
 
 ChartBarItem::ChartBarItem(StatsView &v, ChartZValue z, double borderWidth, bool horizontal) : HideableChartItem(v, z),
@@ -350,6 +360,7 @@ void ChartBarItem::render()
 		view.addQSGNode(node.get(), zValue);
 		positionDirty = colorDirty = true;
 	}
+	updateVisible();
 
 	if (colorDirty) {
 		node->node->setColor(color);
@@ -384,14 +395,14 @@ void ChartBarItem::setColor(QColor colorIn, QColor borderColorIn)
 	color = colorIn;
 	borderColor = borderColorIn;
 	colorDirty = true;
-	view.registerDirtyChartItem(*this);
+	markDirty();
 }
 
 void ChartBarItem::setRect(const QRectF &rectIn)
 {
 	rect = rectIn;
 	positionDirty = true;
-	view.registerDirtyChartItem(*this);
+	markDirty();
 }
 
 QRectF ChartBarItem::getRect() const

stats/chartitem.h

@@ -22,13 +22,14 @@ class ChartItem {
 public:
 	virtual void render() = 0;		// Only call on render thread!
 	bool dirty;				// If true, call render() when rebuilding the scene
-	ChartItem *dirtyPrev, *dirtyNext;	// Double linked list of dirty items
+	ChartItem *prev, *next;			// Double linked list of items
 	const ChartZValue zValue;
+	virtual ~ChartItem();			// Attention: must only be called by render thread.
 protected:
 	ChartItem(StatsView &v, ChartZValue z);
-	virtual ~ChartItem();
 	QSizeF sceneSize() const;
 	StatsView &view;
+	void markDirty();
 };
 
 template <typename Node>
@@ -36,9 +37,11 @@ class HideableChartItem : public ChartItem {
 protected:
 	HideableChartItem(StatsView &v, ChartZValue z);
 	std::unique_ptr<Node> node;
-	bool visible;				// Argh. If visibility is set before node is created, we have to cache it.
+	bool visible;
+	bool visibleChanged;
 	template<class... Args>
 	void createNode(Args&&... args);	// Call to create node with visibility flag.
+	void updateVisible();			// Must be called by child class to update visibility flag!
 public:
 	void setVisible(bool visible);
 };
@@ -186,9 +189,11 @@ private:
 template <typename Node>
 void HideableChartItem<Node>::setVisible(bool visibleIn)
 {
+	if (visible == visibleIn)
+		return;
 	visible = visibleIn;
-	if (node)
+	visibleChanged = true;
-		node->setVisible(visible);
+	markDirty();
 }
 
 template <typename Node>
@@ -196,12 +201,21 @@ template<class... Args>
 void HideableChartItem<Node>::createNode(Args&&... args)
 {
 	node.reset(new Node(visible, std::forward<Args>(args)...));
+	visibleChanged = false;
 }
 
 template <typename Node>
 HideableChartItem<Node>::HideableChartItem(StatsView &v, ChartZValue z) : ChartItem(v, z),
-	visible(true)
+	visible(true), visibleChanged(false)
 {
 }
 
+template <typename Node>
+void HideableChartItem<Node>::updateVisible()
+{
+	if (visibleChanged)
+		node->setVisible(visible);
+	visibleChanged = false;
+}
+
 #endif

stats/pieseries.cpp

@@ -244,8 +244,9 @@ bool PieSeries::hover(QPointF pos)
 		if (!information)
 			information = view.createChartItem<InformationBox>();
 		information->setText(makeInfo(highlighted), pos);
+		information->setVisible(true);
 	} else {
-		information.reset();
+		information->setVisible(false);
 	}
 	return highlighted >= 0;
 }

stats/pieseries.h

@@ -3,6 +3,7 @@
 #ifndef PIE_SERIES_H
 #define PIE_SERIES_H
 
+#include "statshelper.h"
 #include "statsseries.h"
 
 #include <memory>
@@ -33,12 +34,12 @@ private:
 	// Get item under mouse pointer, or -1 if none
 	int getItemUnderMouse(const QPointF &f) const;
 
-	std::unique_ptr<ChartPieItem> item;
+	ChartItemPtr<ChartPieItem> item;
 	QString categoryName;
 	std::vector<QString> makeInfo(int idx) const;
 
 	struct Item {
-		std::unique_ptr<ChartTextItem> innerLabel, outerLabel;
+		ChartItemPtr<ChartTextItem> innerLabel, outerLabel;
 		QString name;
 		double angleFrom, angleTo; // In fraction of total
 		int count;
@@ -58,7 +59,7 @@ private:
 	};
 	std::vector<OtherItem> other;
 
-	std::unique_ptr<InformationBox> information;
+	ChartItemPtr<InformationBox> information;
 	QPointF center; // center of drawing area
 	double radius; // radius of pie
 	int highlighted;

stats/scatterseries.cpp

@@ -116,7 +116,7 @@ bool ScatterSeries::hover(QPointF pos)
 	highlighted = std::move(newHighlighted);
 
 	if (highlighted.empty()) {
-		information.reset();
+		information->setVisible(false);
 		return false;
 	} else {
 		if (!information)
@@ -148,6 +148,7 @@ bool ScatterSeries::hover(QPointF pos)
 		}
 
 		information->setText(text, pos);
+		information->setVisible(true);
 		return true;
 	}
 }

stats/scatterseries.h

@@ -4,6 +4,7 @@
 #ifndef SCATTER_SERIES_H
 #define SCATTER_SERIES_H
 
+#include "statshelper.h"
 #include "statsseries.h"
 
 #include <memory>
@@ -32,7 +33,7 @@ private:
 	std::vector<int> getItemsUnderMouse(const QPointF &f) const;
 
 	struct Item {
-		std::unique_ptr<ChartScatterItem> item;
+		ChartItemPtr<ChartScatterItem> item;
 		dive *d;
 		double pos, value;
 		Item(StatsView &view, ScatterSeries *series, dive *d, double pos, double value);
@@ -40,7 +41,7 @@ private:
 		void highlight(bool highlight);
 	};
 
-	std::unique_ptr<InformationBox> information;
+	ChartItemPtr<InformationBox> information;
 	std::vector<Item> items;
 	std::vector<int> highlighted;
 	const StatsVariable &varX;

stats/statsaxis.cpp

@@ -159,6 +159,9 @@ void StatsAxis::setSize(double sizeIn)
 {
 	size = sizeIn;
 
+	// Ticks (and labels) should probably be reused. For now, clear them.
+	for (Tick &tick: ticks)
+		view.deleteChartItem(tick.item);
 	labels.clear();
 	ticks.clear();
 	updateLabels();

stats/statsaxis.h

@@ -3,6 +3,7 @@
 #define STATS_AXIS_H
 
 #include "chartitem.h"
+#include "statshelper.h"
 
 #include <memory>
 #include <vector>
@@ -36,7 +37,7 @@ public:
 protected:
 	StatsAxis(StatsView &view, const QString &title, bool horizontal, bool labelsBetweenTicks);
 
-	std::unique_ptr<ChartLineItem> line;
+	ChartItemPtr<ChartLineItem> line;
 	QString title;
 	double titleWidth;
 
@@ -51,7 +52,7 @@ protected:
 	virtual std::pair<QString, QString> getFirstLastLabel() const = 0;
 
 	struct Tick {
-		std::unique_ptr<ChartLineItem> item;
+		ChartItemPtr<ChartLineItem> item;
 		double pos;
 	};
 	std::vector<Tick> ticks;

stats/statsgrid.cpp

@@ -20,7 +20,10 @@ void StatsGrid::updatePositions()
 
 	// We probably should be smarter and reuse existing lines.
 	// For now, this does it.
+	for (auto &line: lines)
+		view.deleteChartItem(line);
 	lines.clear();
+
 	if (xtics.empty() || ytics.empty())
 		return;
 

stats/statsgrid.h

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 // The background grid of a chart
 
+#include "statshelper.h"
+
 #include <memory>
 #include <vector>
 
@@ -15,5 +17,5 @@ public:
 private:
 	StatsView &view;
 	const StatsAxis &xAxis, &yAxis;
-	std::vector<std::unique_ptr<ChartLineItem>> lines;
+	std::vector<ChartItemPtr<ChartLineItem>> lines;
 };

stats/statshelper.h

@@ -7,6 +7,52 @@
 #include <memory>
 #include <QSGNode>
 
+// A stupid pointer class that initializes to null and can be copy
+// assigned. This is for historical reasons: unique_ptrs to ChartItems
+// were replaced by plain pointers. Instead of nulling the plain pointers
+// in the constructors, use this. Ultimately, we might think about making
+// this thing smarter, once removal of individual ChartItems is implemented.
+template <typename T>
+class ChartItemPtr {
+	friend class StatsView; // Only the stats view can create these pointers
+	T *ptr;
+	ChartItemPtr(T *ptr) : ptr(ptr)
+	{
+	}
+public:
+	ChartItemPtr() : ptr(nullptr)
+	{
+	}
+	ChartItemPtr(const ChartItemPtr &p) : ptr(p.ptr)
+	{
+	}
+	void reset()
+	{
+		ptr = nullptr;
+	}
+	ChartItemPtr &operator=(const ChartItemPtr &p)
+	{
+		ptr = p.ptr;
+		return *this;
+	}
+	operator bool() const
+	{
+		return !!ptr;
+	}
+	bool operator!() const
+	{
+		return !ptr;
+	}
+	T &operator*() const
+	{
+		return *ptr;
+	}
+	T *operator->() const
+	{
+		return ptr;
+	}
+};
+
 // In general, we want chart items to be hideable. For example to show/hide
 // labels on demand. Very sadly, the QSG API is absolutely terrible with
 // respect to temporarily disabling. Instead of simply having a flag,
@@ -82,6 +128,7 @@ template <typename Node>
 void HideableQSGNode<Node>::setVisible(bool visible)
 {
 	hidden = !visible;
+	Node::markDirty(QSGNode::DirtySubtreeBlocked);
 }
 
 #endif

stats/statsview.cpp

@@ -35,9 +35,7 @@ StatsView::StatsView(QQuickItem *parent) : QQuickItem(parent),
 	xAxis(nullptr),
 	yAxis(nullptr),
 	draggedItem(nullptr),
-	rootNode(nullptr),
+	rootNode(nullptr)
-	firstDirtyChartItem(nullptr),
-	lastDirtyChartItem(nullptr)
 {
 	setFlag(ItemHasContents, true);
 
@@ -68,7 +66,7 @@ void StatsView::mousePressEvent(QMouseEvent *event)
 		if (legend->getRect().contains(pos)) {
 			dragStartMouse = pos;
 			dragStartItem = rect.topLeft();
-			draggedItem = legend.get();
+			draggedItem = &*legend;
 			grabMouse();
 		}
 	}
@@ -114,6 +112,14 @@ QSGNode *StatsView::updatePaintNode(QSGNode *oldNode, QQuickItem::UpdatePaintNod
 	if (!n)
 		n = rootNode = new RootNode(window());
 
+	// Delete all chart items that are marked for deletion.
+	ChartItem *nextitem;
+	for (ChartItem *item = deletedItems.first; item; item = nextitem) {
+		nextitem = item->next;
+		delete item;
+	}
+	deletedItems.clear();
+
 	QRectF rect = boundingRect();
 	if (plotRect != rect) {
 		plotRect = rect;
@@ -121,12 +127,11 @@ QSGNode *StatsView::updatePaintNode(QSGNode *oldNode, QQuickItem::UpdatePaintNod
 		plotAreaChanged(plotRect.size());
 	}
 
-	for (ChartItem *item = std::exchange(firstDirtyChartItem, nullptr); item;
+	for (ChartItem *item = dirtyItems.first; item; item = item->next) {
-	     item = std::exchange(item->dirtyNext, nullptr)) {
 		item->render();
 		item->dirty = false;
-		item->dirtyPrev = nullptr;
 	}
+	dirtyItems.splice(cleanItems);
 
 	return n;
 }
@@ -137,36 +142,76 @@ void StatsView::addQSGNode(QSGNode *node, ChartZValue z)
 	rootNode->zNodes[idx]->appendChildNode(node);
 }
 
-void StatsView::unregisterDirtyChartItem(ChartItem &item)
+void StatsView::registerChartItem(ChartItem &item)
 {
-	if (!item.dirty)
+	cleanItems.append(item);
-		return;
-	if (item.dirtyNext)
-		item.dirtyNext->dirtyPrev = item.dirtyPrev;
-	else
-		lastDirtyChartItem = item.dirtyPrev;
-	if (item.dirtyPrev)
-		item.dirtyPrev->dirtyNext = item.dirtyNext;
-	else
-		firstDirtyChartItem = item.dirtyNext;
-	item.dirtyPrev = item.dirtyNext = nullptr;
-	item.dirty = false;
 }
 
 void StatsView::registerDirtyChartItem(ChartItem &item)
 {
 	if (item.dirty)
 		return;
-	if (!firstDirtyChartItem) {
+	cleanItems.remove(item);
-		firstDirtyChartItem = &item;
+	dirtyItems.append(item);
-	} else {
-		item.dirtyPrev = lastDirtyChartItem;
-		lastDirtyChartItem->dirtyNext = &item;
-	}
-	lastDirtyChartItem = &item;
 	item.dirty = true;
 }
 
+void StatsView::deleteChartItemInternal(ChartItem &item)
+{
+	if (item.dirty)
+		dirtyItems.remove(item);
+	else
+		cleanItems.remove(item);
+	deletedItems.append(item);
+}
+
+StatsView::ChartItemList::ChartItemList() : first(nullptr), last(nullptr)
+{
+}
+
+void StatsView::ChartItemList::clear()
+{
+	first = last = nullptr;
+}
+
+void StatsView::ChartItemList::remove(ChartItem &item)
+{
+	if (item.next)
+		item.next->prev = item.prev;
+	else
+		last = item.prev;
+	if (item.prev)
+		item.prev->next = item.next;
+	else
+		first = item.next;
+	item.prev = item.next = nullptr;
+}
+
+void StatsView::ChartItemList::append(ChartItem &item)
+{
+	if (!first) {
+		first = &item;
+	} else {
+		item.prev = last;
+		last->next = &item;
+	}
+	last = &item;
+}
+
+void StatsView::ChartItemList::splice(ChartItemList &l2)
+{
+	if (!first) // if list is empty -> nothing to do.
+		return;
+	if (!l2.first) {
+		l2 = *this;
+	} else {
+		l2.last->next = first;
+		first->prev = l2.last;
+		l2.last = last;
+	}
+	clear();
+}
+
 QQuickWindow *StatsView::w() const
 {
 	return window();
@@ -286,8 +331,8 @@ T *StatsView::createSeries(Args&&... args)
 
 void StatsView::setTitle(const QString &s)
 {
-	if (s.isEmpty()) {
+	if (title) {
-		title.reset();
+		// Ooops. Currently we do not support setting the title twice.
 		return;
 	}
 	title = createChartItem<ChartTextItem>(ChartZValue::Legend, titleFont, s);
@@ -305,10 +350,7 @@ void StatsView::updateTitlePos()
 template <typename T, class... Args>
 T *StatsView::createAxis(const QString &title, Args&&... args)
 {
-	std::unique_ptr<T> ptr = createChartItem<T>(title, std::forward<Args>(args)...);
+	return &*createChartItem<T>(title, std::forward<Args>(args)...);
-	T *res = ptr.get();
-	axes.push_back(std::move(ptr));
-	return res;
 }
 
 void StatsView::setAxes(StatsAxis *x, StatsAxis *y)
@@ -324,19 +366,18 @@ void StatsView::reset()
 	highlightedSeries = nullptr;
 	xAxis = yAxis = nullptr;
 	draggedItem = nullptr;
-	for (ChartItem *item = std::exchange(firstDirtyChartItem, nullptr); item;
+	title.reset();
-	     item = std::exchange(item->dirtyNext, nullptr)) {
-		item->dirty = false;
-		item->dirtyPrev = nullptr;
-	}
 	legend.reset();
+	regressionItem.reset();
+
+	// Mark clean and dirty chart items for deletion
+	cleanItems.splice(deletedItems);
+	dirtyItems.splice(deletedItems);
+
 	series.clear();
 	quartileMarkers.clear();
 	histogramMarkers.clear();
-	regressionItem.reset();
 	grid.reset();
-	axes.clear();
-	title.reset();
 }
 
 void StatsView::plot(const StatsState &stateIn)

stats/statsview.h

@@ -3,6 +3,7 @@
 #define STATS_VIEW_H
 
 #include "statsstate.h"
+#include "statshelper.h"
 #include <memory>
 #include <QFont>
 #include <QImage>
@@ -46,14 +47,24 @@ public:
 	QSizeF size() const;
 	QRectF plotArea() const;
 	void addQSGNode(QSGNode *node, ChartZValue z);	// Must only be called in render thread!
+	void registerChartItem(ChartItem &item);
 	void registerDirtyChartItem(ChartItem &item);
-	void unregisterDirtyChartItem(ChartItem &item);
-	template <typename T, class... Args>
-	std::unique_ptr<T> createChartItem(Args&&... args);
 
+	// Create a chart item and add it to the scene.
+	// The item must not be deleted by the caller, but can be
+	// scheduled for deletion using deleteChartItem() below.
+	// Most items can be made invisible, which is preferred over deletion.
+	// All items on the scene will be deleted once the chart is reset.
+	template <typename T, class... Args>
+	ChartItemPtr<T> createChartItem(Args&&... args);
+
+	template <typename T>
+	void deleteChartItem(ChartItemPtr<T> &item);
 private slots:
 	void replotIfVisible();
 private:
+	bool resetChart;
+
 	// QtQuick related things
 	QRectF plotRect;
 	QSGNode *updatePaintNode(QSGNode *oldNode, QQuickItem::UpdatePaintNodeData *updatePaintNodeData) override;
@@ -119,17 +130,16 @@ private:
 
 	StatsState state;
 	QFont titleFont;
-	std::vector<std::unique_ptr<StatsAxis>> axes;
-	std::unique_ptr<StatsGrid> grid;
 	std::vector<std::unique_ptr<StatsSeries>> series;
-	std::unique_ptr<Legend> legend;
+	std::unique_ptr<StatsGrid> grid;
-	std::vector<std::unique_ptr<QuartileMarker>> quartileMarkers;
+	std::vector<ChartItemPtr<QuartileMarker>> quartileMarkers;
-	std::vector<std::unique_ptr<HistogramMarker>> histogramMarkers;
+	std::vector<ChartItemPtr<HistogramMarker>> histogramMarkers;
-	std::unique_ptr<ChartTextItem> title;
-	std::unique_ptr<RegressionItem> regressionItem;
 	StatsSeries *highlightedSeries;
 	StatsAxis *xAxis, *yAxis;
+	ChartItemPtr<ChartTextItem> title;
+	ChartItemPtr<Legend> legend;
 	Legend *draggedItem;
+	ChartItemPtr<RegressionItem> regressionItem;
 	QPointF dragStartMouse, dragStartItem;
 
 	void hoverEnterEvent(QHoverEvent *event) override;
@@ -138,16 +148,34 @@ private:
 	void mouseReleaseEvent(QMouseEvent *event) override;
 	void mouseMoveEvent(QMouseEvent *event) override;
 	RootNode *rootNode;
-	ChartItem *firstDirtyChartItem, *lastDirtyChartItem;
+
+	// There are three double linked lists of chart items:
+	// clean items, dirty items and items to be deleted.
+	struct ChartItemList {
+		ChartItemList();
+		ChartItem *first, *last;
+		void append(ChartItem &item);
+		void remove(ChartItem &item);
+		void clear();
+		void splice(ChartItemList &list);
+	};
+	ChartItemList cleanItems, dirtyItems, deletedItems;
+	void deleteChartItemInternal(ChartItem &item);
 };
 
 // This implementation detail must be known to users of the class.
 // Perhaps move it into a statsview_impl.h file.
 template <typename T, class... Args>
-std::unique_ptr<T> StatsView::createChartItem(Args&&... args)
+ChartItemPtr<T> StatsView::createChartItem(Args&&... args)
 {
-	std::unique_ptr<T> res(new T(*this, std::forward<Args>(args)...));
+	return ChartItemPtr(new T(*this, std::forward<Args>(args)...));
-	return res;
+}
+
+template <typename T>
+void StatsView::deleteChartItem(ChartItemPtr<T> &item)
+{
+	deleteChartItemInternal(*item);
+	item.reset();
 }
 
 #endif