diff --git a/src/libs/timeline/timelinemodel.cpp b/src/libs/timeline/timelinemodel.cpp index 0391e55f4dd..099492114c2 100644 --- a/src/libs/timeline/timelinemodel.cpp +++ b/src/libs/timeline/timelinemodel.cpp @@ -80,14 +80,27 @@ void TimelineModel::computeNesting() parentIt = parents.erase(parentIt); } else if (parentEnd >= current.start + current.duration) { // Current range is completely inside the parent range: no need to insert - current.parent = *parentIt; + current.parent = (parent.parent == -1 ? *parentIt : parent.parent); break; } else if (parent.start == current.start) { // The parent range starts at the same time but ends before the current range. - // We need to switch them. - QTC_CHECK(parent.parent == -1); - parent.parent = range; - *parentIt = range; + // We could switch them but that would violate the order requirements. When + // searching for ranges between two timestamps we'd skip the ranges between the + // current range and the parent range if the start timestamp points into the parent + // range. firstIndex() would then return the current range, which has an id greater + // than the parent. The parent could not be found then. To deal with this corner + // case, we assign the parent the "wrong" way around, so that on firstIndex() we + // always end up with the smallest id of any ranges starting at the same time. + + // The other way to deal with this would be fixing up the ordering on insert. In + // fact we do that on insertStart(). + // However, in order to rely on this we would also have to move the start index if + // on insertEnd() it turns out that the range just being ended is shorter than a + // previous one starting at the same time. We don't want to do that as client code + // could not find out about the changes in the IDs for range starts then. + + current.parent = *parentIt; + parents.append(range); break; } else { ++parentIt; @@ -315,8 +328,9 @@ int TimelineModel::firstIndex(qint64 startTime) const } /*! - Looks up the first range with an end time later than the specified \a startTime and - returns its index. If no such range is found, it returns -1. + Of all indexes of ranges starting at the same time as the first range with an end time later + than the specified \a startTime returns the lowest one. If no such range is found, it returns + -1. */ int TimelineModel::TimelineModelPrivate::firstIndexNoParents(qint64 startTime) const { @@ -434,11 +448,10 @@ int TimelineModel::insert(qint64 startTime, qint64 duration, int selectionId) Q_D(TimelineModel); /* Doing insert-sort here is preferable as most of the time the times will actually be * presorted in the right way. So usually this will just result in appending. */ - int index = d->insertSorted(d->ranges, - TimelineModelPrivate::Range(startTime, duration, selectionId)); + int index = d->insertStart(TimelineModelPrivate::Range(startTime, duration, selectionId)); if (index < d->ranges.size() - 1) d->incrementStartIndices(index); - d->insertSorted(d->endTimes, TimelineModelPrivate::RangeEnd(index, startTime + duration)); + d->insertEnd(TimelineModelPrivate::RangeEnd(index, startTime + duration)); return index; } @@ -451,7 +464,7 @@ int TimelineModel::insert(qint64 startTime, qint64 duration, int selectionId) int TimelineModel::insertStart(qint64 startTime, int selectionId) { Q_D(TimelineModel); - int index = d->insertSorted(d->ranges, TimelineModelPrivate::Range(startTime, 0, selectionId)); + int index = d->insertStart(TimelineModelPrivate::Range(startTime, 0, selectionId)); if (index < d->ranges.size() - 1) d->incrementStartIndices(index); return index; @@ -464,8 +477,7 @@ void TimelineModel::insertEnd(int index, qint64 duration) { Q_D(TimelineModel); d->ranges[index].duration = duration; - d->insertSorted(d->endTimes, TimelineModelPrivate::RangeEnd(index, - d->ranges[index].start + duration)); + d->insertEnd(TimelineModelPrivate::RangeEnd(index, d->ranges[index].start + duration)); } bool TimelineModel::expanded() const diff --git a/src/libs/timeline/timelinemodel_p.h b/src/libs/timeline/timelinemodel_p.h index 7f563edf165..fb8e9a7aeea 100644 --- a/src/libs/timeline/timelinemodel_p.h +++ b/src/libs/timeline/timelinemodel_p.h @@ -82,16 +82,31 @@ public: } } - template - static inline int insertSorted(QVector &container, const RangeDelimiter &item) + inline int insertStart(const Range &start) { - for (int i = container.count();;) { + for (int i = ranges.count();;) { if (i == 0) { - container.prepend(item); + ranges.prepend(start); return 0; } - if (container[--i].timestamp() <= item.timestamp()) { - container.insert(++i, item); + const Range &range = ranges[--i]; + if (range.start < start.start || (range.start == start.start && + range.duration >= start.duration)) { + ranges.insert(++i, start); + return i; + } + } + } + + inline int insertEnd(const RangeEnd &end) + { + for (int i = endTimes.count();;) { + if (i == 0) { + endTimes.prepend(end); + return 0; + } + if (endTimes[--i].end <= end.end) { + endTimes.insert(++i, end); return i; } } diff --git a/tests/auto/timeline/timelinemodel/tst_timelinemodel.cpp b/tests/auto/timeline/timelinemodel/tst_timelinemodel.cpp index a316c64d805..c253503d9a6 100644 --- a/tests/auto/timeline/timelinemodel/tst_timelinemodel.cpp +++ b/tests/auto/timeline/timelinemodel/tst_timelinemodel.cpp @@ -102,6 +102,10 @@ DummyModel::DummyModel(QString displayName, QObject *parent) : void DummyModel::loadData() { + // TimelineModel is clever enough to sort the longer events before the short ones with the same + // start time to avoid unnecessary complications when assigning parents. So, if you do e.g. + // firstIndex(0) you get the first event insered in the third iteration of the loop. + for (int i = 0; i < NumItems / 4; ++i) { insert((i / 3) * ItemSpacing, ItemDuration * (i + 1) + 2, 4); insert((i / 3) * ItemSpacing + 1, ItemDuration * (i + 1), 5); @@ -248,8 +252,8 @@ void tst_TimelineModel::times() DummyModel dummy; dummy.loadData(); QCOMPARE(dummy.startTime(0), 0); - QCOMPARE(dummy.duration(0), ItemDuration + 2); - QCOMPARE(dummy.endTime(0), ItemDuration + 2); + QCOMPARE(dummy.duration(0), ItemDuration * 3 + 2); + QCOMPARE(dummy.endTime(0), ItemDuration * 3 + 2); } @@ -272,8 +276,8 @@ void tst_TimelineModel::firstLast() QCOMPARE(dummy.lastIndex(5), 0); dummy.clear(); dummy.loadData(); - QCOMPARE(dummy.firstIndex(0), 2); - QCOMPARE(dummy.firstIndex(ItemSpacing + 1), 2); + QCOMPARE(dummy.firstIndex(0), 0); + QCOMPARE(dummy.firstIndex(ItemSpacing + 1), 0); QCOMPARE(dummy.lastIndex(0), -1); QCOMPARE(dummy.lastIndex(ItemSpacing + 1), 14); QCOMPARE(dummy.firstIndex(ItemDuration * 5000), -1); @@ -362,7 +366,7 @@ void tst_TimelineModel::colorBySelectionId() { DummyModel dummy; dummy.loadData(); - QCOMPARE(dummy.colorBySelectionId(5), QColor::fromHsl(5 * 25, 150, 166)); + QCOMPARE(dummy.colorBySelectionId(5), QColor::fromHsl(6 * 25, 150, 166)); } void tst_TimelineModel::colorByFraction() @@ -388,6 +392,14 @@ void tst_TimelineModel::insertStartEnd() dummy.insertEnd(id2, 10); QCOMPARE(dummy.startTime(id2), 5); QCOMPARE(dummy.endTime(id2), 15); + + int id3 = dummy.insertStart(10, 1); + QVERIFY(id3 > id); + dummy.insertEnd(id3, 20); + + // Make sure that even though the new range is larger than the one pointed to by id, we still + // get to search from id when looking up a timestamp which is in both ranges. + QCOMPARE(dummy.firstIndex(11), id); } void tst_TimelineModel::rowCount() @@ -426,7 +438,7 @@ void tst_TimelineModel::prevNext() QCOMPARE(dummy.nextItemBySelectionId(5, 10, -1), 15); QCOMPARE(dummy.prevItemBySelectionId(5, 10, -1), 6); QCOMPARE(dummy.nextItemBySelectionId(5, 10, 5), 6); - QCOMPARE(dummy.prevItemBySelectionId(5, 10, 5), 3); + QCOMPARE(dummy.prevItemBySelectionId(5, 10, 5), 4); QCOMPARE(dummy.nextItemByTypeId(-1, 10, 5), 6); QCOMPARE(dummy.prevItemByTypeId(-1, 10, 5), 4); }