Make QEventPoint an idiomatic value type

Use atomic ref counting and make type movable.

Make a moved-from QEventPoint valid but undefined. Given that the
d pointer is anyway loaded into the register, the additional check
doesn't cost much, and makes the class more compliant with our
guide for value types in Qt.

Add a free swap overload via Q_DECLARED_SHARED, which also declares
the type as movable.

As a drive-by, remove superfluous inline.
This also silences some static analyser warnings.

Change-Id: I7c4a436fce44556aa37026ac26dc2061e1f01de9
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
This commit is contained in:
Volker Hilsheimer 2020-11-09 11:11:07 +01:00
parent 021a8ce5f7
commit 6fd301b231
3 changed files with 71 additions and 72 deletions

View File

@ -46,6 +46,8 @@ QT_BEGIN_NAMESPACE
Q_LOGGING_CATEGORY(lcPointerVel, "qt.pointer.velocity") Q_LOGGING_CATEGORY(lcPointerVel, "qt.pointer.velocity")
Q_LOGGING_CATEGORY(lcEPDetach, "qt.pointer.eventpoint.detach") Q_LOGGING_CATEGORY(lcEPDetach, "qt.pointer.eventpoint.detach")
QT_DEFINE_QESDP_SPECIALIZATION_DTOR(QEventPointPrivate)
/*! \class QEventPoint /*! \class QEventPoint
\brief The QEventPoint class provides information about a point in a QPointerEvent. \brief The QEventPoint class provides information about a point in a QPointerEvent.
\since 6.0 \since 6.0
@ -94,26 +96,13 @@ QEventPoint::QEventPoint(int pointId, State state, const QPointF &scenePosition,
/*! /*!
Constructs an event point by making a shallow copy of \a other. Constructs an event point by making a shallow copy of \a other.
*/ */
QEventPoint::QEventPoint(const QEventPoint &other) QEventPoint::QEventPoint(const QEventPoint &other) noexcept = default;
: d(other.d)
{
if (d)
d->refCount++;
}
/*! /*!
Assigns \a other to this event point and returns a reference to this Assigns \a other to this event point and returns a reference to this
event point. event point.
*/ */
QEventPoint &QEventPoint::operator=(const QEventPoint &other) QEventPoint &QEventPoint::operator=(const QEventPoint &other) noexcept = default;
{
if (other.d)
other.d->refCount++;
if (d && !(--d->refCount))
delete d;
d = other.d;
return *this;
}
/*! /*!
\fn QEventPoint::QEventPoint(QEventPoint &&other) noexcept \fn QEventPoint::QEventPoint(QEventPoint &&other) noexcept
@ -150,11 +139,7 @@ bool QEventPoint::operator==(const QEventPoint &other) const noexcept
/*! /*!
Destroys the event point. Destroys the event point.
*/ */
QEventPoint::~QEventPoint() QEventPoint::~QEventPoint() = default;
{
if (d && !(--d->refCount))
delete d;
}
/*! \fn QPointF QEventPoint::pos() const /*! \fn QPointF QEventPoint::pos() const
\obsolete \obsolete
@ -171,7 +156,7 @@ QEventPoint::~QEventPoint()
The position is relative to the widget or item that received the event. The position is relative to the widget or item that received the event.
*/ */
QPointF QEventPoint::position() const QPointF QEventPoint::position() const
{ return d->pos; } { return d ? d->pos : QPointF(); }
/*! /*!
\property QEventPoint::pressPosition \property QEventPoint::pressPosition
@ -182,7 +167,7 @@ QPointF QEventPoint::position() const
\sa position \sa position
*/ */
QPointF QEventPoint::pressPosition() const QPointF QEventPoint::pressPosition() const
{ return d->globalPressPos - d->globalPos + d->pos; } { return d ? d->globalPressPos - d->globalPos + d->pos : QPointF(); }
/*! /*!
\property QEventPoint::grabPosition \property QEventPoint::grabPosition
@ -193,7 +178,7 @@ QPointF QEventPoint::pressPosition() const
\sa position \sa position
*/ */
QPointF QEventPoint::grabPosition() const QPointF QEventPoint::grabPosition() const
{ return d->globalGrabPos - d->globalPos + d->pos; } { return d ? d->globalGrabPos - d->globalPos + d->pos : QPointF(); }
/*! /*!
\property QEventPoint::lastPosition \property QEventPoint::lastPosition
@ -204,7 +189,7 @@ QPointF QEventPoint::grabPosition() const
\sa position, pressPosition \sa position, pressPosition
*/ */
QPointF QEventPoint::lastPosition() const QPointF QEventPoint::lastPosition() const
{ return d->globalLastPos - d->globalPos + d->pos; } { return d ? d->globalLastPos - d->globalPos + d->pos : QPointF(); }
/*! /*!
\property QEventPoint::scenePosition \property QEventPoint::scenePosition
@ -217,7 +202,7 @@ QPointF QEventPoint::lastPosition() const
\sa scenePressPosition, position, globalPosition \sa scenePressPosition, position, globalPosition
*/ */
QPointF QEventPoint::scenePosition() const QPointF QEventPoint::scenePosition() const
{ return d->scenePos; } { return d ? d->scenePos : QPointF(); }
/*! /*!
\property QEventPoint::scenePressPosition \property QEventPoint::scenePressPosition
@ -230,7 +215,7 @@ QPointF QEventPoint::scenePosition() const
\sa scenePosition, pressPosition, globalPressPosition \sa scenePosition, pressPosition, globalPressPosition
*/ */
QPointF QEventPoint::scenePressPosition() const QPointF QEventPoint::scenePressPosition() const
{ return d->globalPressPos - d->globalPos + d->scenePos; } { return d ? d->globalPressPos - d->globalPos + d->scenePos : QPointF(); }
/*! /*!
\property QEventPoint::sceneGrabPosition \property QEventPoint::sceneGrabPosition
@ -243,7 +228,7 @@ QPointF QEventPoint::scenePressPosition() const
\sa scenePosition, grabPosition, globalGrabPosition \sa scenePosition, grabPosition, globalGrabPosition
*/ */
QPointF QEventPoint::sceneGrabPosition() const QPointF QEventPoint::sceneGrabPosition() const
{ return d->globalGrabPos - d->globalPos + d->scenePos; } { return d ? d->globalGrabPos - d->globalPos + d->scenePos : QPointF(); }
/*! /*!
\property QEventPoint::sceneLastPosition \property QEventPoint::sceneLastPosition
@ -256,7 +241,7 @@ QPointF QEventPoint::sceneGrabPosition() const
\sa scenePosition, scenePressPosition \sa scenePosition, scenePressPosition
*/ */
QPointF QEventPoint::sceneLastPosition() const QPointF QEventPoint::sceneLastPosition() const
{ return d->globalLastPos - d->globalPos + d->scenePos; } { return d ? d->globalLastPos - d->globalPos + d->scenePos : QPointF(); }
/*! /*!
\property QEventPoint::globalPosition \property QEventPoint::globalPosition
@ -267,7 +252,7 @@ QPointF QEventPoint::sceneLastPosition() const
\sa globalPressPosition, position, scenePosition \sa globalPressPosition, position, scenePosition
*/ */
QPointF QEventPoint::globalPosition() const QPointF QEventPoint::globalPosition() const
{ return d->globalPos; } { return d ? d->globalPos : QPointF(); }
/*! /*!
\property QEventPoint::globalPressPosition \property QEventPoint::globalPressPosition
@ -278,7 +263,7 @@ QPointF QEventPoint::globalPosition() const
\sa globalPosition, pressPosition, scenePressPosition \sa globalPosition, pressPosition, scenePressPosition
*/ */
QPointF QEventPoint::globalPressPosition() const QPointF QEventPoint::globalPressPosition() const
{ return d->globalPressPos; } { return d ? d->globalPressPos : QPointF(); }
/*! /*!
\property QEventPoint::globalGrabPosition \property QEventPoint::globalGrabPosition
@ -289,7 +274,7 @@ QPointF QEventPoint::globalPressPosition() const
\sa globalPosition, grabPosition, sceneGrabPosition \sa globalPosition, grabPosition, sceneGrabPosition
*/ */
QPointF QEventPoint::globalGrabPosition() const QPointF QEventPoint::globalGrabPosition() const
{ return d->globalGrabPos; } { return d ? d->globalGrabPos : QPointF(); }
/*! /*!
\property QEventPoint::globalLastPosition \property QEventPoint::globalLastPosition
@ -300,7 +285,7 @@ QPointF QEventPoint::globalGrabPosition() const
\sa globalPosition, lastPosition, sceneLastPosition \sa globalPosition, lastPosition, sceneLastPosition
*/ */
QPointF QEventPoint::globalLastPosition() const QPointF QEventPoint::globalLastPosition() const
{ return d->globalLastPos; } { return d ? d->globalLastPos : QPointF(); }
/*! /*!
\property QEventPoint::velocity \property QEventPoint::velocity
@ -319,21 +304,21 @@ QPointF QEventPoint::globalLastPosition() const
\sa QInputDevice::capabilities(), QInputEvent::device() \sa QInputDevice::capabilities(), QInputEvent::device()
*/ */
QVector2D QEventPoint::velocity() const QVector2D QEventPoint::velocity() const
{ return d->velocity; } { return d ? d->velocity : QVector2D(); }
/*! /*!
\property QEventPoint::state \property QEventPoint::state
\brief the current state of the event point. \brief the current state of the event point.
*/ */
QEventPoint::State QEventPoint::state() const QEventPoint::State QEventPoint::state() const
{ return d->state; } { return d ? d->state : QEventPoint::State::Unknown; }
/*! /*!
\property QEventPoint::device \property QEventPoint::device
\brief the pointing device from which this event point originates. \brief the pointing device from which this event point originates.
*/ */
const QPointingDevice *QEventPoint::device() const const QPointingDevice *QEventPoint::device() const
{ return d->device; } { return d ? d->device : nullptr; }
/*! /*!
\property QEventPoint::id \property QEventPoint::id
@ -344,7 +329,7 @@ const QPointingDevice *QEventPoint::device() const
the underlying drivers work. the underlying drivers work.
*/ */
int QEventPoint::id() const int QEventPoint::id() const
{ return d->pointId; } { return d ? d->pointId : -1; }
/*! /*!
\property QEventPoint::uniqueId \property QEventPoint::uniqueId
@ -360,7 +345,7 @@ int QEventPoint::id() const
in use with a touchscreen that supports them. in use with a touchscreen that supports them.
*/ */
QPointingDeviceUniqueId QEventPoint::uniqueId() const QPointingDeviceUniqueId QEventPoint::uniqueId() const
{ return d->uniqueId; } { return d ? d->uniqueId : QPointingDeviceUniqueId(); }
/*! /*!
\property QEventPoint::timestamp \property QEventPoint::timestamp
@ -369,7 +354,7 @@ QPointingDeviceUniqueId QEventPoint::uniqueId() const
\sa QPointerEvent::timestamp() \sa QPointerEvent::timestamp()
*/ */
ulong QEventPoint::timestamp() const ulong QEventPoint::timestamp() const
{ return d->timestamp; } { return d ? d->timestamp : 0; }
/*! /*!
\property QEventPoint::lastTimestamp \property QEventPoint::lastTimestamp
@ -378,7 +363,7 @@ ulong QEventPoint::timestamp() const
\sa globalLastPosition \sa globalLastPosition
*/ */
ulong QEventPoint::lastTimestamp() const ulong QEventPoint::lastTimestamp() const
{ return d->lastTimestamp; } { return d ? d->lastTimestamp : 0; }
/*! /*!
\property QEventPoint::pressTimestamp \property QEventPoint::pressTimestamp
@ -387,7 +372,7 @@ ulong QEventPoint::lastTimestamp() const
\sa timestamp \sa timestamp
*/ */
ulong QEventPoint::pressTimestamp() const ulong QEventPoint::pressTimestamp() const
{ return d->pressTimestamp; } { return d ? d->pressTimestamp : 0; }
/*! /*!
\property QEventPoint::timeHeld \property QEventPoint::timeHeld
@ -396,7 +381,7 @@ ulong QEventPoint::pressTimestamp() const
\sa pressTimestamp, timestamp \sa pressTimestamp, timestamp
*/ */
qreal QEventPoint::timeHeld() const qreal QEventPoint::timeHeld() const
{ return (d->timestamp - d->pressTimestamp) / qreal(1000); } { return d ? (d->timestamp - d->pressTimestamp) / qreal(1000) : 0.0; }
/*! /*!
\property QEventPoint::pressure \property QEventPoint::pressure
@ -405,7 +390,7 @@ qreal QEventPoint::timeHeld() const
The return value is in the range \c 0.0 to \c 1.0. The return value is in the range \c 0.0 to \c 1.0.
*/ */
qreal QEventPoint::pressure() const qreal QEventPoint::pressure() const
{ return d->pressure; } { return d ? d->pressure : 0.0; }
/*! /*!
\property QEventPoint::rotation \property QEventPoint::rotation
@ -417,7 +402,7 @@ qreal QEventPoint::pressure() const
Most touchscreens do not detect rotation, so zero is the most common value. Most touchscreens do not detect rotation, so zero is the most common value.
*/ */
qreal QEventPoint::rotation() const qreal QEventPoint::rotation() const
{ return d->rotation; } { return d ? d->rotation : 0.0; }
/*! /*!
\property QEventPoint::ellipseDiameters \property QEventPoint::ellipseDiameters
@ -429,7 +414,7 @@ qreal QEventPoint::rotation() const
may be nonzero and always equal (the ellipse is approximated as a circle). may be nonzero and always equal (the ellipse is approximated as a circle).
*/ */
QSizeF QEventPoint::ellipseDiameters() const QSizeF QEventPoint::ellipseDiameters() const
{ return d->ellipseDiameters; } { return d ? d->ellipseDiameters : QSizeF(); }
/*! /*!
\property QEventPoint::accepted \property QEventPoint::accepted
@ -449,11 +434,12 @@ QSizeF QEventPoint::ellipseDiameters() const
*/ */
void QEventPoint::setAccepted(bool accepted) void QEventPoint::setAccepted(bool accepted)
{ {
if (d)
d->accept = accepted; d->accept = accepted;
} }
bool QEventPoint::isAccepted() const bool QEventPoint::isAccepted() const
{ return d->accept; } { return d ? d->accept : false; }
/*! /*!
@ -474,6 +460,9 @@ bool QEventPoint::isAccepted() const
*/ */
QPointF QEventPoint::normalizedPosition() const QPointF QEventPoint::normalizedPosition() const
{ {
if (!d)
return {};
auto geom = d->device->availableVirtualGeometry(); auto geom = d->device->availableVirtualGeometry();
if (geom.isNull()) if (geom.isNull())
return QPointF(); return QPointF();
@ -488,6 +477,9 @@ QPointF QEventPoint::normalizedPosition() const
*/ */
QPointF QEventPoint::startNormalizedPos() const QPointF QEventPoint::startNormalizedPos() const
{ {
if (!d)
return {};
auto geom = d->device->availableVirtualGeometry(); auto geom = d->device->availableVirtualGeometry();
if (geom.isNull()) if (geom.isNull())
return QPointF(); return QPointF();
@ -508,6 +500,9 @@ QPointF QEventPoint::startNormalizedPos() const
*/ */
QPointF QEventPoint::lastNormalizedPos() const QPointF QEventPoint::lastNormalizedPos() const
{ {
if (!d)
return {};
auto geom = d->device->availableVirtualGeometry(); auto geom = d->device->availableVirtualGeometry();
if (geom.isNull()) if (geom.isNull())
return QPointF(); return QPointF();
@ -523,19 +518,14 @@ QPointF QEventPoint::lastNormalizedPos() const
event, or code that wants to save an eventpoint for later, has event, or code that wants to save an eventpoint for later, has
responsibility to detach before calling any setters, so as to hold and 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 modify an independent copy. (The independent copy can then be used in a
subsequent event.) If detaching is unnecessary, because refCount shows that subsequent event.)
there is only one QEventPoint referring to the QEventPointPrivate instance,
this function does nothing.
*/ */
void QMutableEventPoint::detach() void QMutableEventPoint::detach()
{ {
if (d->refCount == 1) if (d)
return; // no need: there is only one QEventPoint using it d.detach();
qCDebug(lcEPDetach) << "detaching: refCount" << d->refCount << this; else
auto old = d; d.reset(new QEventPointPrivate(-1, nullptr));
d = new QEventPointPrivate(*d);
d->refCount = 1;
--old->refCount;
} }
/*! \internal /*! \internal
@ -613,18 +603,20 @@ void QMutableEventPoint::setTimestamp(const ulong t)
// then a press. Both events will get the same timestamp. So we need to set // then a press. Both events will get the same timestamp. So we need to set
// the press timestamp and position even when the timestamp isn't advancing, // the press timestamp and position even when the timestamp isn't advancing,
// but skip setting lastTimestamp and velocity because those need a time delta. // but skip setting lastTimestamp and velocity because those need a time delta.
if (d) {
if (state() == QEventPoint::State::Pressed) { if (state() == QEventPoint::State::Pressed) {
d->pressTimestamp = t; d->pressTimestamp = t;
d->globalPressPos = d->globalPos; d->globalPressPos = d->globalPos;
} }
if (d->timestamp == t) if (d->timestamp == t)
return; return;
}
detach(); detach();
if (device()) { if (device()) {
// get the persistent instance out of QPointingDevicePrivate::activePoints // get the persistent instance out of QPointingDevicePrivate::activePoints
// (which sometimes might be the same as this instance) // (which sometimes might be the same as this instance)
QEventPointPrivate *pd = QPointingDevicePrivate::get( QEventPointPrivate *pd = QPointingDevicePrivate::get(
const_cast<QPointingDevice *>(d->device))->pointById(id())->eventPoint.d; const_cast<QPointingDevice *>(d->device))->pointById(id())->eventPoint.d.get();
if (t > pd->timestamp) { if (t > pd->timestamp) {
pd->lastTimestamp = pd->timestamp; pd->lastTimestamp = pd->timestamp;
pd->timestamp = t; pd->timestamp = t;

View File

@ -43,10 +43,14 @@
#include <QtGui/qtguiglobal.h> #include <QtGui/qtguiglobal.h>
#include <QtGui/qvector2d.h> #include <QtGui/qvector2d.h>
#include <QtGui/qpointingdevice.h> #include <QtGui/qpointingdevice.h>
#include <QtCore/qshareddata.h>
#include <QtCore/qmetatype.h>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
struct QEventPointPrivate; class QEventPointPrivate;
QT_DECLARE_QESDP_SPECIALIZATION_DTOR_WITH_EXPORT(QEventPointPrivate, Q_GUI_EXPORT)
class Q_GUI_EXPORT QEventPoint class Q_GUI_EXPORT QEventPoint
{ {
Q_GADGET Q_GADGET
@ -87,14 +91,14 @@ public:
Q_DECLARE_FLAGS(States, State) Q_DECLARE_FLAGS(States, State)
Q_FLAG(States) Q_FLAG(States)
QEventPoint(int id = -1, const QPointingDevice *device = nullptr); explicit QEventPoint(int id = -1, const QPointingDevice *device = nullptr);
QEventPoint(int pointId, State state, const QPointF &scenePosition, const QPointF &globalPosition); QEventPoint(int pointId, State state, const QPointF &scenePosition, const QPointF &globalPosition);
QEventPoint(const QEventPoint &other); QEventPoint(const QEventPoint &other) noexcept;
QEventPoint(QEventPoint && other) noexcept : d(std::move(other.d)) { other.d = nullptr; } QEventPoint &operator=(const QEventPoint &other) noexcept;
QEventPoint &operator=(const QEventPoint &other); QEventPoint(QEventPoint && other) noexcept = default;
QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QEventPoint) QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QEventPoint)
bool operator==(const QEventPoint &other) const noexcept; bool operator==(const QEventPoint &other) const noexcept;
inline bool operator!=(const QEventPoint &other) const noexcept { return !operator==(other); } bool operator!=(const QEventPoint &other) const noexcept { return !operator==(other); }
~QEventPoint(); ~QEventPoint();
inline void swap(QEventPoint &other) noexcept inline void swap(QEventPoint &other) noexcept
{ qSwap(d, other.d); } { qSwap(d, other.d); }
@ -157,7 +161,7 @@ public:
void setAccepted(bool accepted = true); void setAccepted(bool accepted = true);
private: private:
QEventPointPrivate *d; QExplicitlySharedDataPointer<QEventPointPrivate> d;
friend class QMutableEventPoint; friend class QMutableEventPoint;
friend class QPointerEvent; friend class QPointerEvent;
}; };
@ -167,6 +171,8 @@ Q_GUI_EXPORT QDebug operator<<(QDebug, const QEventPoint *);
Q_GUI_EXPORT QDebug operator<<(QDebug, const QEventPoint &); Q_GUI_EXPORT QDebug operator<<(QDebug, const QEventPoint &);
#endif #endif
Q_DECLARE_SHARED(QEventPoint)
QT_END_NAMESPACE QT_END_NAMESPACE
#endif // QEVENTPOINT_H #endif // QEVENTPOINT_H

View File

@ -62,7 +62,9 @@ Q_DECLARE_LOGGING_CATEGORY(lcEPDetach);
class QPointingDevice; class QPointingDevice;
struct QEventPointPrivate { class QEventPointPrivate : public QSharedData
{
public:
QEventPointPrivate(int id, const QPointingDevice *device) QEventPointPrivate(int id, const QPointingDevice *device)
: device(device), pointId(id) { } : device(device), pointId(id) { }
@ -108,7 +110,6 @@ struct QEventPointPrivate {
ulong lastTimestamp = 0; ulong lastTimestamp = 0;
ulong pressTimestamp = 0; ulong pressTimestamp = 0;
QPointingDeviceUniqueId uniqueId; QPointingDeviceUniqueId uniqueId;
int refCount = 1;
int pointId = -1; int pointId = -1;
QEventPoint::State state = QEventPoint::State::Unknown; QEventPoint::State state = QEventPoint::State::Unknown;
bool accept = false; bool accept = false;