From 12b7eab4d24e867675a27ab1785ff03318f9add1 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 14 Oct 2022 10:15:01 +0200 Subject: [PATCH] Detach event points when cloning pointer events Input events that originate from actual device interaction should reflect the device's state, and device and events need to be kept in sync so that event sequences (such as multi-touch events, where we have begin/update/end cycles spanning multiple events) are working correctly. For that reason, the event point data in pointer events is explicitly shared, and we only detach in exceptional situations. This saves us memory allocations, and makes sure that the event point data carried by events, and the event point data stored persistently in the device, are kept in sync. Cloned pointer events do not originate from device interactions, and should therefore not sync back to the device. E.g. accepting a clone should not modify the original event data stored in the device. There are exceptions here as well, e.g. when cloning an event in Qt in order to deliver a translated version of it to a different scene. Different points might even get delivered to different scenes or windows, or at least different items in the same scene. For that reason, we explicitly detach, and then explicitly write back the relevant states after the cloned event has been delivered. But in general, we should assume that cloned events do not write back to the device. Since QEventPoint is an explicitly shared data type that never detaches itself, we have to explicitly detach it when making copies that should not be shared. The ideal implementation of this would be to do the detach in the copy constructor of QPointerEvent, which is called when cloning. However, Qt itself makes copies of QPointerEvent without using clone, e.g. when assembling lists of touch events for the different subscenes or windows in QGuiApplicationPrivate::processTouchEvent, where event objects are added to a QVarLengthArray. This makes copies, and those copies must not detach. So we have to implement the special cloning behavior in each override of QPointerEvent::clone(). For this, introduce a dedicated macro for the common member functions. This macro must be used for QPointerEvent subclasses. Fixes: QTBUG-107560 Change-Id: I4b56f9e71c7d067ba9054a2a631e8ba5bc7b1ab9 Reviewed-by: Shawn Rutledge --- src/gui/kernel/qevent.cpp | 34 ++++++++++++++----- .../kernel/qmouseevent/tst_qmouseevent.cpp | 20 +++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/gui/kernel/qevent.cpp b/src/gui/kernel/qevent.cpp index 014bf8bc854..3e3631f8ca1 100644 --- a/src/gui/kernel/qevent.cpp +++ b/src/gui/kernel/qevent.cpp @@ -28,6 +28,22 @@ #include +#define Q_IMPL_POINTER_EVENT(Class) \ + Class::Class(const Class &) = default; \ + Class::~Class() = default; \ + Class* Class::clone() const \ + { \ + auto c = new Class(*this); \ + for (auto &point : c->m_points) \ + QMutableEventPoint::detach(point); \ + QEvent *e = c; \ + /* check that covariant return is safe to add */ \ + Q_ASSERT(reinterpret_cast(c) == reinterpret_cast(e)); \ + return c; \ + } + + + QT_BEGIN_NAMESPACE static_assert(sizeof(QMutableTouchEvent) == sizeof(QTouchEvent)); @@ -59,7 +75,7 @@ QEnterEvent::QEnterEvent(const QPointF &localPos, const QPointF &scenePos, const { } -Q_IMPL_EVENT_COMMON(QEnterEvent) +Q_IMPL_POINTER_EVENT(QEnterEvent) /*! \fn QPoint QEnterEvent::globalPos() const @@ -252,7 +268,7 @@ QPointerEvent::QPointerEvent(QEvent::Type type, QEvent::SinglePointEventTag, con { } -Q_IMPL_EVENT_COMMON(QPointerEvent) +Q_IMPL_POINTER_EVENT(QPointerEvent); /*! Returns the point whose \l {QEventPoint::id()}{id} matches the given \a id, @@ -555,7 +571,7 @@ QSinglePointEvent::QSinglePointEvent(QEvent::Type type, const QPointingDevice *d m_points << point; } -Q_IMPL_EVENT_COMMON(QSinglePointEvent) +Q_IMPL_POINTER_EVENT(QSinglePointEvent) /*! Returns \c true if this event represents a \l {button()}{button} being pressed. @@ -744,7 +760,7 @@ QMouseEvent::QMouseEvent(QEvent::Type type, const QPointF &localPos, const QPoin { } -Q_IMPL_EVENT_COMMON(QMouseEvent) +Q_IMPL_POINTER_EVENT(QMouseEvent) /*! \fn Qt::MouseEventSource QMouseEvent::source() const @@ -1062,7 +1078,7 @@ QHoverEvent::QHoverEvent(Type type, const QPointF &pos, const QPointF &oldPos, } #endif -Q_IMPL_EVENT_COMMON(QHoverEvent) +Q_IMPL_POINTER_EVENT(QHoverEvent) #if QT_CONFIG(wheelevent) /*! @@ -1187,7 +1203,7 @@ QWheelEvent::QWheelEvent(const QPointF &pos, const QPointF &globalPos, QPoint pi m_invertedScrolling = inverted; } -Q_IMPL_EVENT_COMMON(QWheelEvent) +Q_IMPL_POINTER_EVENT(QWheelEvent) /*! Returns \c true if this event's phase() is Qt::ScrollBegin. @@ -2494,7 +2510,7 @@ QTabletEvent::QTabletEvent(Type type, const QPointingDevice *dev, const QPointF QMutableEventPoint::setRotation(p, rotation); } -Q_IMPL_EVENT_COMMON(QTabletEvent) +Q_IMPL_POINTER_EVENT(QTabletEvent) /*! \fn qreal QTabletEvent::tangentialPressure() const @@ -2818,7 +2834,7 @@ QNativeGestureEvent::QNativeGestureEvent(Qt::NativeGestureType type, const QPoin Q_ASSERT(fingerCount < 16); // we store it in 4 bits unsigned } -Q_IMPL_EVENT_COMMON(QNativeGestureEvent) +Q_IMPL_POINTER_EVENT(QNativeGestureEvent) /*! \fn QNativeGestureEvent::gestureType() const @@ -4481,7 +4497,7 @@ QTouchEvent::QTouchEvent(QEvent::Type eventType, } #endif // QT_DEPRECATED_SINCE(6, 0) -Q_IMPL_EVENT_COMMON(QTouchEvent) +Q_IMPL_POINTER_EVENT(QTouchEvent) /*! Returns true if this event includes at least one newly-pressed touchpoint. diff --git a/tests/auto/gui/kernel/qmouseevent/tst_qmouseevent.cpp b/tests/auto/gui/kernel/qmouseevent/tst_qmouseevent.cpp index d7cc8a572ca..dda91225e51 100644 --- a/tests/auto/gui/kernel/qmouseevent/tst_qmouseevent.cpp +++ b/tests/auto/gui/kernel/qmouseevent/tst_qmouseevent.cpp @@ -97,6 +97,7 @@ private slots: void grabbers_data(); void grabbers(); void velocity(); + void clone(); private: MouseEventWidget* testMouseWidget; @@ -309,5 +310,24 @@ void tst_QMouseEvent::velocity() QVERIFY(testMouseWidget->velocity.y() > 0); } +void tst_QMouseEvent::clone() +{ + const QPointF pos(10.0f, 10.0f); + + QMouseEvent originalMe(QEvent::MouseButtonPress, pos, pos, pos, Qt::LeftButton, Qt::LeftButton, Qt::NoModifier); + QVERIFY(!originalMe.allPointsAccepted()); + QVERIFY(!originalMe.points().first().isAccepted()); + + // create a clone of the original + std::unique_ptr clonedMe(originalMe.clone()); + QVERIFY(!clonedMe->allPointsAccepted()); + QVERIFY(!clonedMe->points().first().isAccepted()); + + // now we alter originalMe, which should *not* change clonedMe + originalMe.setAccepted(true); + QVERIFY(!clonedMe->allPointsAccepted()); + QVERIFY(!clonedMe->points().first().isAccepted()); +} + QTEST_MAIN(tst_QMouseEvent) #include "tst_qmouseevent.moc"