summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVolker Hilsheimer <volker.hilsheimer@qt.io>2021-08-13 12:26:52 +0200
committerVolker Hilsheimer <volker.hilsheimer@qt.io>2021-08-18 07:17:50 +0200
commit31ebf45781bdda505adafc76de6117d20069d084 (patch)
treeda1796689943df4efda534f491102aa815b96b01
parent2a857ee28315c5bacfe2ecaf402ca9005b35c20e (diff)
Don't crash in high-precision wheel scrolls on a QGraphicsProxyWidget
For high-precision wheel scrolling sequences, the widget that gets the first (typically ScrollBegin) event grabs the wheel. Qt directs all future wheel events within the same sequence (i.e. until ScrollEnd) to that widget. QGraphicsView passes wheel events through to the item under the mouse, and QGraphicsProxyWidget implements wheelEvent to forward a synthesized QWheelEvent to the embedded widget. Since QGraphicsView's viewport has already grabbed the wheel, any forwarded event would end up back in QGraphicsView, resulting in infinite recursion (if the assert doesn't fail first in debug builds). The correct fix requires that QGraphicsProxyWidget knows that this is a high-precision wheel event, allowing it to adjust the wheel grabber temporarily to the embedded widget. However, QGraphicsSceneWheelEvent doesn't provide this information. To fix the infinite recursion, mark the generated event as synthesized by Qt (but still send it spontaneously to enable propagarion within the proxy widget hierarchy). In QApplication's notification routine, interpret such events then to override the wheel grabber. Add a test case for the various scenarios. This 6.1 compatible fix does not pass all situations. A follow up commit that introduces the missing APIs to QGraphicsSceneWheelEvent then fixes those as well. Task-number: QTBUG-95552 Change-Id: I78400ceae8da7a4e22a988c06ed58f99f1a979f4 Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io> (cherry picked from commit 4982c872efef7ce8673ed257dce24b971e456a08) Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
-rw-r--r--src/widgets/graphicsview/qgraphicsproxywidget.cpp2
-rw-r--r--src/widgets/kernel/qapplication.cpp12
-rw-r--r--tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp147
3 files changed, 157 insertions, 4 deletions
diff --git a/src/widgets/graphicsview/qgraphicsproxywidget.cpp b/src/widgets/graphicsview/qgraphicsproxywidget.cpp
index fcd675aa76..aaddaa894f 100644
--- a/src/widgets/graphicsview/qgraphicsproxywidget.cpp
+++ b/src/widgets/graphicsview/qgraphicsproxywidget.cpp
@@ -1302,7 +1302,7 @@ void QGraphicsProxyWidget::wheelEvent(QGraphicsSceneWheelEvent *event)
// were not preserved in the QGraphicsSceneWheelEvent unfortunately
QWheelEvent wheelEvent(pos, event->screenPos(), QPoint(), angleDelta,
event->buttons(), event->modifiers(), Qt::NoScrollPhase,
- false, Qt::MouseEventNotSynthesized,
+ false, Qt::MouseEventSynthesizedByQt,
QPointingDevice::primaryPointingDevice());
QPointer<QWidget> focusWidget = d->widget->focusWidget();
extern bool qt_sendSpontaneousEvent(QObject *, QEvent *);
diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp
index 059617eb83..0daa0cfcb6 100644
--- a/src/widgets/kernel/qapplication.cpp
+++ b/src/widgets/kernel/qapplication.cpp
@@ -2949,9 +2949,15 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
// a widget has already grabbed the wheel for a sequence
if (QApplicationPrivate::wheel_widget) {
- Q_ASSERT(phase != Qt::NoScrollPhase);
- w = QApplicationPrivate::wheel_widget;
- relpos = w->mapFromGlobal(wheel->globalPosition().toPoint());
+ // Qt explicitly synthesizes a spontaneous event for the receiver, done
+ // by QGraphicsProxyWidget - so trust it
+ if (wheel->source() == Qt::MouseEventSynthesizedByQt) {
+ QApplicationPrivate::wheel_widget = w;
+ } else {
+ Q_ASSERT(phase != Qt::NoScrollPhase);
+ w = QApplicationPrivate::wheel_widget;
+ relpos = w->mapFromGlobal(wheel->globalPosition().toPoint());
+ }
}
/*
Start or finish a scrolling sequence by grabbing/releasing the wheel via
diff --git a/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp b/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp
index e666ac24c9..bbcef8849e 100644
--- a/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp
+++ b/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp
@@ -198,6 +198,7 @@ private slots:
void sendEvent();
#if QT_CONFIG(wheelevent)
void wheelEvent();
+ void wheelEventPropagation();
#endif
#ifndef QT_NO_CURSOR
void cursor();
@@ -2242,6 +2243,152 @@ void tst_QGraphicsView::wheelEvent()
QCOMPARE(spy.count(), 2);
QVERIFY(widget->hasFocus());
}
+
+/*!
+ QGraphicsProxyWidget receives wheel events from QGraphicsScene, and then
+ generates a new event that is sent spontaneously in order to enable event
+ propagation. This requires extra handling of the wheel grabbing we do for
+ high-precision wheel event streams.
+
+ Test that this doesn't trigger infinite recursion, while still resulting in
+ event propagation within the embedded widget hierarchy, and back to the
+ QGraphicsView if the event is not accepted.
+
+ See tst_QApplication::wheelEventPropagation for a similar test.
+*/
+void tst_QGraphicsView::wheelEventPropagation()
+{
+ QGraphicsScene scene(0, 0, 600, 600);
+
+ QWidget *label = new QLabel("Direct");
+ label->setFixedSize(300, 30);
+ QGraphicsProxyWidget *labelProxy = scene.addWidget(label);
+ labelProxy->setPos(0, 50);
+ labelProxy->show();
+
+ class NestedWidget : public QWidget
+ {
+ public:
+ NestedWidget(const QString &text)
+ {
+ setObjectName("Nested Label");
+ QLabel *nested = new QLabel(text);
+ QHBoxLayout *hbox = new QHBoxLayout;
+ hbox->addWidget(nested);
+ setLayout(hbox);
+ }
+
+ int wheelEventCount = 0;
+ protected:
+ void wheelEvent(QWheelEvent *) override
+ {
+ ++wheelEventCount;
+ }
+ };
+ NestedWidget *nestedWidget = new NestedWidget("Nested");
+ nestedWidget->setFixedSize(300, 60);
+ QGraphicsProxyWidget *nestedProxy = scene.addWidget(nestedWidget);
+ nestedProxy->setPos(0, 120);
+ nestedProxy->show();
+
+ QGraphicsView view(&scene);
+ view.setFixedHeight(200);
+ view.show();
+
+ QVERIFY(QTest::qWaitForWindowActive(&view));
+ QVERIFY(view.verticalScrollBar()->isVisible());
+
+ view.verticalScrollBar()->setValue(0);
+ QSignalSpy scrollSpy(view.verticalScrollBar(), &QScrollBar::valueChanged);
+
+ const QPoint wheelPosition(50, 25);
+ auto wheelUp = [&view, wheelPosition](Qt::ScrollPhase phase) {
+ const QPoint global = view.mapToGlobal(wheelPosition);
+ const QPoint pixelDelta(0, -25);
+ const QPoint angleDelta(0, -120);
+ QWindowSystemInterface::handleWheelEvent(view.windowHandle(), wheelPosition, global,
+ pixelDelta, angleDelta, Qt::NoModifier,
+ phase);
+ QCoreApplication::processEvents();
+ };
+
+ int scrollCount = 0;
+ // test non-kinetic events; they are not grabbed, and should scroll the view unless
+ // accepted by the embedded widget
+ QCOMPARE(view.itemAt(wheelPosition), nullptr);
+ wheelUp(Qt::NoScrollPhase);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+
+ // wheeling on the label, which ignores the event, should scroll the view
+ QCOMPARE(view.itemAt(wheelPosition), labelProxy);
+ wheelUp(Qt::NoScrollPhase);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+ QCOMPARE(view.itemAt(wheelPosition), labelProxy);
+ wheelUp(Qt::NoScrollPhase);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+
+ // left the widget
+ QCOMPARE(view.itemAt(wheelPosition), nullptr);
+ wheelUp(Qt::NoScrollPhase);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+
+ // reached the nested widget, which accepts the wheel event, so no more scrolling
+ QCOMPARE(view.itemAt(wheelPosition), nestedProxy);
+ // remember this position for later
+ const int scrollBarValueOnNestedProxy = view.verticalScrollBar()->value();
+ wheelUp(Qt::NoScrollPhase);
+ QCOMPARE(scrollSpy.count(), scrollCount);
+ QCOMPARE(nestedWidget->wheelEventCount, 1);
+
+ // reset, try with kinetic events
+ view.verticalScrollBar()->setValue(0);
+ ++scrollCount;
+
+ // starting a scroll outside any widget and scrolling through the widgets should work,
+ // no matter if the widget accepts wheel events - the view has the grab
+ QCOMPARE(view.itemAt(wheelPosition), nullptr);
+ wheelUp(Qt::ScrollBegin);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+ for (int i = 0; i < 5; ++i) {
+ wheelUp(Qt::ScrollUpdate);
+ if (i >= 1)
+ QEXPECT_FAIL("", "Fixed for Qt 6.2 - QTBUG-65552", Continue);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+ }
+ wheelUp(Qt::ScrollEnd);
+ QEXPECT_FAIL("", "Fixed for Qt 6.2 - QTBUG-65552", Continue);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+
+ // reset
+ view.verticalScrollBar()->setValue(0);
+ scrollCount = scrollSpy.count();
+
+ // starting a scroll on a widget that doesn't accept wheel events
+ // should also scroll the view, which still gets the grab
+ wheelUp(Qt::NoScrollPhase);
+ scrollCount = scrollSpy.count();
+
+ QCOMPARE(view.itemAt(wheelPosition), labelProxy);
+ wheelUp(Qt::ScrollBegin);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+ for (int i = 0; i < 5; ++i) {
+ wheelUp(Qt::ScrollUpdate);
+ QEXPECT_FAIL("", "Fixed for Qt 6.2 - QTBUG-65552", Continue);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+ }
+ wheelUp(Qt::ScrollEnd);
+ QEXPECT_FAIL("", "Fixed for Qt 6.2 - QTBUG-65552", Continue);
+ QCOMPARE(scrollSpy.count(), ++scrollCount);
+
+ // starting a scroll on a widget that does accept wheel events
+ // should not scroll the view
+ view.verticalScrollBar()->setValue(scrollBarValueOnNestedProxy);
+ scrollCount = scrollSpy.count();
+
+ QCOMPARE(view.itemAt(wheelPosition), nestedProxy);
+ wheelUp(Qt::ScrollBegin);
+ QCOMPARE(scrollSpy.count(), scrollCount);
+}
#endif // QT_CONFIG(wheelevent)
#ifndef QT_NO_CURSOR