QmlProfiler: Clean up type selection in statistics model

The statistics have one extra valid event type: "Main Program". This
should not be mapped to typeId -1, as -1 is the invalid typeId. Map it
instead to std::numeric_limits<int>::max() and remove all the hacks
around it.

Additionally, optimize the selection algorithm for the statistics main
view: We don't need to iterate all types to select one. We can rather
use the fact that the row numbers in the source model match the type
indices (except for the two special ones).

Change-Id: I7c4dc4f84fd167f9a21c418466ad2bfce56e441f
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>

src/plugins/qmlprofiler/qmlprofilerstatisticsmodel.cpp

@@ -49,16 +49,18 @@ QString nameForType(RangeType typeNumber)
 
 double QmlProfilerStatisticsModel::durationPercent(int typeId) const
 {
-    return (typeId >= 0)
+    if (typeId < 0)
-            ? double(m_data[typeId].totalNonRecursive()) / double(m_rootDuration) * 100
+        return 0;
-            : 100;
+    else if (typeId >= m_data.length())
+        return 100;
+    return double(m_data[typeId].totalNonRecursive()) / double(m_rootDuration) * 100;
 }
 
 double QmlProfilerStatisticsModel::durationSelfPercent(int typeId) const
 {
-    return (typeId >= 0)
+    if (typeId < 0 || typeId >= m_data.length())
-            ? (double(m_data[typeId].self) / double(m_rootDuration) * 100)
+        return 0;
-            : 0;
+    return double(m_data[typeId].self) / double(m_rootDuration) * 100;
 }
 
 QmlProfilerStatisticsModel::QmlProfilerStatisticsModel(QmlProfilerModelManager *modelManager)
@@ -112,7 +114,7 @@ void QmlProfilerStatisticsModel::restrictToFeatures(quint64 features)
         clear();
     } else {
         finalize();
-        notesChanged(-1); // Reload notes
+        notesChanged(QmlProfilerStatisticsModel::s_invalidTypeId); // Reload notes
     }
 }
 
@@ -125,10 +127,10 @@ QStringList QmlProfilerStatisticsModel::details(int typeIndex) const
 {
     QString data;
     QString displayName;
-    if (typeIndex < 0) {
+
-        data = tr("Main Program");
+    const QVector<QmlEventType> &eventTypes = m_modelManager->eventTypes();
-    } else {
+    if (typeIndex >= 0 && typeIndex < eventTypes.count()) {
-        const QmlEventType &type = m_modelManager->eventTypes().at(typeIndex);
+        const QmlEventType &type = eventTypes.at(typeIndex);
         displayName = nameForType(type.rangeType());
 
         const QChar ellipsisChar(0x2026);
@@ -215,7 +217,7 @@ QVariant QmlProfilerStatisticsModel::dataForMainEntry(const QModelIndex &index,
     case FilterRole:
         return m_rootDuration > 0 ? "+" : "-";
     case TypeIdRole:
-        return -1;
+        return s_mainEntryTypeId;
     case Qt::TextColorRole:
         return Utils::creatorTheme()->color(Utils::Theme::Timeline_TextColor);
     case SortRole:
@@ -400,11 +402,11 @@ void QmlProfilerStatisticsModel::notesChanged(int typeIndex)
 {
     static const QVector<int> noteRoles({Qt::ToolTipRole, Qt::TextColorRole});
     const QmlProfilerNotesModel *notesModel = m_modelManager->notesModel();
-    if (typeIndex == -1) {
+    if (typeIndex == s_invalidTypeId) {
         m_notes.clear();
         for (int noteId = 0; noteId < notesModel->count(); ++noteId) {
             int noteType = notesModel->typeId(noteId);
-            if (noteType != -1) {
+            if (noteType != s_invalidTypeId) {
                 QString &note = m_notes[noteType];
                 if (note.isEmpty()) {
                     note = notesModel->text(noteId);
@@ -520,7 +522,8 @@ void QmlProfilerStatisticsRelativesModel::loadEvent(RangeType type, const QmlEve
         stack.push(Frame(event.timestamp(), event.typeIndex()));
         break;
     case RangeEnd: {
-        int callerTypeIndex = stack.count() > 1 ? stack[stack.count() - 2].typeId : -1;
+        int callerTypeIndex = stack.count() > 1 ? stack[stack.count() - 2].typeId
+                                                : QmlProfilerStatisticsModel::s_mainEntryTypeId;
         int relativeTypeIndex = (m_relation == QmlProfilerStatisticsCallers) ? callerTypeIndex :
                                                                                event.typeIndex();
         int selfTypeIndex = (m_relation == QmlProfilerStatisticsCallers) ? event.typeIndex() :
@@ -547,7 +550,12 @@ void QmlProfilerStatisticsRelativesModel::loadEvent(RangeType type, const QmlEve
 
 int QmlProfilerStatisticsRelativesModel::rowCount(const QModelIndex &parent) const
 {
-    return parent.isValid() ? 0 : m_data[m_relativeTypeIndex].count();
+    if (parent.isValid()) {
+        return 0;
+    } else {
+        auto it = m_data.constFind(m_relativeTypeIndex);
+        return it == m_data.constEnd() ? 0 : it->count();
+    }
 }
 
 int QmlProfilerStatisticsRelativesModel::columnCount(const QModelIndex &parent) const
@@ -560,7 +568,7 @@ QVariant QmlProfilerStatisticsRelativesModel::dataForMainEntry(qint64 totalDurat
 {
     switch (role) {
     case TypeIdRole:
-        return -1;
+        return QmlProfilerStatisticsModel::s_mainEntryTypeId;
     case Qt::TextColorRole:
         return Utils::creatorTheme()->color(Utils::Theme::Timeline_TextColor);
     case SortRole:
@@ -593,11 +601,15 @@ QVariant QmlProfilerStatisticsRelativesModel::data(const QModelIndex &index, int
     QTC_ASSERT(row >= 0 && row < data.length(), return QVariant());
 
     const QmlStatisticsRelativesData &stats = data.at(row);
+    QTC_ASSERT(stats.typeIndex >= 0, return QVariant());
 
-    if (stats.typeIndex < 0)
+    const QVector<QmlEventType> &eventTypes = m_modelManager->eventTypes();
+
+    if (stats.typeIndex == QmlProfilerStatisticsModel::s_mainEntryTypeId)
         return dataForMainEntry(stats.duration, role, index.column());
 
-    const QmlEventType &type = m_modelManager->eventTypes().at(stats.typeIndex);
+    QTC_ASSERT(stats.typeIndex < eventTypes.size(), return QVariant());
+    const QmlEventType &type = eventTypes.at(stats.typeIndex);
 
     switch (role) {
     case TypeIdRole:
@@ -688,7 +700,7 @@ bool QmlProfilerStatisticsRelativesModel::setData(const QModelIndex &index, cons
 void QmlProfilerStatisticsRelativesModel::clear()
 {
     beginResetModel();
-    m_relativeTypeIndex = std::numeric_limits<int>::max();
+    m_relativeTypeIndex = QmlProfilerStatisticsModel::s_invalidTypeId;
     m_data.clear();
     m_callStack.clear();
     m_compileStack.clear();

src/plugins/qmlprofiler/qmlprofilerstatisticsmodel.h

@@ -145,6 +145,9 @@ public:
     QVariant data(const QModelIndex &index, int role) const override;
     QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
 
+    static const int s_mainEntryTypeId = std::numeric_limits<int>::max();
+    static const int s_invalidTypeId = -1;
+
 private:
     void loadEvent(const QmlEvent &event, const QmlEventType &type);
     void finalize();
@@ -178,7 +181,8 @@ class QmlProfilerStatisticsRelativesModel : public QAbstractTableModel
 public:
 
     struct QmlStatisticsRelativesData {
-        QmlStatisticsRelativesData(qint64 duration = 0, qint64 calls = 0, int typeIndex = -1,
+        QmlStatisticsRelativesData(qint64 duration = 0, qint64 calls = 0,
+                                   int typeIndex = QmlProfilerStatisticsModel::s_invalidTypeId,
                                    bool isRecursive = false)
             : duration(duration), calls(calls), typeIndex(typeIndex), isRecursive(isRecursive) {}
         qint64 duration;
@@ -207,10 +211,11 @@ private:
     QHash<int, QVector<QmlStatisticsRelativesData>> m_data;
     QPointer<QmlProfilerModelManager> m_modelManager;
 
-    int m_relativeTypeIndex = std::numeric_limits<int>::max();
+    int m_relativeTypeIndex = QmlProfilerStatisticsModel::s_invalidTypeId;
 
     struct Frame {
-        Frame(qint64 startTime = 0, int typeId = -1) : startTime(startTime), typeId(typeId) {}
+        Frame(qint64 startTime = 0, int typeId = QmlProfilerStatisticsModel::s_invalidTypeId)
+            : startTime(startTime), typeId(typeId) {}
         qint64 startTime;
         int typeId;
     };

src/plugins/qmlprofiler/qmlprofilerstatisticsview.cpp

@@ -74,8 +74,14 @@ QmlProfilerStatisticsView::QmlProfilerStatisticsView(QmlProfilerModelManager *pr
     m_mainView.reset(new QmlProfilerStatisticsMainView(model));
     connect(m_mainView.get(), &QmlProfilerStatisticsMainView::gotoSourceLocation,
             this, &QmlProfilerStatisticsView::gotoSourceLocation);
-    connect(m_mainView.get(), &QmlProfilerStatisticsMainView::typeSelected,
+
-            this, &QmlProfilerStatisticsView::typeSelected);
+    connect(m_mainView.get(), &QmlProfilerStatisticsMainView::typeClicked,
+            this, [this, profilerModelManager](int typeIndex) {
+        // Statistics view has an extra type for "whole program". Translate that into "invalid" for
+        // others.
+        emit typeSelected((typeIndex < profilerModelManager->eventTypes().count())
+                          ? typeIndex : QmlProfilerStatisticsModel::s_invalidTypeId);
+    });
 
     m_calleesView.reset(new QmlProfilerStatisticsRelativesView(
                 new QmlProfilerStatisticsRelativesModel(profilerModelManager, model,
@@ -83,18 +89,14 @@ QmlProfilerStatisticsView::QmlProfilerStatisticsView(QmlProfilerModelManager *pr
     m_callersView.reset(new QmlProfilerStatisticsRelativesView(
                 new QmlProfilerStatisticsRelativesModel(profilerModelManager, model,
                                                         QmlProfilerStatisticsCallers)));
-    connect(m_mainView.get(), &QmlProfilerStatisticsMainView::typeSelected,
-            m_calleesView.get(), &QmlProfilerStatisticsRelativesView::displayType);
-    connect(m_mainView.get(), &QmlProfilerStatisticsMainView::typeSelected,
-            m_callersView.get(), &QmlProfilerStatisticsRelativesView::displayType);
     connect(m_calleesView.get(), &QmlProfilerStatisticsRelativesView::typeClicked,
-            m_mainView.get(), &QmlProfilerStatisticsMainView::selectType);
+            m_mainView.get(), &QmlProfilerStatisticsMainView::jumpToItem);
     connect(m_callersView.get(), &QmlProfilerStatisticsRelativesView::typeClicked,
-            m_mainView.get(), &QmlProfilerStatisticsMainView::selectType);
+            m_mainView.get(), &QmlProfilerStatisticsMainView::jumpToItem);
-    connect(m_calleesView.get(), &QmlProfilerStatisticsRelativesView::gotoSourceLocation,
+    connect(m_mainView.get(), &QmlProfilerStatisticsMainView::propagateTypeIndex,
-            this, &QmlProfilerStatisticsView::gotoSourceLocation);
+            m_calleesView.get(), &QmlProfilerStatisticsRelativesView::displayType);
-    connect(m_callersView.get(), &QmlProfilerStatisticsRelativesView::gotoSourceLocation,
+    connect(m_mainView.get(), &QmlProfilerStatisticsMainView::propagateTypeIndex,
-            this, &QmlProfilerStatisticsView::gotoSourceLocation);
+            m_callersView.get(), &QmlProfilerStatisticsRelativesView::displayType);
 
     // widget arrangement
     QVBoxLayout *groupLayout = new QVBoxLayout;
@@ -180,8 +182,13 @@ bool QmlProfilerStatisticsView::mouseOnTable(const QPoint &position) const
 
 void QmlProfilerStatisticsView::selectByTypeId(int typeIndex)
 {
-    if (m_mainView->selectedTypeId() != typeIndex)
+    // Other models cannot discern between "nothing" and "Main Program". So don't propagate invalid
-        m_mainView->selectType(typeIndex);
+    // typeId, if we already have whole program selected.
+    if (typeIndex >= 0
+            || m_mainView->currentIndex().data(TypeIdRole).toInt()
+            != QmlProfilerStatisticsModel::s_mainEntryTypeId) {
+        m_mainView->displayTypeIndex(typeIndex);
+    }
 }
 
 void QmlProfilerStatisticsView::onVisibleFeaturesChanged(quint64 features)
@@ -205,7 +212,9 @@ QmlProfilerStatisticsMainView::QmlProfilerStatisticsMainView(QmlProfilerStatisti
 
     setModel(sortModel);
 
-    connect(this, &QAbstractItemView::activated, this, &QmlProfilerStatisticsMainView::jumpToItem);
+    connect(this, &QAbstractItemView::activated, this, [this](const QModelIndex &index) {
+        jumpToItem(index.data(TypeIdRole).toInt());
+    });
 
     setSortingEnabled(true);
     sortByColumn(DEFAULT_SORT_COLUMN, Qt::DescendingOrder);
@@ -261,43 +270,41 @@ QStringList QmlProfilerStatisticsMainView::details(int typeId) const
     return m_model->details(typeId);
 }
 
-int QmlProfilerStatisticsMainView::selectedTypeId() const
+void QmlProfilerStatisticsMainView::jumpToItem(int typeIndex)
 {
-    return model()->data(selectedModelIndex(), TypeIdRole).toInt();
+    displayTypeIndex(typeIndex);
-}
 
-void QmlProfilerStatisticsMainView::jumpToItem(const QModelIndex &index)
-{
     // show in editor
-    getSourceLocation(index, [this](const QString &fileName, int line, int column) {
+    getSourceLocation(model()->index(typeIndex, MainLocation),
+                      [this](const QString &fileName, int line, int column) {
         emit gotoSourceLocation(fileName, line, column);
     });
 
-    // show in callers/callees subwindow
+    emit typeClicked(typeIndex);
-    emit typeSelected(model()->data(index, TypeIdRole).toInt());
 }
 
-void QmlProfilerStatisticsMainView::selectItem(const QModelIndex &index)
+void QmlProfilerStatisticsMainView::displayTypeIndex(int typeIndex)
 {
-    // If the same item is already selected, don't reselect it.
+    if (typeIndex < 0) {
-    if (index != currentIndex()) {
+        setCurrentIndex(QModelIndex());
-        setCurrentIndex(index);
+    } else {
+        QSortFilterProxyModel *sortModel = qobject_cast<QSortFilterProxyModel *>(model());
+        QTC_ASSERT(sortModel, return);
+
+        QAbstractItemModel *sourceModel = sortModel->sourceModel();
+        QTC_ASSERT(sourceModel, return);
+
+        QModelIndex sourceIndex = sourceModel->index(qMin(typeIndex, sourceModel->rowCount() - 1),
+                                                     MainCallCount);
+        QTC_ASSERT(sourceIndex.data(TypeIdRole).toInt() == typeIndex, return);
+
+        setCurrentIndex(sourceIndex.data(SortRole).toInt() > 0
+                        ? sortModel->mapFromSource(sourceIndex)
+                        : QModelIndex());
+    }
 
     // show in callers/callees subwindow
-        emit typeSelected(index.data(TypeIdRole).toInt());
+    emit propagateTypeIndex(typeIndex);
-    }
-}
-
-void QmlProfilerStatisticsMainView::selectType(int typeIndex)
-{
-    QAbstractItemModel *itemModel = model();
-    for (int i = 0; i < itemModel->rowCount(); i++) {
-        QModelIndex index = itemModel->index(i, MainLocation);
-        if (itemModel->data(index, TypeIdRole).toInt() == typeIndex) {
-            selectItem(index);
-            return;
-        }
-    }
 }
 
 QModelIndex QmlProfilerStatisticsMainView::selectedModelIndex() const
@@ -374,8 +381,9 @@ QmlProfilerStatisticsRelativesView::QmlProfilerStatisticsRelativesView(
     setSortingEnabled(true);
     sortByColumn(DEFAULT_SORT_COLUMN, Qt::DescendingOrder);
 
-    connect(this, &QAbstractItemView::activated,
+    connect(this, &QAbstractItemView::activated, this, [this](const QModelIndex &index) {
-            this, &QmlProfilerStatisticsRelativesView::jumpToItem);
+        jumpToItem(index.data(TypeIdRole).toInt());
+    });
 }
 
 QmlProfilerStatisticsRelativesView::~QmlProfilerStatisticsRelativesView()
@@ -388,14 +396,9 @@ void QmlProfilerStatisticsRelativesView::displayType(int typeIndex)
     resizeColumnToContents(RelativeLocation);
 }
 
-void QmlProfilerStatisticsRelativesView::jumpToItem(const QModelIndex &index)
+void QmlProfilerStatisticsRelativesView::jumpToItem(int typeIndex)
 {
-    // show in editor
+    emit typeClicked(typeIndex);
-    getSourceLocation(index, [this](const QString &fileName, int line, int column) {
-        emit gotoSourceLocation(fileName, line, column);
-    });
-
-    emit typeClicked(index.data(TypeIdRole).toInt());
 }
 
 } // namespace Internal

src/plugins/qmlprofiler/qmlprofilerstatisticsview.h

@@ -81,13 +81,11 @@ public:
     void copyTableToClipboard() const;
     void copyRowToClipboard() const;
 
-    int selectedTypeId() const;
-
     void setShowExtendedStatistics(bool);
     bool showExtendedStatistics() const;
 
-    void jumpToItem(const QModelIndex &index);
+    void displayTypeIndex(int typeIndex);
-    void selectType(int typeIndex);
+    void jumpToItem(int typeIndex);
 
     void restrictToFeatures(quint64 features);
     bool isRestrictedToRange() const;
@@ -97,10 +95,10 @@ public:
 
 signals:
     void gotoSourceLocation(const QString &fileName, int lineNumber, int columnNumber);
-    void typeSelected(int typeIndex);
+    void typeClicked(int typeIndex);
+    void propagateTypeIndex(int typeIndex);
 
 private:
-    void selectItem(const QModelIndex &index);
     QString textForItem(const QModelIndex &index) const;
 
     std::unique_ptr<QmlProfilerStatisticsModel> m_model;
@@ -115,11 +113,10 @@ public:
     ~QmlProfilerStatisticsRelativesView();
 
     void displayType(int typeIndex);
-    void jumpToItem(const QModelIndex &);
+    void jumpToItem(int typeIndex);
 
 signals:
     void typeClicked(int typeIndex);
-    void gotoSourceLocation(const QString &fileName, int lineNumber, int columnNumber);
 
 private: