From c679b3202a8622b47d4508799e6eb0cbb80d5f24 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 14 May 2018 09:34:22 +0200 Subject: [PATCH] Tracing: Simplify TimelineModel signals Instead of laborously calculating which properties have changed under which circumstances, we can just connect the signals of dependent properties. This will give us a few false positive signals at a greatly reduced risk of missing some actual change. Also, the number of expanded and collapsed rows will always be determined by the content of the model. We don't need separate signals for those. Change-Id: Id8495ee525a91405b039fd032509afa125f96412 Reviewed-by: Tobias Hunger --- src/libs/tracing/timelinemodel.cpp | 43 ++++--------------- src/libs/tracing/timelinemodel.h | 6 +-- .../timelinemodel/tst_timelinemodel.cpp | 11 ++--- 3 files changed, 15 insertions(+), 45 deletions(-) diff --git a/src/libs/tracing/timelinemodel.cpp b/src/libs/tracing/timelinemodel.cpp index 39a43b3f681..c7b7c989042 100644 --- a/src/libs/tracing/timelinemodel.cpp +++ b/src/libs/tracing/timelinemodel.cpp @@ -111,14 +111,8 @@ int TimelineModel::collapsedRowCount() const void TimelineModel::setCollapsedRowCount(int rows) { - if (d->collapsedRowCount != rows) { + if (d->collapsedRowCount != rows) d->collapsedRowCount = rows; - emit collapsedRowCountChanged(); - if (!d->expanded) { - emit rowCountChanged(); - emit heightChanged(); // collapsed rows have a fixed size - } - } } int TimelineModel::expandedRowCount() const @@ -129,16 +123,9 @@ int TimelineModel::expandedRowCount() const void TimelineModel::setExpandedRowCount(int rows) { if (d->expandedRowCount != rows) { - int prevHeight = height(); if (d->rowOffsets.length() > rows) d->rowOffsets.resize(rows); d->expandedRowCount = rows; - emit expandedRowCountChanged(); - if (d->expanded) { - emit rowCountChanged(); - if (height() != prevHeight) - emit heightChanged(); - } } } @@ -158,6 +145,13 @@ TimelineModel::TimelineModel(TimelineModelAggregator *parent) : { connect(this, &TimelineModel::contentChanged, this, &TimelineModel::labelsChanged); connect(this, &TimelineModel::contentChanged, this, &TimelineModel::detailsChanged); + connect(this, &TimelineModel::hiddenChanged, this, &TimelineModel::heightChanged); + connect(this, &TimelineModel::expandedChanged, this, &TimelineModel::heightChanged); + connect(this, &TimelineModel::expandedRowHeightChanged, this, &TimelineModel::heightChanged); + connect(this, &TimelineModel::expandedChanged, this, &TimelineModel::rowCountChanged); + connect(this, &TimelineModel::contentChanged, this, &TimelineModel::rowCountChanged); + connect(this, &TimelineModel::contentChanged, + this, [this]() { emit expandedRowHeightChanged(-1, -1); }); } TimelineModel::~TimelineModel() @@ -220,8 +214,6 @@ void TimelineModel::setExpandedRowHeight(int rowNumber, int height) d->rowOffsets[offsetRow] += difference; } emit expandedRowHeightChanged(rowNumber, height); - if (d->expanded) - emit heightChanged(); } } @@ -486,13 +478,8 @@ bool TimelineModel::expanded() const void TimelineModel::setExpanded(bool expanded) { if (expanded != d->expanded) { - int prevHeight = height(); d->expanded = expanded; emit expandedChanged(); - if (prevHeight != height()) - emit heightChanged(); - if (d->collapsedRowCount != d->expandedRowCount) - emit rowCountChanged(); } } @@ -504,11 +491,8 @@ bool TimelineModel::hidden() const void TimelineModel::setHidden(bool hidden) { if (hidden != d->hidden) { - int prevHeight = height(); d->hidden = hidden; emit hiddenChanged(); - if (height() != prevHeight) - emit heightChanged(); } } @@ -574,8 +558,6 @@ int TimelineModel::selectionId(int index) const void TimelineModel::clear() { - bool hadRowHeights = !d->rowOffsets.empty(); - bool wasEmpty = isEmpty(); setExpandedRowCount(1); setCollapsedRowCount(1); setExpanded(false); @@ -583,14 +565,7 @@ void TimelineModel::clear() d->rowOffsets.clear(); d->ranges.clear(); d->endTimes.clear(); - if (hadRowHeights) - emit expandedRowHeightChanged(-1, -1); - if (!wasEmpty) { - emit contentChanged(); - emit heightChanged(); - emit labelsChanged(); - emit detailsChanged(); - } + emit contentChanged(); } int TimelineModel::nextItemBySelectionId(int selectionId, qint64 time, int currentItem) const diff --git a/src/libs/tracing/timelinemodel.h b/src/libs/tracing/timelinemodel.h index a873860f502..65d4784e07e 100644 --- a/src/libs/tracing/timelinemodel.h +++ b/src/libs/tracing/timelinemodel.h @@ -44,8 +44,8 @@ class TRACING_EXPORT TimelineModel : public QObject Q_PROPERTY(bool hidden READ hidden WRITE setHidden NOTIFY hiddenChanged) Q_PROPERTY(bool expanded READ expanded WRITE setExpanded NOTIFY expandedChanged) Q_PROPERTY(int height READ height NOTIFY heightChanged) - Q_PROPERTY(int expandedRowCount READ expandedRowCount NOTIFY expandedRowCountChanged) - Q_PROPERTY(int collapsedRowCount READ collapsedRowCount NOTIFY collapsedRowCountChanged) + Q_PROPERTY(int expandedRowCount READ expandedRowCount NOTIFY contentChanged) + Q_PROPERTY(int collapsedRowCount READ collapsedRowCount NOTIFY contentChanged) Q_PROPERTY(int rowCount READ rowCount NOTIFY rowCountChanged) Q_PROPERTY(QVariantList labels READ labels NOTIFY labelsChanged) Q_PROPERTY(int count READ count NOTIFY contentChanged) @@ -122,8 +122,6 @@ signals: void expandedRowHeightChanged(int row, int height); void contentChanged(); void heightChanged(); - void expandedRowCountChanged(); - void collapsedRowCountChanged(); void rowCountChanged(); void displayNameChanged(); void labelsChanged(); diff --git a/tests/auto/tracing/timelinemodel/tst_timelinemodel.cpp b/tests/auto/tracing/timelinemodel/tst_timelinemodel.cpp index be527af1273..812f51d2267 100644 --- a/tests/auto/tracing/timelinemodel/tst_timelinemodel.cpp +++ b/tests/auto/tracing/timelinemodel/tst_timelinemodel.cpp @@ -108,6 +108,7 @@ void DummyModel::loadData() computeNesting(); setCollapsedRowCount(2); setExpandedRowCount(3); + emit contentChanged(); } tst_TimelineModel::tst_TimelineModel() : @@ -216,7 +217,6 @@ void tst_TimelineModel::height() int heightAfterLastSignal = 0; int heightChangedSignals = 0; connect(&dummy, &Timeline::TimelineModel::heightChanged, [&](){ - QVERIFY(dummy.height() != heightAfterLastSignal); ++heightChangedSignals; heightAfterLastSignal = dummy.height(); }); @@ -432,20 +432,17 @@ void tst_TimelineModel::insertStartEnd() void tst_TimelineModel::rowCount() { DummyModel dummy(&aggregator); - QSignalSpy expandedSpy(&dummy, SIGNAL(expandedRowCountChanged())); - QSignalSpy collapsedSpy(&dummy, SIGNAL(collapsedRowCountChanged())); + QSignalSpy contentSpy(&dummy, SIGNAL(contentChanged())); QCOMPARE(dummy.rowCount(), 1); dummy.setExpanded(true); QCOMPARE(dummy.rowCount(), 1); dummy.loadData(); - QCOMPARE(expandedSpy.count(), 1); - QCOMPARE(collapsedSpy.count(), 1); + QCOMPARE(contentSpy.count(), 1); QCOMPARE(dummy.rowCount(), 3); dummy.setExpanded(false); QCOMPARE(dummy.rowCount(), 2); dummy.clear(); - QCOMPARE(expandedSpy.count(), 2); - QCOMPARE(collapsedSpy.count(), 2); + QCOMPARE(contentSpy.count(), 2); QCOMPARE(dummy.rowCount(), 1); }