From 7b5db5db90decf6d1aaa911f3a195e0515a161df Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 24 Mar 2017 12:04:36 +0100 Subject: [PATCH] Timeline: Fix and clarify index lookup There was a subtle bug in firstIndexNoParents(): If the model had exactly one range with an endtime <= the given start time, it would return 0, rather than -1. Also, add some comments, and don't check for count == 1. The check for count == 1 is redundant as that case is already covered by the endTime and startTime checks for the first and last elements. As count == 1 is really rare and this is a very hot code path, we drop it. Change-Id: Ic21318cf82d2aea4c70d96989c56c2870dc871f7 Reviewed-by: hjk Reviewed-by: Joerg Bornemann --- src/libs/timeline/timelinemodel.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libs/timeline/timelinemodel.cpp b/src/libs/timeline/timelinemodel.cpp index aee72685026..438c2c36a5c 100644 --- a/src/libs/timeline/timelinemodel.cpp +++ b/src/libs/timeline/timelinemodel.cpp @@ -322,12 +322,14 @@ int TimelineModel::firstIndex(qint64 startTime) const int TimelineModel::TimelineModelPrivate::firstIndexNoParents(qint64 startTime) const { // in the "endtime" list, find the first event that ends after startTime - if (endTimes.isEmpty()) + + // lowerBound() cannot deal with empty lists, and it never finds the last element. + if (endTimes.isEmpty() || endTimes.last().end <= startTime) return -1; - if (endTimes.count() == 1 || endTimes.first().end > startTime) + + // lowerBound() never returns "invalid", so handle this manually. + if (endTimes.first().end > startTime) return endTimes.first().startIndex; - if (endTimes.last().end <= startTime) - return -1; return endTimes[lowerBound(endTimes, startTime) + 1].startIndex; } @@ -340,10 +342,12 @@ int TimelineModel::lastIndex(qint64 endTime) const { Q_D(const TimelineModel); // in the "starttime" list, find the last event that starts before endtime + + // lowerBound() never returns "invalid", so handle this manually. if (d->ranges.isEmpty() || d->ranges.first().start >= endTime) return -1; - if (d->ranges.count() == 1) - return 0; + + // lowerBound() never finds the last element. if (d->ranges.last().start < endTime) return d->ranges.count() - 1;