Fix memory management of charts

QtCharts can't deal with charts being deleted, so we need to update them
rather than to re-create them.

analyzer/analytics/analyticsview.cpp

@@ -124,11 +124,6 @@ AnalyticsView::AnalyticsView(QWidget* parent) :
 
 AnalyticsView::~AnalyticsView()
 {
-    if (ui->singularChartView->chart())
-        disconnect(ui->singularChartView->chart(), &QObject::destroyed, this, &AnalyticsView::updateChart);
-    if (ui->timelineChartView->chart())
-        disconnect(ui->timelineChartView->chart(), &QObject::destroyed, this, &AnalyticsView::updateChart);
-
     QSettings settings;
     settings.beginGroup(QStringLiteral("Analytics"));
     settings.setValue(QStringLiteral("TimeAggregationMode"), m_timeAggregationModel->aggregationMode());
@@ -201,18 +196,11 @@ void AnalyticsView::updateChart()
     if (!aggr)
         return;
 
-    if (ui->singularChartView->chart())
-        disconnect(ui->singularChartView->chart(), &QObject::destroyed, this, &AnalyticsView::updateChart);
-    if (ui->timelineChartView->chart())
-        disconnect(ui->timelineChartView->chart(), &QObject::destroyed, this, &AnalyticsView::updateChart);
-
     if (ui->actionTimelineChart->isChecked()) {
         ui->timelineChartView->setChart(aggr->timelineChart());
-        connect(ui->timelineChartView->chart(), &QObject::destroyed, this, &AnalyticsView::updateChart);
         ui->chartStack->setCurrentWidget(ui->timelinePage);
     } else if (ui->actionSingularChart->isChecked()) {
         ui->singularChartView->setChart(aggr->singlularChart());
-        connect(ui->singularChartView->chart(), &QObject::destroyed, this, &AnalyticsView::updateChart);
         ui->chartStack->setCurrentWidget(ui->singularPage);
     }
 }

analyzer/analytics/categoryaggregator.cpp

@@ -32,6 +32,8 @@
 #include <QtCharts/QValueAxis>
 #include <QtCharts/QVXYModelMapper>
 
+#include <QDebug>
+
 using namespace UserFeedback::Analyzer;
 using namespace QtCharts;
 
@@ -60,7 +62,8 @@ QAbstractItemModel* CategoryAggregator::timeAggregationModel()
         const auto e = aggregation().elements().at(0);
         m_model->setAggregationValue(e.schemaEntry().name() + QLatin1Char('.') + e.schemaEntryElement().name());
         QObject::connect(m_model.get(), &QAbstractItemModel::modelReset, [this]() {
-                m_timelineChart.reset();
+            updateSingularChart();
+            updateTimelineChart();
         });
     }
     return m_model.get();
@@ -68,16 +71,28 @@ QAbstractItemModel* CategoryAggregator::timeAggregationModel()
 
 QtCharts::QChart* CategoryAggregator::timelineChart()
 {
-    if (m_timelineChart)
-        return m_timelineChart.get();
+    if (!m_timelineChart) {
+        m_timelineChart.reset(new QChart);
+        ChartUtil::applyTheme(m_timelineChart.get());
+        auto xAxis = new QDateTimeAxis(m_timelineChart.get());
+        xAxis->setFormat(QStringLiteral("yyyy-MM-dd")); // TODO, follow aggregation mode
+        auto yAxis = new QValueAxis(m_timelineChart.get());
+        xAxis->setTickCount(std::min(timeAggregationModel()->rowCount(), 12));
+        yAxis->setMinorTickCount(4);
+
+        m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
+        m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+        updateTimelineChart();
+    }
+
+    return m_timelineChart.get();
+}
 
-    m_timelineChart.reset(new QChart);
-    ChartUtil::applyTheme(m_timelineChart.get());
-    auto xAxis = new QDateTimeAxis(m_timelineChart.get());
-    xAxis->setFormat(QStringLiteral("yyyy-MM-dd")); // TODO, follow aggregation mode
-    auto yAxis = new QValueAxis(m_timelineChart.get());
-    m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
-    m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+void CategoryAggregator::updateTimelineChart()
+{
+    if (!m_timelineChart)
+        return;
+    m_timelineChart->removeAllSeries();
 
     QLineSeries *prevSeries = nullptr;
     auto model = new RoleMappingProxyModel(m_timelineChart.get());
@@ -98,26 +113,36 @@ QtCharts::QChart* CategoryAggregator::timelineChart()
         areaSeries->setName(timeAggregationModel()->headerData(i, Qt::Horizontal).toString().toHtmlEscaped());
         m_timelineChart->addSeries(areaSeries);
 
-        areaSeries->attachAxis(xAxis);
-        areaSeries->attachAxis(yAxis);
+        areaSeries->attachAxis(m_timelineChart->axisX());
+        areaSeries->attachAxis(m_timelineChart->axisY());
 
         prevSeries = series;
     }
 
-    xAxis->setTickCount(std::min(timeAggregationModel()->rowCount(), 12));
-    yAxis->setMin(0);
-    yAxis->setMinorTickCount(4);
-
-    return m_timelineChart.get();
+    const auto max = timeAggregationModel()->index(0, 0).data(TimeAggregationModel::MaximumValueRole).toInt();
+    m_timelineChart->axisY()->setRange(0, max);
+    qobject_cast<QValueAxis*>(m_timelineChart->axisY())->applyNiceNumbers();
 }
 
 QtCharts::QChart* CategoryAggregator::singlularChart()
 {
-    if (m_singlularChart)
-        return m_singlularChart.get();
+    if (!m_singlularChart) {
+        m_singlularChart.reset(new QChart);
+        ChartUtil::applyTheme(m_singlularChart.get());
+        updateSingularChart();
+    }
 
-    m_singlularChart.reset(new QChart);
-    ChartUtil::applyTheme(m_singlularChart.get());
+    return m_singlularChart.get();
+}
+
+void CategoryAggregator::updateSingularChart()
+{
+    if (!m_singlularChart)
+        return;
+    m_singlularChart->removeAllSeries();
+
+    if (sourceModel()->rowCount() <= 0)
+        return;
 
     auto series = new QPieSeries(m_singlularChart.get());
     auto mapper = new QHPieModelMapper(m_singlularChart.get());
@@ -130,5 +155,4 @@ QtCharts::QChart* CategoryAggregator::singlularChart()
     mapper->setSeries(series);
 
     m_singlularChart->addSeries(series);
-    return m_singlularChart.get();
 }

analyzer/analytics/categoryaggregator.h

@@ -40,6 +40,9 @@ public:
     QtCharts::QChart* singlularChart() override;
 
 private:
+    void updateTimelineChart();
+    void updateSingularChart();
+
     std::unique_ptr<CategoryAggregationModel> m_model;
     std::unique_ptr<QtCharts::QChart> m_timelineChart;
     std::unique_ptr<QtCharts::QChart> m_singlularChart;

analyzer/analytics/numericaggregator.cpp

@@ -55,7 +55,7 @@ QAbstractItemModel* NumericAggregator::timeAggregationModel()
         const auto e = aggregation().elements().at(0);
         m_model->setAggregationValue(e.schemaEntry().name() + QLatin1Char('.') + e.schemaEntryElement().name());
         QObject::connect(m_model.get(), &QAbstractItemModel::modelReset, [this]() {
-            m_timelineChart.reset();
+            updateTimelineChart();
         });
     }
     return m_model.get();
@@ -63,15 +63,25 @@ QAbstractItemModel* NumericAggregator::timeAggregationModel()
 
 QtCharts::QChart* NumericAggregator::timelineChart()
 {
-    if (m_timelineChart)
-        return m_timelineChart.get();
+    if (!m_timelineChart) {
+        m_timelineChart.reset(new QChart);
+        ChartUtil::applyTheme(m_timelineChart.get());
+        auto xAxis = new QBarCategoryAxis(m_timelineChart.get());
+        auto yAxis = new QValueAxis(m_timelineChart.get());
+        yAxis->setMinorTickCount(4);
+        m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
+        m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+        updateTimelineChart();
+    }
+
+    return m_timelineChart.get();
+}
 
-    m_timelineChart.reset(new QChart);
-    ChartUtil::applyTheme(m_timelineChart.get());
-    auto xAxis = new QBarCategoryAxis(m_timelineChart.get());
-    auto yAxis = new QValueAxis(m_timelineChart.get());
-    m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
-    m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+void NumericAggregator::updateTimelineChart()
+{
+    if (!m_timelineChart)
+        return;
+    m_timelineChart->removeAllSeries();
 
     auto series = new QBoxPlotSeries(m_timelineChart.get());
     series->setName(displayName());
@@ -83,15 +93,16 @@ QtCharts::QChart* NumericAggregator::timelineChart()
     mapper->setSeries(series);
     m_timelineChart->addSeries(series);
 
-    series->attachAxis(xAxis);
-    series->attachAxis(yAxis);
+    series->attachAxis(m_timelineChart->axisX());
+    series->attachAxis(m_timelineChart->axisY());
 
     QStringList l;
     for (int i = 0; i < m_model->rowCount(); ++i) {
         l.push_back(timeAggregationModel()->index(i, 0).data(TimeAggregationModel::DateTimeRole).toDateTime().toString(QStringLiteral("yyyy-MM-dd")));
     }
-    xAxis->setCategories(l);
-    yAxis->setMinorTickCount(4);
 
-    return m_timelineChart.get();
+    qobject_cast<QBarCategoryAxis*>(m_timelineChart->axisX())->setCategories(l);
+    const auto max = timeAggregationModel()->index(0, 0).data(TimeAggregationModel::MaximumValueRole).toInt();
+    m_timelineChart->axisY()->setRange(0, max);
+    qobject_cast<QValueAxis*>(m_timelineChart->axisY())->applyNiceNumbers();
 }

analyzer/analytics/numericaggregator.h

@@ -39,6 +39,8 @@ public:
     QtCharts::QChart* timelineChart() override;
 
 private:
+    void updateTimelineChart();
+
     std::unique_ptr<NumericAggregationModel> m_model;
     std::unique_ptr<QtCharts::QChart> m_timelineChart;
 };

analyzer/analytics/ratiosetaggregator.cpp

@@ -55,7 +55,7 @@ QAbstractItemModel* RatioSetAggregator::timeAggregationModel()
         const auto e = aggregation().elements().at(0);
         m_model->setAggregationValue(e.schemaEntry().name());
         QObject::connect(m_model.get(), &QAbstractItemModel::modelReset, [this]() {
-                m_timelineChart.reset();
+            updateTimelineChart();
         });
     }
     return m_model.get();
@@ -63,16 +63,26 @@ QAbstractItemModel* RatioSetAggregator::timeAggregationModel()
 
 QtCharts::QChart* RatioSetAggregator::timelineChart()
 {
-    if (m_timelineChart)
-        return m_timelineChart.get();
+    if (!m_timelineChart) {
+        m_timelineChart.reset(new QChart);
+        ChartUtil::applyTheme(m_timelineChart.get());
+        auto xAxis = new QDateTimeAxis(m_timelineChart.get());
+        xAxis->setFormat(QStringLiteral("yyyy-MM-dd")); // TODO, follow aggregation mode
+        auto yAxis = new QValueAxis(m_timelineChart.get());
+        yAxis->setTickCount(11);
+        m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
+        m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+        updateTimelineChart();
+    }
 
-    m_timelineChart.reset(new QChart);
-    ChartUtil::applyTheme(m_timelineChart.get());
-    auto xAxis = new QDateTimeAxis(m_timelineChart.get());
-    xAxis->setFormat(QStringLiteral("yyyy-MM-dd")); // TODO, follow aggregation mode
-    auto yAxis = new QValueAxis(m_timelineChart.get());
-    m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
-    m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+    return m_timelineChart.get();
+}
+
+void RatioSetAggregator::updateTimelineChart()
+{
+    if (!m_timelineChart)
+        return;
+    m_timelineChart->removeAllSeries();
 
     QLineSeries *prevSeries = nullptr;
     for (int i = 1; i < timeAggregationModel()->columnCount(); ++i) {
@@ -90,16 +100,12 @@ QtCharts::QChart* RatioSetAggregator::timelineChart()
         areaSeries->setName(timeAggregationModel()->headerData(i, Qt::Horizontal).toString().toHtmlEscaped());
         m_timelineChart->addSeries(areaSeries);
 
-        areaSeries->attachAxis(xAxis);
-        areaSeries->attachAxis(yAxis);
+        areaSeries->attachAxis(m_timelineChart->axisX());
+        areaSeries->attachAxis(m_timelineChart->axisY());
 
         prevSeries = series;
     }
 
-    xAxis->setTickCount(std::min(timeAggregationModel()->rowCount(), 12));
-    yAxis->setMin(0);
-    yAxis->setMax(1); // TODO can we turn this into *100% for display?
-    yAxis->setTickCount(11);
-
-    return m_timelineChart.get();
+    qobject_cast<QDateTimeAxis*>(m_timelineChart->axisX())->setTickCount(std::min(timeAggregationModel()->rowCount(), 12));
+    m_timelineChart->axisY()->setRange(0, 1); // TODO can we turn this into *100% for display?
 }

analyzer/analytics/ratiosetaggregator.h

@@ -39,6 +39,8 @@ public:
     QtCharts::QChart* timelineChart() override;
 
 private:
+    void updateTimelineChart();
+
     std::unique_ptr<RatioSetAggregationModel> m_model;
     std::unique_ptr<QtCharts::QChart> m_timelineChart;
 };

analyzer/analytics/totalaggregator.cpp

@@ -18,6 +18,8 @@
 #include "totalaggregator.h"
 #include "chartutil.h"
 
+#include <model/timeaggregationmodel.h>
+
 #include <QtCharts/QChart>
 #include <QtCharts/QDateTimeAxis>
 #include <QtCharts/QLineSeries>
@@ -51,20 +53,32 @@ QAbstractItemModel* TotalAggregator::timeAggregationModel()
 
 QtCharts::QChart* TotalAggregator::timelineChart()
 {
-    if (m_timelineChart)
-        return m_timelineChart.get();
+    if (!m_timelineChart) {
+        m_timelineChart.reset(new QChart);
+        ChartUtil::applyTheme(m_timelineChart.get());
+        auto xAxis = new QDateTimeAxis(m_timelineChart.get());
+        xAxis->setFormat(QStringLiteral("yyyy-MM-dd")); // TODO, follow aggregation mode
+        auto yAxis = new QValueAxis(m_timelineChart.get());
+        yAxis->setMinorTickCount(4);
+        m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
+        m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+        updateTimelineChart();
+
+        QObject::connect(sourceModel(), &QAbstractItemModel::modelReset, [this]() {
+            updateTimelineChart();
+        });
+    }
 
-    QObject::connect(sourceModel(), &QAbstractItemModel::modelReset, sourceModel(), [this]() {
-            m_timelineChart.reset();
-    }, Qt::UniqueConnection);
+    return m_timelineChart.get();
+}
 
-    m_timelineChart.reset(new QChart);
-    ChartUtil::applyTheme(m_timelineChart.get());
-    auto xAxis = new QDateTimeAxis(m_timelineChart.get());
-    xAxis->setFormat(QStringLiteral("yyyy-MM-dd")); // TODO, follow aggregation mode
-    auto yAxis = new QValueAxis(m_timelineChart.get());
-    m_timelineChart->addAxis(xAxis, Qt::AlignBottom);
-    m_timelineChart->addAxis(yAxis, Qt::AlignLeft);
+void TotalAggregator::updateTimelineChart()
+{
+    if (!m_timelineChart)
+        return;
+    m_timelineChart->removeAllSeries();
+    if (timeAggregationModel()->rowCount() <= 0)
+        return;
 
     auto series = new QLineSeries(m_timelineChart.get());
     series->setName(displayName());
@@ -76,12 +90,14 @@ QtCharts::QChart* TotalAggregator::timelineChart()
     mapper->setSeries(series);
     m_timelineChart->addSeries(series);
 
-    series->attachAxis(xAxis);
-    series->attachAxis(yAxis);
-
-    xAxis->setTickCount(std::min(timeAggregationModel()->rowCount(), 12));
-    yAxis->setMin(0);
-    yAxis->setMinorTickCount(4);
+    series->attachAxis(m_timelineChart->axisX());
+    series->attachAxis(m_timelineChart->axisY());
 
-    return m_timelineChart.get();
+    const auto beginDt = timeAggregationModel()->index(0, 0).data(TimeAggregationModel::DateTimeRole).toDateTime();
+    const auto endDt = timeAggregationModel()->index(timeAggregationModel()->rowCount() - 1, 0).data(TimeAggregationModel::DateTimeRole).toDateTime();
+    m_timelineChart->axisX()->setRange(beginDt, endDt);
+    qobject_cast<QDateTimeAxis*>(m_timelineChart->axisX())->setTickCount(std::min(timeAggregationModel()->rowCount(), 12));
+    const auto max = timeAggregationModel()->index(0, 0).data(TimeAggregationModel::MaximumValueRole).toInt();
+    m_timelineChart->axisY()->setRange(0, max);
+    qobject_cast<QValueAxis*>(m_timelineChart->axisY())->applyNiceNumbers();
 }

analyzer/analytics/totalaggregator.h

@@ -40,6 +40,8 @@ public:
     QtCharts::QChart* timelineChart() override;
 
 private:
+    void updateTimelineChart();
+
     std::unique_ptr<QtCharts::QChart> m_timelineChart;
 };