diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-08-13 12:26:52 +0200 |
---|---|---|
committer | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-08-18 07:17:50 +0200 |
commit | 31ebf45781bdda505adafc76de6117d20069d084 (patch) | |
tree | da1796689943df4efda534f491102aa815b96b01 | |
parent | 2a857ee28315c5bacfe2ecaf402ca9005b35c20e (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.cpp | 2 | ||||
-rw-r--r-- | src/widgets/kernel/qapplication.cpp | 12 | ||||
-rw-r--r-- | tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp | 147 |
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 |