Give QEventPoint a d-pointer after all

I still have doubts that QEventPoint can't be made small enough that
copying would be cheaper than reference-counting and all the indirections
in now-noninline accessors, but this gives us the usual freedom to
change the data members later on.

Change-Id: I792f7fc85ac3a9538589da9d7618b647edf0e70c
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

src/gui/kernel/qevent.cpp

@@ -240,18 +240,105 @@ QInputEvent::~QInputEvent()
     as in qgraphicsscene_p.h.
 */
 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)
+    : d(new QEventPointPrivate(id, device)) {}
+
+/*!
+    Constructs an event point with the given \a pointId, \a state,
+    \a scenePosition and \a globalPosition.
+*/
+QEventPoint::QEventPoint(int pointId, State state, const QPointF &scenePosition, const QPointF &globalPosition)
+    : d(new QEventPointPrivate(pointId, state, scenePosition, globalPosition)) {}
+
+QEventPoint::QEventPoint(const QEventPoint &other)
+    : d(other.d)
 {
+    d->refCount++;
 }
 
-QEventPoint::QEventPoint(int pointId, State state, const QPointF &scenePosition, const QPointF &globalPosition)
+QEventPoint &QEventPoint::operator=(const QEventPoint &other)
-    : m_scenePos(scenePosition), m_globalPos(globalPosition), m_pointId(pointId), m_state(state),
-      m_accept(false), m_stationaryWithModifiedProperty(false), m_reserved(0)
 {
-    if (state == QEventPoint::State::Released)
+    other.d->refCount++;
-        m_pressure = 0;
+    if (!(--d->refCount))
+        delete d;
+    d = other.d;
+    return *this;
 }
 
+QEventPoint::~QEventPoint()
+{
+    if (!(--d->refCount))
+        delete d;
+}
+
+QPointF QEventPoint::position() const
+{ return d->pos; }
+
+QPointF QEventPoint::pressPosition() const
+{ return d->globalPressPos - d->globalPos + d->pos; }
+
+QPointF QEventPoint::grabPosition() const
+{ return d->globalGrabPos - d->globalPos + d->pos; }
+
+QPointF QEventPoint::lastPosition() const
+{ return d->globalLastPos - d->globalPos + d->pos; }
+
+QPointF QEventPoint::scenePosition() const
+{ return d->scenePos; }
+
+QPointF QEventPoint::scenePressPosition() const
+{ return d->globalPressPos - d->globalPos + d->scenePos; }
+
+QPointF QEventPoint::sceneGrabPosition() const
+{ return d->globalGrabPos - d->globalPos + d->scenePos; }
+
+QPointF QEventPoint::sceneLastPosition() const
+{ return d->globalLastPos - d->globalPos + d->scenePos; }
+
+QPointF QEventPoint::globalPosition() const
+{ return d->globalPos; }
+
+QPointF QEventPoint::globalPressPosition() const
+{ return d->globalPressPos; }
+
+QPointF QEventPoint::globalGrabPosition() const
+{ return d->globalGrabPos; }
+
+QPointF QEventPoint::globalLastPosition() const
+{ return d->globalLastPos; }
+
+QVector2D QEventPoint::velocity() const
+{ return d->velocity; }
+
+QEventPoint::State QEventPoint::state() const
+{ return d->state; }
+
+const QPointingDevice *QEventPoint::device() const
+{ return d->device; }
+
+int QEventPoint::id() const
+{ return d->pointId; }
+
+QPointingDeviceUniqueId QEventPoint::uniqueId() const
+{ return d->uniqueId; }
+
+ulong QEventPoint::pressTimestamp() const
+{ return d->pressTimestamp; }
+
+qreal QEventPoint::timeHeld() const
+{ return (d->timestamp - d->pressTimestamp) / qreal(1000); }
+
+qreal QEventPoint::pressure() const
+{ return d->pressure; }
+
+qreal QEventPoint::rotation() const
+{ return d->rotation; }
+
+QSizeF QEventPoint::ellipseDiameters() const
+{ return d->ellipseDiameters; }
+
+bool QEventPoint::isAccepted() const
+{ return d->accept; }
+
 /*
     Sets the accepted state of the point.
 
@@ -269,9 +356,12 @@ QEventPoint::QEventPoint(int pointId, State state, const QPointF &scenePosition,
 */
 void QEventPoint::setAccepted(bool accepted)
 {
-    m_accept = accepted;
+    d->accept = accepted;
 }
 
+QObject *QEventPoint::exclusiveGrabber() const
+{ return d->exclusiveGrabber.data(); }
+
 /*
     Informs the delivery logic that the given \a exclusiveGrabber is to
     receive all future update events and the release event containing
@@ -281,16 +371,19 @@ void QEventPoint::setAccepted(bool accepted)
 */
 void QEventPoint::setExclusiveGrabber(QObject *exclusiveGrabber)
 {
-    if (m_exclusiveGrabber == exclusiveGrabber)
+    if (d->exclusiveGrabber == exclusiveGrabber)
         return;
     if (Q_UNLIKELY(lcPointerGrab().isDebugEnabled())) {
-        qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << Qt::hex << m_pointId << m_state << "@" << m_scenePos
+        qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << Qt::hex << d->pointId << d->state << "@" << d->scenePos
-                               << ": grab" << m_exclusiveGrabber << "->" << exclusiveGrabber;
+                               << ": grab" << d->exclusiveGrabber << "->" << exclusiveGrabber;
     }
-    m_exclusiveGrabber = exclusiveGrabber;
+    d->exclusiveGrabber = exclusiveGrabber;
-    m_globalGrabPos = m_globalPos;
+    d->globalGrabPos = d->globalPos;
 }
 
+const QList<QPointer<QObject> > &QEventPoint::passiveGrabbers() const
+{ return d->passiveGrabbers; }
+
 /*
     Informs the delivery logic that the given \a grabbers are to receive all
     future update events and the release event containing this point,
@@ -300,9 +393,9 @@ void QEventPoint::setExclusiveGrabber(QObject *exclusiveGrabber)
 */
 void QEventPoint::setPassiveGrabbers(const QList<QPointer<QObject> > &grabbers)
 {
-    m_passiveGrabbers = grabbers;
+    d->passiveGrabbers = grabbers;
     if (Q_UNLIKELY(lcPointerGrab().isDebugEnabled())) {
-        qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << Qt::hex << m_pointId << m_state
+        qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << Qt::hex << d->pointId << d->state
                                << ": grab (passive)" << grabbers;
     }
 }
@@ -310,10 +403,10 @@ void QEventPoint::setPassiveGrabbers(const QList<QPointer<QObject> > &grabbers)
 void QEventPoint::clearPassiveGrabbers()
 {
     if (Q_UNLIKELY(lcPointerGrab().isDebugEnabled())) {
-        qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << Qt::hex << m_pointId << m_state
+        qCDebug(lcPointerGrab) << pointDeviceName(this) << "point" << Qt::hex << d->pointId << d->state
-                               << ": clearing" << m_passiveGrabbers;
+                               << ": clearing" << d->passiveGrabbers;
     }
-    m_passiveGrabbers.clear();
+    d->passiveGrabbers.clear();
 }
 
 /*! \internal
@@ -329,7 +422,7 @@ void QEventPoint::clearPassiveGrabbers()
 
 QPointF QEventPoint::normalizedPos() const
 {
-    auto geom = m_device->availableVirtualGeometry();
+    auto geom = d->device->availableVirtualGeometry();
     if (geom.isNull())
         return QPointF();
     return (globalPosition() - geom.topLeft()) / geom.width();
@@ -337,7 +430,7 @@ QPointF QEventPoint::normalizedPos() const
 
 QPointF QEventPoint::startNormalizedPos() const
 {
-    auto geom = m_device->availableVirtualGeometry();
+    auto geom = d->device->availableVirtualGeometry();
     if (geom.isNull())
         return QPointF();
     return (globalPressPosition() - geom.topLeft()) / geom.width();
@@ -345,12 +438,32 @@ QPointF QEventPoint::startNormalizedPos() const
 
 QPointF QEventPoint::lastNormalizedPos() const
 {
-    auto geom = m_device->availableVirtualGeometry();
+    auto geom = d->device->availableVirtualGeometry();
     if (geom.isNull())
         return QPointF();
     return (globalLastPosition() - geom.topLeft()) / geom.width();
 }
 
+
+/*! \internal
+    This class is explicitly shared, which means if you construct an event and
+    then the point(s) that it holds are modified before the event is delivered,
+    the event will be seen to hold the modified points. The workaround is that
+    any code which modifies an eventpoint that could already be included in an
+    event, or code that wants to save an eventpoint for later, has
+    responsibility to detach before calling any setters, so as to hold and
+    modify an independent copy. (The independent copy can then be used in a
+    subsequent event.) If detaching is unnecessary, because refCount shows that
+    there is only one QEventPoint referring to the QEventPointPrivate instance,
+    this function does nothing.
+*/
+void QMutableEventPoint::detach()
+{
+    if (d->refCount == 1)
+        return; // no need: there is only one QEventPoint using it
+    d = new QEventPointPrivate(*d);
+}
+
 QPointerEvent::QPointerEvent(QEvent::Type type, const QPointingDevice *dev, Qt::KeyboardModifiers modifiers)
     : QInputEvent(type, QEvent::PointerEventTag{}, dev, modifiers)
 {

src/gui/kernel/qevent.h

@@ -90,10 +90,7 @@ protected:
     qint64 m_extra = 0; // reserved, unused for now
 };
 
-namespace QTest {
+struct QEventPointPrivate;
-    class QTouchEventSequence; // just for the friend declaration below
-}
-
 class Q_GUI_EXPORT QEventPoint
 {
     Q_GADGET
@@ -110,19 +107,22 @@ public:
 
     QEventPoint(int id = -1, const QPointingDevice *device = nullptr);
     QEventPoint(int pointId, State state, const QPointF &scenePosition, const QPointF &globalPosition);
+    QEventPoint(const QEventPoint &other);
+    QEventPoint &operator=(const QEventPoint &other);
+    ~QEventPoint();
 
-    QPointF position() const { return m_pos; }
+    QPointF position() const;
-    QPointF pressPosition() const { return m_globalPressPos - m_globalPos + m_pos; }
+    QPointF pressPosition() const;
-    QPointF grabPosition() const { return m_globalGrabPos - m_globalPos + m_pos; }
+    QPointF grabPosition() const;
-    QPointF lastPosition() const { return m_globalLastPos - m_globalPos + m_pos; }
+    QPointF lastPosition() const;
-    QPointF scenePosition() const { return m_scenePos; }
+    QPointF scenePosition() const;
-    QPointF scenePressPosition() const { return m_globalPressPos - m_globalPos + m_scenePos; }
+    QPointF scenePressPosition() const;
-    QPointF sceneGrabPosition() const { return m_globalGrabPos - m_globalPos + m_scenePos; }
+    QPointF sceneGrabPosition() const;
-    QPointF sceneLastPosition() const { return m_globalLastPos - m_globalPos + m_scenePos; }
+    QPointF sceneLastPosition() const;
-    QPointF globalPosition() const { return m_globalPos; }
+    QPointF globalPosition() const;
-    QPointF globalPressPosition() const { return m_globalPressPos; }
+    QPointF globalPressPosition() const;
-    QPointF globalGrabPosition() const { return m_globalGrabPos; }
+    QPointF globalGrabPosition() const;
-    QPointF globalLastPosition() const { return m_globalLastPos; }
+    QPointF globalLastPosition() const;
 
 #if QT_DEPRECATED_SINCE(6, 0)
     // QEventPoint replaces QTouchEvent::TouchPoint, so we need all its old accessors, for now
@@ -151,45 +151,28 @@ public:
     QT_DEPRECATED_VERSION_X_6_0("Use globalLastPosition()")
     QPointF lastNormalizedPos() const;
 #endif // QT_DEPRECATED_SINCE(6, 0)
-    QVector2D velocity() const { return m_velocity; }
+    QVector2D velocity() const;
-    State state() const { return m_state; }
+    State state() const;
-    const QPointingDevice *device() const { return m_device; }
+    const QPointingDevice *device() const;
-    int id() const { return m_pointId; }
+    int id() const;
-    QPointingDeviceUniqueId uniqueId() const { return m_uniqueId; }
+    QPointingDeviceUniqueId uniqueId() const;
-    ulong pressTimestamp() const { return m_pressTimestamp; }
+    ulong pressTimestamp() const;
-    qreal timeHeld() const { return (m_timestamp - m_pressTimestamp) / qreal(1000); }
+    qreal timeHeld() const;
-    qreal pressure() const { return m_pressure; }
+    qreal pressure() const;
-    qreal rotation() const { return m_rotation; }
+    qreal rotation() const;
-    QSizeF ellipseDiameters() const { return m_ellipseDiameters; }
+    QSizeF ellipseDiameters() const;
 
-    bool isAccepted() const { return m_accept; }
+    bool isAccepted() const;
     void setAccepted(bool accepted = true);
-    QObject *exclusiveGrabber() const { return m_exclusiveGrabber.data(); }
+    QObject *exclusiveGrabber() const;
     void setExclusiveGrabber(QObject *exclusiveGrabber);
-    const QList<QPointer <QObject>> &passiveGrabbers() const { return m_passiveGrabbers; }
+    const QList<QPointer <QObject>> &passiveGrabbers() const;
     void setPassiveGrabbers(const QList<QPointer <QObject>> &grabbers);
     void clearPassiveGrabbers();
 
-protected:
+private:
-    const QPointingDevice *m_device = nullptr;
+    QEventPointPrivate *d;
-    QPointF m_pos, m_scenePos, m_globalPos,
+    friend class QMutableEventPoint;
-            m_globalPressPos, m_globalGrabPos, m_globalLastPos;
-    qreal m_pressure = 1;
-    qreal m_rotation = 0;
-    QSizeF m_ellipseDiameters = QSizeF(0, 0);
-    QVector2D m_velocity;
-    QPointer<QObject> m_exclusiveGrabber;
-    QList<QPointer <QObject> > m_passiveGrabbers;
-    ulong m_timestamp = 0;
-    ulong m_pressTimestamp = 0;
-    QPointingDeviceUniqueId m_uniqueId;
-    int m_pointId = -1;
-    State m_state : 8;
-    quint32 m_accept : 1;
-    quint32 m_stationaryWithModifiedProperty : 1;
-    quint32 m_reserved : 22;
-
-    friend class QTest::QTouchEventSequence;
 };
 
 #ifndef QT_NO_DEBUG_STREAM

src/gui/kernel/qevent_p.h

@@ -58,6 +58,36 @@
 
 QT_BEGIN_NAMESPACE
 
+struct QEventPointPrivate {
+    QEventPointPrivate(int id, const QPointingDevice *device)
+      : device(device), pointId(id) { }
+
+    QEventPointPrivate(int pointId, QEventPoint::State state, const QPointF &scenePosition, const QPointF &globalPosition)
+        : scenePos(scenePosition), globalPos(globalPosition), pointId(pointId), state(state)
+    {
+        if (state == QEventPoint::State::Released)
+            pressure = 0;
+    }
+
+    const QPointingDevice *device = nullptr;
+    QPointF pos, scenePos, globalPos,
+            globalPressPos, globalGrabPos, globalLastPos;
+    qreal pressure = 1;
+    qreal rotation = 0;
+    QSizeF ellipseDiameters = QSizeF(0, 0);
+    QVector2D velocity;
+    QPointer<QObject> exclusiveGrabber;
+    QList<QPointer <QObject> > passiveGrabbers;
+    ulong timestamp = 0;
+    ulong pressTimestamp = 0;
+    QPointingDeviceUniqueId uniqueId;
+    int refCount = 1;
+    int pointId = -1;
+    QEventPoint::State state = QEventPoint::State::Unknown;
+    bool accept = false;
+    bool stationaryWithModifiedProperty = false;
+};
+
 // Private subclasses to allow accessing and modifying protected variables.
 // These should NOT hold any extra state.
 
@@ -72,59 +102,61 @@ public:
                        const QPointF &position, const QPointF &scenePosition, const QPointF &globalPosition) :
         QEventPoint(pointId, state, scenePosition, globalPosition)
     {
-        m_timestamp = timestamp;
+        d->timestamp = timestamp;
-        m_pos = position;
+        d->pos = position;
     }
 
     static QMutableEventPoint *from(QEventPoint *me) { return static_cast<QMutableEventPoint *>(me); }
 
     static QMutableEventPoint &from(QEventPoint &me) { return static_cast<QMutableEventPoint &>(me); }
 
-    bool stationaryWithModifiedProperty() const { return m_stationaryWithModifiedProperty; }
+    void detach();
 
-    void setId(int pointId) { m_pointId = pointId; }
+    bool stationaryWithModifiedProperty() const { return d->stationaryWithModifiedProperty; }
 
-    void setDevice(const QPointingDevice *device) { m_device = device; }
+    void setId(int pointId) { d->pointId = pointId; }
 
-    void setTimestamp(const ulong t) { m_timestamp = t; }
+    void setDevice(const QPointingDevice *device) { d->device = device; }
 
-    void setPressTimestamp(const ulong t) { m_pressTimestamp = t; }
+    void setTimestamp(const ulong t) { d->timestamp = t; }
 
-    void setState(QEventPoint::State state) { m_state = state; }
+    void setPressTimestamp(const ulong t) { d->pressTimestamp = t; }
 
-    void setUniqueId(const QPointingDeviceUniqueId &uid) { m_uniqueId = uid; }
+    void setState(QEventPoint::State state) { d->state = state; }
 
-    void setPosition(const QPointF &pos) { m_pos = pos; }
+    void setUniqueId(const QPointingDeviceUniqueId &uid) { d->uniqueId = uid; }
 
-    void setScenePosition(const QPointF &pos) { m_scenePos = pos; }
+    void setPosition(const QPointF &pos) { d->pos = pos; }
 
-    void setGlobalPosition(const QPointF &pos) { m_globalPos = pos; }
+    void setScenePosition(const QPointF &pos) { d->scenePos = pos; }
+
+    void setGlobalPosition(const QPointF &pos) { d->globalPos = pos; }
 
 #if QT_DEPRECATED_SINCE(6, 0)
     // temporary replacements for QTouchEvent::TouchPoint setters, mainly to make porting easier
     QT_DEPRECATED_VERSION_X_6_0("Use setPosition()")
-    void setPos(const QPointF &pos) { m_pos = pos; }
+    void setPos(const QPointF &pos) { d->pos = pos; }
     QT_DEPRECATED_VERSION_X_6_0("Use setScenePosition()")
-    void setScenePos(const QPointF &pos) { m_scenePos = pos; }
+    void setScenePos(const QPointF &pos) { d->scenePos = pos; }
     QT_DEPRECATED_VERSION_X_6_0("Use setGlobalPosition()")
-    void setScreenPos(const QPointF &pos) { m_globalPos = pos; }
+    void setScreenPos(const QPointF &pos) { d->globalPos = pos; }
 #endif
 
-    void setGlobalPressPosition(const QPointF &pos) { m_globalPressPos = pos; }
+    void setGlobalPressPosition(const QPointF &pos) { d->globalPressPos = pos; }
 
-    void setGlobalGrabPosition(const QPointF &pos) { m_globalGrabPos = pos; }
+    void setGlobalGrabPosition(const QPointF &pos) { d->globalGrabPos = pos; }
 
-    void setGlobalLastPosition(const QPointF &pos) { m_globalLastPos = pos; }
+    void setGlobalLastPosition(const QPointF &pos) { d->globalLastPos = pos; }
 
-    void setEllipseDiameters(const QSizeF &d) { m_ellipseDiameters = d; }
+    void setEllipseDiameters(const QSizeF &diams) { d->ellipseDiameters = diams; }
 
-    void setPressure(qreal v) { m_pressure = v; }
+    void setPressure(qreal v) { d->pressure = v; }
 
-    void setRotation(qreal v) { m_rotation = v; }
+    void setRotation(qreal v) { d->rotation = v; }
 
-    void setVelocity(const QVector2D &v) { m_velocity = v; }
+    void setVelocity(const QVector2D &v) { d->velocity = v; }
 
-    void setStationaryWithModifiedProperty(bool s = true) { m_stationaryWithModifiedProperty = s; }
+    void setStationaryWithModifiedProperty(bool s = true) { d->stationaryWithModifiedProperty = s; }
 };
 
 static_assert(sizeof(QMutableEventPoint) == sizeof(QEventPoint));

tests/auto/gui/kernel/qtouchevent/tst_qtouchevent.cpp

@@ -1508,11 +1508,13 @@ void tst_QTouchEvent::deleteInEventHandler()
                                     touchScreenDevice,
                                     Qt::NoModifier,
                                     {touchPoint});
+        touchPoint.detach();
         touchPoint.setState(QEventPoint::State::Updated);
         QTouchEvent touchUpdateEvent(QEvent::TouchUpdate,
                                touchScreenDevice,
                                Qt::NoModifier,
                                 {touchPoint});
+        touchPoint.detach();
         touchPoint.setState(QEventPoint::State::Released);
         QTouchEvent touchEndEvent(QEvent::TouchEnd,
                                touchScreenDevice,