From 02164b292f002b051f34a88871145415fad94f32 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Sun, 24 Oct 2021 14:31:33 +0200 Subject: [PATCH] Don't use QCursor::pos in QTabBar and fix hover handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relying on QCursor::pos makes tests fragile and prevents multi-seat support. Instead, record the mouse position in the already existing event handling, and use that instead. Styles might use either WA_Hover or enable mouse tracking for the widget to enable hover-effects, so we need to support both. Fix the scenario where a newly inserted tab ends up under the mouse, which was previously not handled correctly (only the case of removing a tab was). Clean up the repaint management when the hovered tab changes; just call update on the old rect, and then later update on the new rect; there's no need to make a copy first, updates are posted and compressed. Add a unit test that makes sure that we paint tabs that should be under the mouse in the hovered state. Since not all styles enable hovering and/or mouse tracking in all cases, use a style sheet for those styles that don't. Change-Id: I7cdbb18e9e04b52651e273680fec87b50cb81e05 Reviewed-by: Qt CI Bot Reviewed-by: Richard Moe Gustavsen Reviewed-by: Jan Arve Sæther --- src/widgets/widgets/qtabbar.cpp | 56 ++++++------- src/widgets/widgets/qtabbar_p.h | 1 + .../widgets/widgets/qtabbar/tst_qtabbar.cpp | 84 +++++++++++++++++++ 3 files changed, 111 insertions(+), 30 deletions(-) diff --git a/src/widgets/widgets/qtabbar.cpp b/src/widgets/widgets/qtabbar.cpp index 8fd5bbfb56b..8d21e7d2572 100644 --- a/src/widgets/widgets/qtabbar.cpp +++ b/src/widgets/widgets/qtabbar.cpp @@ -42,8 +42,6 @@ #include "qabstractitemdelegate.h" #endif #include "qapplication.h" -#include "qbitmap.h" -#include "qcursor.h" #include "qevent.h" #include "qpainter.h" #include "qstyle.h" @@ -1020,6 +1018,11 @@ int QTabBar::insertTab(int index, const QIcon& icon, const QString &text) ++tab->lastTab; } + if (tabAt(d->mousePosition) == index) { + d->hoverIndex = index; + d->hoverRect = tabRect(index); + } + tabInserted(index); d->autoHideTabs(); return index; @@ -1104,16 +1107,15 @@ void QTabBar::removeTab(int index) } d->refresh(); d->autoHideTabs(); - if (!d->hoverRect.isEmpty()) { - for (int i = 0; i < d->tabList.count(); ++i) { - const QRect area = tabRect(i); - if (area.contains(mapFromGlobal(QCursor::pos()))) { - d->hoverIndex = i; - d->hoverRect = area; - break; - } - } + if (d->hoverRect.isValid()) { update(d->hoverRect); + d->hoverIndex = tabAt(d->mousePosition); + if (d->validIndex(d->hoverIndex)) { + d->hoverRect = tabRect(d->hoverIndex); + update(d->hoverRect); + } else { + d->hoverRect = QRect(); + } } tabRemoved(index); } @@ -1696,33 +1698,26 @@ bool QTabBar::event(QEvent *event) case QEvent::HoverMove: case QEvent::HoverEnter: { QHoverEvent *he = static_cast(event); - if (!d->hoverRect.contains(he->position().toPoint())) { - QRect oldHoverRect = d->hoverRect; - bool cursorOverTabs = false; - for (int i = 0; i < d->tabList.count(); ++i) { - QRect area = tabRect(i); - if (area.contains(he->position().toPoint())) { - d->hoverIndex = i; - d->hoverRect = area; - cursorOverTabs = true; - break; - } - } - if (!cursorOverTabs) { - d->hoverIndex = -1; + d->mousePosition = he->position().toPoint(); + if (!d->hoverRect.contains(d->mousePosition)) { + if (d->hoverRect.isValid()) + update(d->hoverRect); + d->hoverIndex = tabAt(d->mousePosition); + if (d->validIndex(d->hoverIndex)) { + d->hoverRect = tabRect(d->hoverIndex); + update(d->hoverRect); + } else { d->hoverRect = QRect(); } - if (he->oldPos() != QPoint(-1, -1)) - update(oldHoverRect); - update(d->hoverRect); } return true; } case QEvent::HoverLeave: { - QRect oldHoverRect = d->hoverRect; + d->mousePosition = {-1, -1}; + if (d->hoverRect.isValid()) + update(d->hoverRect); d->hoverIndex = -1; d->hoverRect = QRect(); - update(oldHoverRect); return true; } #if QT_CONFIG(tooltip) @@ -1796,6 +1791,7 @@ bool QTabBar::event(QEvent *event) case QEvent::MouseButtonPress: case QEvent::MouseButtonRelease: case QEvent::MouseMove: + d->mousePosition = static_cast(event)->position().toPoint(); d->mouseButtons = static_cast(event)->buttons(); break; default: diff --git a/src/widgets/widgets/qtabbar_p.h b/src/widgets/widgets/qtabbar_p.h index eb1339a802e..5135839f89d 100644 --- a/src/widgets/widgets/qtabbar_p.h +++ b/src/widgets/widgets/qtabbar_p.h @@ -101,6 +101,7 @@ public: QRect hoverRect; QPoint dragStartPosition; + QPoint mousePosition = {-1, -1}; QSize iconSize; QToolButton* rightB = nullptr; // right or bottom QToolButton* leftB = nullptr; // left or top diff --git a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp index 9fa85ee8e57..41f1cb5e642 100644 --- a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp +++ b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp @@ -107,6 +107,9 @@ private slots: void currentTabLargeFont(); + void hoverTab_data(); + void hoverTab(); + private: void checkPositions(const TabBar &tabbar, const QList &positions); }; @@ -1033,5 +1036,86 @@ void tst_QTabBar::currentTabLargeFont() QVERIFY(oldTabRects != newTabRects); } +void tst_QTabBar::hoverTab_data() +{ + // Since we still rely on moving the mouse via QCursor::setPos in QTest::mouseMove, + // skip this test if we can't + const QPoint cursorPos = QCursor::pos() + QPoint(10, 10); + QCursor::setPos(cursorPos); + if (!QTest::qWaitFor([cursorPos]{ return QCursor::pos() == cursorPos; }, 500)) + QSKIP("Can't move mouse"); + + QTest::addColumn("documentMode"); + QTest::addRow("normal mode") << true; + QTest::addRow("document mode") << true; +} + +void tst_QTabBar::hoverTab() +{ + QFETCH(bool, documentMode); + QWidget window; + class TabBar : public QTabBar + { + public: + using QTabBar::QTabBar; + void initStyleOption(QStyleOptionTab *option, int tabIndex) const override + { + QTabBar::initStyleOption(option, tabIndex); + styleOptions[tabIndex] = *option; + } + mutable QHash styleOptions; + } tabbar(&window); + + tabbar.setDocumentMode(documentMode); + tabbar.addTab("A"); + tabbar.addTab("B"); + tabbar.addTab("C"); + tabbar.addTab("D"); + + tabbar.move(0,0); + window.setMinimumSize(tabbar.sizeHint()); + tabbar.ensurePolished(); + + // some styles set those flags, some don't. If not, use a style sheet + if (!(tabbar.testAttribute(Qt::WA_Hover) || tabbar.hasMouseTracking())) { + tabbar.setStyleSheet(R"( + QTabBar::tab { background: blue; } + QTabBar::tab::hover { background: yellow; } + QTabBar::tab::selected { background: red; } + )"); + } + + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + QTest::mouseMove(&tabbar, tabbar.tabRect(0).center()); + QTRY_VERIFY(tabbar.styleOptions[0].state & QStyle::State_Selected); + QTRY_COMPARE(tabbar.styleOptions[1].state & QStyle::State_MouseOver, QStyle::State_None); + QTRY_COMPARE(tabbar.styleOptions[2].state & QStyle::State_MouseOver, QStyle::State_None); + QTRY_COMPARE(tabbar.styleOptions[3].state & QStyle::State_MouseOver, QStyle::State_None); + + QTest::mouseMove(&tabbar, tabbar.tabRect(1).center()); + QTRY_COMPARE(tabbar.styleOptions[1].state & QStyle::State_MouseOver, QStyle::State_MouseOver); + QCOMPARE(tabbar.styleOptions[2].state & QStyle::State_MouseOver, QStyle::State_None); + QCOMPARE(tabbar.styleOptions[3].state & QStyle::State_MouseOver, QStyle::State_None); + + QTest::mouseMove(&tabbar, tabbar.tabRect(2).center()); + QTRY_COMPARE(tabbar.styleOptions[2].state & QStyle::State_MouseOver, QStyle::State_MouseOver); + QCOMPARE(tabbar.styleOptions[1].state & QStyle::State_MouseOver, QStyle::State_None); + QCOMPARE(tabbar.styleOptions[3].state & QStyle::State_MouseOver, QStyle::State_None); + + // removing tab 2 lays the tabs out so that they stretch across the + // tab bar; tab 1 is now where the cursor was. What matters is that a + // different tab is now hovered (rather than none). + tabbar.removeTab(2); + QTRY_COMPARE(tabbar.styleOptions[1].state & QStyle::State_MouseOver, QStyle::State_MouseOver); + QCOMPARE(tabbar.styleOptions[2].state & QStyle::State_MouseOver, QStyle::State_None); + + // inserting a tab at index 2 again should paint the new tab hovered + tabbar.insertTab(2, "C2"); + QTRY_COMPARE(tabbar.styleOptions[2].state & QStyle::State_MouseOver, QStyle::State_MouseOver); + QCOMPARE(tabbar.styleOptions[1].state & QStyle::State_MouseOver, QStyle::State_None); +} + QTEST_MAIN(tst_QTabBar) #include "tst_qtabbar.moc"