From 38806273e567a39bd7ca4ced5e6e38dfd7426f27 Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Thu, 6 Aug 2020 17:28:06 +0200 Subject: [PATCH] Remove QEventPoint::event() in favor of device() event()->device() was the most common use case anyway. The idea that the "parent" of a QEventPoint is the QPointerEvent interferes with the ability to copy and move event objects: the parent pointers are dangling unless we use the QPointerEvent subclass destructors to set the points' parents to null. Since there is no move constructor, even returning a QEventPoint from a function by value results in destroying the temporary instance and copying it to the caller's space. So the parent pointer is often useless, unless we do even more work to maintain it when the event moves. If we optimize to avoid copying QEventPoints too much (and perhaps enable exposing _mutable_ points to QML) by storing reusable instances in QPointingDevice (which is the current plan), then the actual parent will no longer be the event. Events are usually stack-allocated, thus temporary and intended to be movable. Change-Id: I24b648dcc046fc79d2401c781f1fda6cb00f47b0 Reviewed-by: Volker Hilsheimer --- src/gui/kernel/qevent.cpp | 23 +++++++++---------- src/gui/kernel/qevent.h | 8 +++---- src/gui/kernel/qevent_p.h | 2 +- .../kernel/qtouchevent/tst_qtouchevent.cpp | 2 +- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/gui/kernel/qevent.cpp b/src/gui/kernel/qevent.cpp index df8a18beb04..2414a0d9511 100644 --- a/src/gui/kernel/qevent.cpp +++ b/src/gui/kernel/qevent.cpp @@ -65,9 +65,7 @@ Q_LOGGING_CATEGORY(lcPointerGrab, "qt.pointer.grab") static const QString pointDeviceName(const QEventPoint *point) { - if (!point->event()) - return {}; - auto device = point->event()->device(); + const auto device = point->device(); QString deviceName = (device ? device->name() : QLatin1String("null device")); deviceName.resize(16, u' '); // shorten, and align in case of sequential output return deviceName; @@ -228,13 +226,14 @@ QInputEvent::~QInputEvent() /*! \internal - Constructs an invalid event point with the given \a id and \a parent. + Constructs an invalid event point with the given \a id and the \a device + from which it originated. This acts as a default constructor in usages like QMap, as in qgraphicsscene_p.h. */ -QEventPoint::QEventPoint(int id, const QPointerEvent *parent) - : m_parent(parent), m_pointId(id), m_state(State::Unknown), m_accept(false), m_stationaryWithModifiedProperty(false), m_reserved(0) +QEventPoint::QEventPoint(int id, const QPointingDevice *device) + : m_device(device), m_pointId(id), m_state(State::Unknown), m_accept(false), m_stationaryWithModifiedProperty(false), m_reserved(0) { } @@ -323,7 +322,7 @@ void QEventPoint::clearPassiveGrabbers() QPointF QEventPoint::normalizedPos() const { - auto geom = event()->device()->availableVirtualGeometry(); + auto geom = m_device->availableVirtualGeometry(); if (geom.isNull()) return QPointF(); return (globalPosition() - geom.topLeft()) / geom.width(); @@ -331,7 +330,7 @@ QPointF QEventPoint::normalizedPos() const QPointF QEventPoint::startNormalizedPos() const { - auto geom = event()->device()->availableVirtualGeometry(); + auto geom = m_device->availableVirtualGeometry(); if (geom.isNull()) return QPointF(); return (globalPressPosition() - geom.topLeft()) / geom.width(); @@ -339,7 +338,7 @@ QPointF QEventPoint::startNormalizedPos() const QPointF QEventPoint::lastNormalizedPos() const { - auto geom = event()->device()->availableVirtualGeometry(); + auto geom = m_device->availableVirtualGeometry(); if (geom.isNull()) return QPointF(); return (globalLastPosition() - geom.topLeft()) / geom.width(); @@ -369,7 +368,7 @@ const QPointingDevice *QPointerEvent::pointingDevice() const QSinglePointEvent::QSinglePointEvent(QEvent::Type type, const QPointingDevice *dev, const QPointF &localPos, const QPointF &scenePos, const QPointF &globalPos, Qt::MouseButton button, Qt::MouseButtons buttons, Qt::KeyboardModifiers modifiers) : QPointerEvent(type, dev, modifiers), - m_point(0, this), + m_point(0, dev), m_button(button), m_mouseState(buttons), m_source(Qt::MouseEventNotSynthesized), @@ -4243,7 +4242,7 @@ QTouchEvent::QTouchEvent(QEvent::Type eventType, { for (QEventPoint &point : m_touchPoints) { m_touchPointStates |= point.state(); - QMutableEventPoint::from(point).setParent(this); + QMutableEventPoint::from(point).setDevice(device); } } @@ -4266,7 +4265,7 @@ QTouchEvent::QTouchEvent(QEvent::Type eventType, m_touchPoints(touchPoints) { for (QEventPoint &point : m_touchPoints) - QMutableEventPoint::from(point).setParent(this); + QMutableEventPoint::from(point).setDevice(device); } /*! diff --git a/src/gui/kernel/qevent.h b/src/gui/kernel/qevent.h index 8ba138a1bb9..86878bfb679 100644 --- a/src/gui/kernel/qevent.h +++ b/src/gui/kernel/qevent.h @@ -106,10 +106,9 @@ public: Q_DECLARE_FLAGS(States, State) Q_FLAG(States) - QEventPoint(int id = -1, const QPointerEvent *parent = nullptr); + QEventPoint(int id = -1, const QPointingDevice *device = nullptr); QEventPoint(int pointId, State state, const QPointF &scenePosition, const QPointF &globalPosition); - const QPointerEvent *event() const { return m_parent; } QPointF position() const { return m_pos; } QPointF pressPosition() const { return m_globalPressPos - m_globalPos + m_pos; } QPointF grabPosition() const { return m_globalGrabPos - m_globalPos + m_pos; } @@ -152,6 +151,7 @@ public: #endif // QT_DEPRECATED_SINCE(6, 0) QVector2D velocity() const { return m_velocity; } State state() const { return m_state; } + const QPointingDevice *device() const { return m_device; } int id() const { return m_pointId; } QPointingDeviceUniqueId uniqueId() const { return m_uniqueId; } ulong pressTimestamp() const { return m_pressTimestamp; } @@ -169,7 +169,7 @@ public: void clearPassiveGrabbers(); protected: - const QPointerEvent *m_parent = nullptr; + const QPointingDevice *m_device = nullptr; QPointF m_pos, m_scenePos, m_globalPos, m_globalPressPos, m_globalGrabPos, m_globalLastPos; qreal m_pressure = 1; @@ -178,7 +178,7 @@ protected: QVector2D m_velocity; QPointer m_exclusiveGrabber; QList > m_passiveGrabbers; - ulong m_timestamp = 0; // redundant with m_parent->timestamp(), but keeps timeHeld() working in a saved copy + ulong m_timestamp = 0; ulong m_pressTimestamp = 0; QPointingDeviceUniqueId m_uniqueId; int m_pointId = -1; diff --git a/src/gui/kernel/qevent_p.h b/src/gui/kernel/qevent_p.h index 275a56eeb66..a8da0aede24 100644 --- a/src/gui/kernel/qevent_p.h +++ b/src/gui/kernel/qevent_p.h @@ -84,7 +84,7 @@ public: void setId(int pointId) { m_pointId = pointId; } - void setParent(const QPointerEvent *p) { m_parent = p; } + void setDevice(const QPointingDevice *device) { m_device = device; } void setTimestamp(const ulong t) { m_timestamp = t; } diff --git a/tests/auto/gui/kernel/qtouchevent/tst_qtouchevent.cpp b/tests/auto/gui/kernel/qtouchevent/tst_qtouchevent.cpp index d3d8eba34d9..0b5de830186 100644 --- a/tests/auto/gui/kernel/qtouchevent/tst_qtouchevent.cpp +++ b/tests/auto/gui/kernel/qtouchevent/tst_qtouchevent.cpp @@ -79,7 +79,7 @@ public: seenTouchBegin = !seenTouchBegin && !seenTouchUpdate && !seenTouchEnd; auto touchEvent = static_cast(event); touchBeginPoints = touchEvent->touchPoints(); - Q_ASSERT(touchBeginPoints.first().event() == event); + Q_ASSERT(touchBeginPoints.first().device() == touchEvent->pointingDevice()); for (const QEventPoint &pt : touchEvent->touchPoints()) lastNormalizedPositions << pt.normalizedPos(); timestamp = touchEvent->timestamp();