Tracing: Handle mouse events in FlameGraph QQuickItem

Having an additional MouseArea as child of a ScrollView or a Flickable
is not well defined and leads to inconsistent behavior on different
systems. We can easily catch the relevant events in the FlameGraph item
itself. Also, don't redirect the typeSelected() signals through the
model. They don't belong there.

Change-Id: I77c17977b5a51d57ccd2ef880d3d6c6a604b7f78
Task-number: QTCREATORBUG-20573
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/libs/tracing/flamegraph.cpp

@@ -30,6 +30,8 @@ namespace FlameGraph {
 FlameGraph::FlameGraph(QQuickItem *parent) :
     QQuickItem(parent)
 {
+    setAcceptedMouseButtons(Qt::LeftButton);
+
     // Queue the rebuild in this case so that a delegate can set the root without getting deleted
     // during the call.
     connect(this, &FlameGraph::rootChanged, this, &FlameGraph::rebuild, Qt::QueuedConnection);
@@ -130,7 +132,6 @@ QObject *FlameGraph::appendChild(QObject *parentObject, QQuickItem *parentItem,
     return childObject;
 }
 
-
 int FlameGraph::buildNode(const QModelIndex &parentIndex, QObject *parentObject, int depth,
                           int maximumDepth)
 {
@@ -194,4 +195,22 @@ void FlameGraph::rebuild()
     emit depthChanged(m_depth);
 }
 
+void FlameGraph::mousePressEvent(QMouseEvent *event)
+{
+    Q_UNUSED(event);
+}
+
+void FlameGraph::mouseReleaseEvent(QMouseEvent *event)
+{
+    Q_UNUSED(event);
+    setSelectedTypeId(-1);
+}
+
+void FlameGraph::mouseDoubleClickEvent(QMouseEvent *event)
+{
+    Q_UNUSED(event);
+    setSelectedTypeId(-1);
+    resetRoot();
+}
+
 } // namespace FlameGraph

src/libs/tracing/flamegraph.h

@@ -45,6 +45,8 @@ class TRACING_EXPORT FlameGraph : public QQuickItem
                NOTIFY maximumDepthChanged)
     Q_PROPERTY(int depth READ depth NOTIFY depthChanged)
     Q_PROPERTY(QPersistentModelIndex root READ root WRITE setRoot NOTIFY rootChanged)
+    Q_PROPERTY(int selectedTypeId READ selectedTypeId WRITE setSelectedTypeId
+               NOTIFY selectedTypeIdChanged)
 
 public:
     FlameGraph(QQuickItem *parent = nullptr);
@@ -101,6 +103,20 @@ public:
 
     static FlameGraphAttached *qmlAttachedProperties(QObject *object);
 
+    int selectedTypeId() const
+    {
+        return m_selectedTypeId;
+    }
+
+    void setSelectedTypeId(int selectedTypeId)
+    {
+        if (m_selectedTypeId == selectedTypeId)
+            return;
+
+        m_selectedTypeId = selectedTypeId;
+        emit selectedTypeIdChanged(m_selectedTypeId);
+    }
+
 signals:
     void delegateChanged(QQmlComponent *delegate);
     void modelChanged(QAbstractItemModel *model);
@@ -109,6 +125,7 @@ signals:
     void depthChanged(int depth);
     void maximumDepthChanged();
     void rootChanged(const QPersistentModelIndex &root);
+    void selectedTypeIdChanged(int selectedTypeId);
 
 private:
     void rebuild();
@@ -120,11 +137,16 @@ private:
     int m_depth = 0;
     qreal m_sizeThreshold = 0;
     int m_maximumDepth = std::numeric_limits<int>::max();
+    int m_selectedTypeId = -1;
 
     int buildNode(const QModelIndex &parentIndex, QObject *parentObject, int depth,
                   int maximumDepth);
     QObject *appendChild(QObject *parentObject, QQuickItem *parentItem, QQmlContext *context,
                          const QModelIndex &childIndex, qreal position, qreal size);
+
+    void mousePressEvent(QMouseEvent *event) final;
+    void mouseReleaseEvent(QMouseEvent *event) final;
+    void mouseDoubleClickEvent(QMouseEvent *event) final;
 };
 
 } // namespace FlameGraph

src/libs/tracing/qml/FlameGraphView.qml

@@ -32,7 +32,16 @@ import QtQuick.Controls 1.3
 
 ScrollView {
     id: root
+
     property int selectedTypeId: -1
+    signal typeSelected(int typeId)
+
+    onSelectedTypeIdChanged: {
+        // selectedTypeId can be set from outside. Don't send typeSelected() from here.
+        tooltip.hoveredNode = null;
+        flamegraph.selectedTypeId = selectedTypeId;
+    }
+
     property int sizeRole: -1
     property var model: null
 
@@ -117,25 +126,6 @@ ScrollView {
         }
     }
 
-    onSelectedTypeIdChanged: tooltip.hoveredNode = null
-
-    MouseArea {
-        anchors.fill: parent
-        onClicked: {
-            selectedTypeId = -1;
-            tooltip.selectedNode = null;
-            if (model !== null)
-                model.typeSelected(-1);
-        }
-        onDoubleClicked: {
-            selectedTypeId = -1;
-            tooltip.selectedNode = null;
-            if (model !== null)
-                model.typeSelected(-1);
-            flamegraph.resetRoot();
-        }
-    }
-
     Flickable {
         id: flickable
         contentHeight: flamegraph.height
@@ -158,6 +148,14 @@ ScrollView {
             maximumDepth: 128
             y: flickable.height > height ? flickable.height - height : 0
 
+            onSelectedTypeIdChanged: {
+                if (selectedTypeId !== root.selectedTypeId) {
+                    // Change originates from inside. Send typeSelected().
+                    root.selectedTypeId = selectedTypeId;
+                    root.typeSelected(selectedTypeId);
+                }
+            }
+
             delegate: FlameGraphDelegate {
                 id: flamegraphItem
 
@@ -165,7 +163,7 @@ ScrollView {
                 property bool isHighlighted: root.isHighlighted(flamegraphItem)
 
                 itemHeight: flamegraph.delegateHeight
-                isSelected: typeId !== -1 && typeId === root.selectedTypeId
+                isSelected: typeId !== -1 && typeId === flamegraph.selectedTypeId
 
                 borderColor: {
                     if (isSelected)
@@ -189,7 +187,7 @@ ScrollView {
 
                 onIsSelectedChanged: {
                     if (isSelected && (tooltip.selectedNode === null ||
-                            tooltip.selectedNode.typeId !== root.selectedTypeId)) {
+                            tooltip.selectedNode.typeId !== flamegraph.selectedTypeId)) {
                         tooltip.selectedNode = flamegraphItem;
                     } else if (!isSelected && tooltip.selectedNode === flamegraphItem) {
                         tooltip.selectedNode = null;
@@ -222,7 +220,7 @@ ScrollView {
                     if (tooltip.hoveredNode === flamegraphItem) {
                         // Keep the window around until something else is hovered or selected.
                         if (tooltip.selectedNode === null
-                                || tooltip.selectedNode.typeId !== root.selectedTypeId) {
+                                || tooltip.selectedNode.typeId !== flamegraph.selectedTypeId) {
                             tooltip.selectedNode = flamegraphItem;
                         }
                         tooltip.hoveredNode = null;
@@ -232,8 +230,7 @@ ScrollView {
                 function selectClicked() {
                     if (FlameGraph.dataValid) {
                         tooltip.selectedNode = flamegraphItem;
-                        selectedTypeId = FlameGraph.data(root.typeIdRole);
-                        model.typeSelected(selectedTypeId);
+                        flamegraph.selectedTypeId = FlameGraph.data(root.typeIdRole);
                         model.gotoSourceLocation(
                                     FlameGraph.data(root.sourceFileRole),
                                     FlameGraph.data(root.sourceLineRole),
@@ -246,6 +243,7 @@ ScrollView {
                     tooltip.selectedNode = null;
                     tooltip.hoveredNode = null;
                     flamegraph.root = FlameGraph.modelIndex;
+                    selectClicked();
                 }
 
                 // Functions, not properties to limit the initial overhead when creating the nodes,
@@ -288,9 +286,8 @@ ScrollView {
             }
 
             onClearSelection: {
-                selectedTypeId = -1;
+                flamegraph.selectedTypeId = -1;
                 selectedNode = null;
-                root.model.typeSelected(-1);
             }
 
             dialogTitle: {

src/plugins/qmlprofiler/flamegraphmodel.h

@@ -95,7 +95,6 @@ public:
 
 signals:
     void gotoSourceLocation(const QString &fileName, int lineNumber, int columnNumber);
-    void typeSelected(int typeIndex);
 
 private:
     QVariant lookup(const FlameGraphData &data, int role) const;

src/plugins/qmlprofiler/flamegraphview.cpp

@@ -67,7 +67,7 @@ FlameGraphView::FlameGraphView(QmlProfilerModelManager *manager, QWidget *parent
     layout->addWidget(m_content);
     setLayout(layout);
 
-    connect(m_model, &FlameGraphModel::typeSelected, this, &FlameGraphView::typeSelected);
+    connect(m_content->rootObject(), SIGNAL(typeSelected(int)), this, SIGNAL(typeSelected(int)));
     connect(m_model, &FlameGraphModel::gotoSourceLocation,
             this, &FlameGraphView::gotoSourceLocation);
 }

tests/auto/tracing/flamegraphview/TestFlameGraphView.qml

@@ -0,0 +1,63 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+import TestFlameGraphModel 1.0
+import "../tracing/"
+
+FlameGraphView {
+    id: root
+    sizeRole: TestFlameGraphModel.SizeRole
+
+    model: flameGraphModel
+
+    typeIdRole: TestFlameGraphModel.TypeIdRole
+    sourceFileRole: TestFlameGraphModel.SourceFileRole
+    sourceLineRole: TestFlameGraphModel.SourceLineRole
+    sourceColumnRole: TestFlameGraphModel.SourceColumnRole
+    detailsTitleRole: TestFlameGraphModel.DetailsTitleRole
+    summaryRole: TestFlameGraphModel.SummaryRole
+
+    modes: [
+        TestFlameGraphModel.SizeRole,
+        TestFlameGraphModel.SourceLineRole,
+        TestFlameGraphModel.SourceColumnRole,
+    ]
+
+    trRoleNames: [
+        TestFlameGraphModel.SizeRole,         qsTr("Size"),
+        TestFlameGraphModel.SourceFileRole,   qsTr("Source File"),
+        TestFlameGraphModel.SourceLineRole,   qsTr("Source Line"),
+        TestFlameGraphModel.SourceColumnRole, qsTr("Source Column"),
+    ].reduce(toMap, {})
+
+    details: function(flameGraph) {
+        var model = [];
+        root.addDetail(TestFlameGraphModel.SizeRole, detailFormats.noop,
+                       model, flameGraph);
+        root.addDetail(TestFlameGraphModel.SourceFileRole, detailFormats.addLine,
+                       model, flameGraph);
+        return model;
+    }
+}

tests/auto/tracing/flamegraphview/flamegraphview.pro

@@ -0,0 +1,10 @@
+QT += quick quickwidgets
+
+QTC_LIB_DEPENDS += tracing
+include(../../qttest.pri)
+
+SOURCES += \
+    tst_flamegraphview.cpp
+
+RESOURCES += \
+    flamegraphview.qrc

tests/auto/tracing/flamegraphview/flamegraphview.qbs

@@ -0,0 +1,15 @@
+import qbs
+import "../tracingautotest.qbs" as TracingAutotest
+
+TracingAutotest {
+    name: "FlameGraphView autotest"
+
+    Depends { name: "Qt.quickwidgets" }
+
+    Group {
+        name: "Test sources"
+        files: [
+            "tst_flamegraphview.cpp", "flamegraphview.qrc"
+        ]
+    }
+}

tests/auto/tracing/flamegraphview/flamegraphview.qrc

@@ -0,0 +1,5 @@
+<RCC>
+    <qresource prefix="/tracingtest">
+        <file>TestFlameGraphView.qml</file>
+    </qresource>
+</RCC>

tests/auto/tracing/flamegraphview/tst_flamegraphview.cpp

@@ -0,0 +1,176 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#include <tracing/flamegraph.h>
+#include <tracing/timelinetheme.h>
+#include <utils/theme/theme_p.h>
+
+#include <QObject>
+#include <QStandardItemModel>
+#include <QQmlContext>
+#include <QQuickWidget>
+#include <QtTest>
+
+class TestFlameGraphModel : public QStandardItemModel
+{
+    Q_OBJECT
+    Q_ENUMS(Role)
+public:
+    enum Role {
+        TypeIdRole = Qt::UserRole + 1,
+        SizeRole,
+        SourceFileRole,
+        SourceLineRole,
+        SourceColumnRole,
+        DetailsTitleRole,
+        SummaryRole,
+        MaxRole
+    };
+
+    void fill() {
+        qreal sizeSum = 0;
+        for (int i = 1; i < 10; ++i) {
+            QStandardItem *item = new QStandardItem;
+            item->setData(i, SizeRole);
+            item->setData(100 / i, TypeIdRole);
+            item->setData("trara", SourceFileRole);
+            item->setData(20, SourceLineRole);
+            item->setData(10, SourceColumnRole);
+            item->setData("details", DetailsTitleRole);
+            item->setData("summary", SummaryRole);
+
+            for (int j = 1; j < i; ++j) {
+                QStandardItem *item2 = new QStandardItem;
+                item2->setData(1, SizeRole);
+                item2->setData(100 / j, TypeIdRole);
+                item2->setData(1, SourceLineRole);
+                item2->setData("child", DetailsTitleRole);
+                item2->setData("childsummary", SummaryRole);
+                for (int k = 1; k < 10; ++k) {
+                    QStandardItem *skipped = new QStandardItem;
+                    skipped->setData(0.001, SizeRole);
+                    skipped->setData(100 / k, TypeIdRole);
+                    item2->appendRow(skipped);
+                }
+                item->appendRow(item2);
+            }
+
+            appendRow(item);
+            sizeSum += i;
+        }
+        invisibleRootItem()->setData(sizeSum, SizeRole);
+        invisibleRootItem()->setData(9 * 20, SourceLineRole);
+        invisibleRootItem()->setData(9 * 10, SourceColumnRole);
+    }
+
+    Q_INVOKABLE void gotoSourceLocation(const QString &file, int line, int column)
+    {
+        Q_UNUSED(file);
+        Q_UNUSED(line);
+        Q_UNUSED(column);
+    }
+};
+
+class DummyTheme : public Utils::Theme
+{
+public:
+    DummyTheme() : Utils::Theme(QLatin1String("dummy"))
+    {
+        for (int i = 0; i < d->colors.count(); ++i) {
+            d->colors[i] = QPair<QColor, QString>(
+                        QColor::fromRgb(qrand() % 256, qrand() % 256, qrand() % 256),
+                        QString::number(i));
+        }
+    }
+};
+
+class tst_FlameGraphView : public QObject
+{
+    Q_OBJECT
+public:
+    tst_FlameGraphView() { Utils::setCreatorTheme(&theme); }
+
+private slots:
+    void initTestCase();
+    void testZoom();
+
+private:
+    static const int sizeRole = Qt::UserRole + 1;
+    static const int dataRole = Qt::UserRole + 2;
+    TestFlameGraphModel model;
+    QQuickWidget widget;
+    DummyTheme theme;
+};
+
+void tst_FlameGraphView::initTestCase()
+{
+    model.fill();
+    qmlRegisterType<FlameGraph::FlameGraph>("FlameGraph", 1, 0, "FlameGraph");
+    qmlRegisterUncreatableType<TestFlameGraphModel>(
+                "TestFlameGraphModel", 1, 0, "TestFlameGraphModel",
+                QLatin1String("use the context property"));
+
+
+    Timeline::TimelineTheme::setupTheme(widget.engine());
+
+    widget.rootContext()->setContextProperty(QStringLiteral("flameGraphModel"), &model);
+    widget.setSource(QUrl(QStringLiteral("qrc:/tracingtest/TestFlameGraphView.qml")));
+
+    widget.setResizeMode(QQuickWidget::SizeRootObjectToView);
+    widget.setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
+    widget.resize(800, 600);
+
+    widget.show();
+}
+
+void tst_FlameGraphView::testZoom()
+{
+    auto selectedTypeId = [&]() {
+        return widget.rootObject()->property("selectedTypeId").toInt();
+    };
+
+    QWindow *window = widget.windowHandle();
+
+    QCOMPARE(selectedTypeId(), -1);
+    QTest::mouseClick(window, Qt::LeftButton, Qt::NoModifier, QPoint(widget.width() - 5,
+                                                                     widget.height() - 5));
+    QTRY_VERIFY(selectedTypeId() != -1);
+    const int typeId1 = selectedTypeId();
+
+    QTest::mouseDClick(window, Qt::LeftButton, Qt::NoModifier, QPoint(5, widget.height() - 5));
+    QTRY_VERIFY(selectedTypeId() != typeId1);
+    QVERIFY(selectedTypeId() != -1);
+
+    QTest::mouseDClick(window, Qt::LeftButton, Qt::NoModifier, QPoint(widget.width() / 2,
+                                                                      widget.height() / 2));
+    QTRY_COMPARE(selectedTypeId(), -1);
+    QTest::mouseClick(window, Qt::LeftButton, Qt::NoModifier, QPoint(widget.width() - 5,
+                                                                     widget.height() - 5));
+    QTRY_COMPARE(selectedTypeId(), typeId1);
+}
+
+QTEST_MAIN(tst_FlameGraphView)
+
+#include "tst_flamegraphview.moc"

tests/auto/tracing/tracing.pro

@@ -3,6 +3,7 @@ TEMPLATE = subdirs
 
 SUBDIRS = \
     flamegraph \
+    flamegraphview \
     timelineabstractrenderer \
     timelineitemsrenderpass \
     timelinemodel \