Skip to content

Commit

Permalink
QApplication: refactor delivery and propagation of wheel events
Browse files Browse the repository at this point in the history 
Handle wheel grabbing via wheel_widget in a single place, and
propagate events in the same way for all (spontaneous) events.

Handle ScrollMomentum the same way as ScrollUpdate to allow
partial sequences.

Fix the incorrect ignoring of wheel events by default; like all
other input events, they are now again accepted by default and
ignored in the default event handler implementation of QWidget.
This way, implementing the handle suffices to accept the event.
Note that QWidget::wheelEvent doesn't need to be changed, as the
event is ignored there today (an oversight of the change made in
f253f4c, perhaps).

This also fixes changing of direction of a wheel event while
the event sequence is grabbed by a widget.

Change-Id: Ia0f03c14dede80322d690ca50d085898a0497dbe
Fixes: QTBUG-67032
Task-number: QTBUG-79102
Reviewed-by: Shawn Rutledge <[email protected]>
  • Loading branch information
@vohi
vohi committed May 13, 2020
1 parent 01b2ea8 commit 92df790
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 77 deletions.
114 changes: 55 additions & 59 deletions src/widgets/kernel/qapplication.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2992,8 +2992,18 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
}

QWheelEvent* wheel = static_cast<QWheelEvent*>(e);
const bool spontaneous = wheel->spontaneous();
if (!wheel->spontaneous()) {
/*
Synthesized events shouldn't propagate, e.g. QScrollArea passes events from the
viewport on to the scrollbars, which might ignore the event if there is no more
space to scroll. If we would propagate, the event would come back to the viewport.
*/
res = d->notify_helper(w, wheel);
break;
}

const Qt::ScrollPhase phase = wheel->phase();
QPoint relpos = wheel->position().toPoint();

// Ideally, we should lock on a widget when it starts receiving wheel
// events. This avoids other widgets to start receiving those events
Expand All @@ -3014,68 +3024,54 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
//
// This means that we can have scrolling sequences (starting with ScrollBegin)
// or partial sequences (after a ScrollEnd and starting with ScrollUpdate).
// If wheel_widget is null because it was deleted, we also take the same
// code path as an initial sequence.
if (phase == Qt::NoScrollPhase || phase == Qt::ScrollBegin || !QApplicationPrivate::wheel_widget) {

// A system-generated ScrollBegin event starts a new user scrolling
// sequence, so we reset wheel_widget in case no one accepts the event
// or if we didn't get (or missed) a ScrollEnd previously.
if (spontaneous && phase == Qt::ScrollBegin)
QApplicationPrivate::wheel_widget = nullptr;

const QPoint relpos = wheel->position().toPoint();
// 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());
}
/*
Start or finish a scrolling sequence by grabbing/releasing the wheel via
wheel_widget. The sequence might be partial (ie. not start with ScrollBegin),
e.g. if the previous wheel_widget was destroyed mid-sequence.
*/
switch (phase) {
case Qt::ScrollEnd:
QApplicationPrivate::wheel_widget = nullptr;
break;
case Qt::ScrollBegin:
QApplicationPrivate::wheel_widget = w;
Q_FALLTHROUGH();
case Qt::ScrollUpdate:
case Qt::ScrollMomentum:
if (!QApplicationPrivate::wheel_widget)
QApplicationPrivate::wheel_widget = w;
Q_FALLTHROUGH();
case Qt::NoScrollPhase:
QApplicationPrivate::giveFocusAccordingToFocusPolicy(w, e, relpos);
break;
// no default: - we want warnings if we don't handle all phases explicitly
}

if (spontaneous && (phase == Qt::NoScrollPhase || phase == Qt::ScrollUpdate))
QApplicationPrivate::giveFocusAccordingToFocusPolicy(w, e, relpos);
QWheelEvent we(relpos, wheel->globalPosition(), wheel->pixelDelta(), wheel->angleDelta(), wheel->buttons(),
wheel->modifiers(), phase, wheel->inverted(), wheel->source());

QWheelEvent we(relpos, wheel->globalPosition(), wheel->pixelDelta(), wheel->angleDelta(), wheel->buttons(),
wheel->modifiers(), phase, wheel->inverted(), wheel->source());
we.setTimestamp(wheel->timestamp());
bool eventAccepted;
do {
we.spont = spontaneous && w == receiver;
we.ignore();
res = d->notify_helper(w, &we);
eventAccepted = we.isAccepted();
if (res && eventAccepted) {
// A new scrolling sequence or partial sequence starts and w has accepted
// the event. Therefore, we can set wheel_widget, but only if it's not
// the end of a sequence.
if (QApplicationPrivate::wheel_widget == nullptr && (phase == Qt::ScrollBegin || phase == Qt::ScrollUpdate))
QApplicationPrivate::wheel_widget = w;
break;
}
if (w->isWindow() || w->testAttribute(Qt::WA_NoMousePropagation))
break;
we.setTimestamp(wheel->timestamp());
bool eventAccepted;
do {
we.spont = wheel->spontaneous() && w == receiver;
res = d->notify_helper(w, &we);
eventAccepted = we.isAccepted();
if (res && eventAccepted)
break;
if (w->isWindow() || w->testAttribute(Qt::WA_NoMousePropagation))
break;

we.p += w->pos();
w = w->parentWidget();
} while (w);
wheel->setAccepted(eventAccepted);
} else if (!spontaneous) {
// wheel_widget may forward the wheel event to a delegate widget,
// either directly or indirectly (e.g. QAbstractScrollArea will
// forward to its QScrollBars through viewportEvent()). In that
// case, the event will not be spontaneous but synthesized, so
// we can send it straight to the receiver.
d->notify_helper(w, wheel);
} else {
// The phase is either ScrollUpdate, ScrollMomentum, or ScrollEnd, and wheel_widget
// is set. Since it accepted the wheel event previously, we continue
// sending those events until we get a ScrollEnd, which signifies
// the end of the natural scrolling sequence.
const QPoint &relpos = QApplicationPrivate::wheel_widget->mapFromGlobal(wheel->globalPosition().toPoint());
QWheelEvent we(relpos, wheel->globalPosition(), wheel->pixelDelta(), wheel->angleDelta(), wheel->buttons(),
wheel->modifiers(), wheel->phase(), wheel->inverted(), wheel->source());
we.setTimestamp(wheel->timestamp());
we.spont = true;
we.ignore();
d->notify_helper(QApplicationPrivate::wheel_widget, &we);
wheel->setAccepted(we.isAccepted());
if (phase == Qt::ScrollEnd)
QApplicationPrivate::wheel_widget = nullptr;
}
we.p += w->pos();
w = w->parentWidget();
} while (w);
wheel->setAccepted(eventAccepted);
}
break;
#endif
Expand Down
73 changes: 55 additions & 18 deletions tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2150,33 +2150,53 @@ void tst_QApplication::touchEventPropagation()
For scenario 2 "outer", the expectation is that the outer scrollarea handles all wheel
events.
*/
using PhaseList = QList<Qt::ScrollPhase>;
struct WheelEvent
{
WheelEvent(Qt::ScrollPhase p = Qt::NoScrollPhase, Qt::Orientation o = Qt::Vertical)
: phase(p), orientation(o)
{}
Qt::ScrollPhase phase = Qt::NoScrollPhase;
Qt::Orientation orientation = Qt::Vertical;
};
using WheelEventList = QList<WheelEvent>;

void tst_QApplication::wheelEventPropagation_data()
{
qRegisterMetaType<PhaseList>();
qRegisterMetaType<WheelEventList>();

QTest::addColumn<bool>("innerScrolls");
QTest::addColumn<PhaseList>("phases");
QTest::addColumn<WheelEventList>("events");

QTest::addRow("inner, classic")
<< true
<< PhaseList{Qt::NoScrollPhase, Qt::NoScrollPhase, Qt::NoScrollPhase};
<< WheelEventList{{}, {}, {}};
QTest::addRow("outer, classic")
<< false
<< PhaseList{Qt::NoScrollPhase, Qt::NoScrollPhase, Qt::NoScrollPhase};
<< WheelEventList{{}, {}, {}};
QTest::addRow("inner, kinetic")
<< true
<< PhaseList{Qt::ScrollBegin, Qt::ScrollUpdate, Qt::ScrollMomentum, Qt::ScrollEnd};
<< WheelEventList{Qt::ScrollBegin, Qt::ScrollUpdate, Qt::ScrollMomentum, Qt::ScrollEnd};
QTest::addRow("outer, kinetic")
<< false
<< PhaseList{Qt::ScrollBegin, Qt::ScrollUpdate, Qt::ScrollMomentum, Qt::ScrollEnd};
<< WheelEventList{Qt::ScrollBegin, Qt::ScrollUpdate, Qt::ScrollMomentum, Qt::ScrollEnd};
QTest::addRow("inner, partial kinetic")
<< true
<< WheelEventList{Qt::ScrollUpdate, Qt::ScrollMomentum, Qt::ScrollEnd};
QTest::addRow("outer, partial kinetic")
<< false
<< WheelEventList{Qt::ScrollUpdate, Qt::ScrollMomentum, Qt::ScrollEnd};
QTest::addRow("inner, changing direction")
<< true
<< WheelEventList{Qt::ScrollUpdate, {Qt::ScrollUpdate, Qt::Horizontal}, Qt::ScrollMomentum, Qt::ScrollEnd};
QTest::addRow("outer, changing direction")
<< false
<< WheelEventList{Qt::ScrollUpdate, {Qt::ScrollUpdate, Qt::Horizontal}, Qt::ScrollMomentum, Qt::ScrollEnd};
}

void tst_QApplication::wheelEventPropagation()
{
QFETCH(bool, innerScrolls);
QFETCH(PhaseList, phases);
QFETCH(WheelEventList, events);

const QSize baseSize(500, 500);
const QPointF center(baseSize.width() / 2, baseSize.height() / 2);
Expand All @@ -2199,7 +2219,7 @@ void tst_QApplication::wheelEventPropagation()
largeWidget.setFixedSize(baseSize * 8);

// classic wheel events will be grabbed by the widget under the mouse, so don't place a trap
if (phases.at(0) == Qt::NoScrollPhase)
if (events.at(0).phase == Qt::NoScrollPhase)
trap.hide();
// kinetic wheel events should all go to the first widget; place a trap
else
Expand All @@ -2220,24 +2240,41 @@ void tst_QApplication::wheelEventPropagation()
auto innerVBar = innerArea.verticalScrollBar();
innerVBar->setObjectName("innerArea_vbar");
QCOMPARE(innerVBar->isVisible(), innerScrolls);
auto innerHBar = innerArea.horizontalScrollBar();
innerHBar->setObjectName("innerArea_hbar");
QCOMPARE(innerHBar->isVisible(), innerScrolls);
auto outerVBar = outerArea.verticalScrollBar();
outerVBar->setObjectName("outerArea_vbar");
QVERIFY(outerVBar->isVisible());
auto outerHBar = outerArea.horizontalScrollBar();
outerHBar->setObjectName("outerArea_hbar");
QVERIFY(outerHBar->isVisible());

const QPointF global(outerArea.mapToGlobal(center.toPoint()));

QSignalSpy innerSpy(innerVBar, &QAbstractSlider::valueChanged);
QSignalSpy outerSpy(outerVBar, &QAbstractSlider::valueChanged);
QSignalSpy innerVSpy(innerVBar, &QAbstractSlider::valueChanged);
QSignalSpy innerHSpy(innerHBar, &QAbstractSlider::valueChanged);
QSignalSpy outerVSpy(outerVBar, &QAbstractSlider::valueChanged);
QSignalSpy outerHSpy(outerHBar, &QAbstractSlider::valueChanged);

int count = 0;
for (const auto &phase : qAsConst(phases)) {
int vcount = 0;
int hcount = 0;

for (const auto &event : qAsConst(events)) {
const QPoint pixelDelta = event.orientation == Qt::Vertical ? QPoint(0, -scrollStep) : QPoint(-scrollStep, 0);
const QPoint angleDelta = event.orientation == Qt::Vertical ? QPoint(0, -120) : QPoint(-120, 0);
QWindowSystemInterface::handleWheelEvent(outerArea.windowHandle(), center, global,
QPoint(0, -scrollStep), QPoint(0, -120), Qt::NoModifier,
phase);
++count;
pixelDelta, angleDelta, Qt::NoModifier,
event.phase);
if (event.orientation == Qt::Vertical)
++vcount;
else
++hcount;
QCoreApplication::processEvents();
QCOMPARE(innerSpy.count(), innerScrolls ? count : 0);
QCOMPARE(outerSpy.count(), innerScrolls ? 0 : count);
QCOMPARE(innerVSpy.count(), innerScrolls ? vcount : 0);
QCOMPARE(innerHSpy.count(), innerScrolls ? hcount : 0);
QCOMPARE(outerVSpy.count(), innerScrolls ? 0 : vcount);
QCOMPARE(outerHSpy.count(), innerScrolls ? 0 : hcount);
}
}

Expand Down

0 comments on commit 92df790

Please sign in to comment.