From ae49252dbcc21de1dccee03c54a8e0986c5a0acc Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 6 Jan 2022 13:53:17 +0100 Subject: [PATCH] QMutableEventPoint: add static overloads of setters These overloads don't require a cast from QEventPoint to QMutableEventPoint, thus avoiding undefined behavior. Port easy users of QMutableEventPosition::(const)from() to the new API. Pick-to: 6.3 Task-number: QTBUG-99615 Change-Id: I4e9228322134ef7c712ca478ee8286466efc3585 Reviewed-by: Qt CI Bot Reviewed-by: Volker Hilsheimer --- src/gui/kernel/qevent.cpp | 14 ++--- src/gui/kernel/qeventpoint.cpp | 40 +++++++------- src/gui/kernel/qeventpoint_p.h | 60 ++++++++++----------- src/gui/kernel/qguiapplication.cpp | 8 ++- src/gui/kernel/qpointingdevice.cpp | 10 ++-- src/gui/kernel/qtestsupport_gui.cpp | 22 ++++---- src/widgets/graphicsview/qgraphicsscene.cpp | 4 +- src/widgets/graphicsview/qgraphicsview.cpp | 2 +- src/widgets/kernel/qapplication.cpp | 18 +++---- src/widgets/kernel/qtestsupport_widgets.cpp | 22 ++++---- 10 files changed, 99 insertions(+), 101 deletions(-) diff --git a/src/gui/kernel/qevent.cpp b/src/gui/kernel/qevent.cpp index 81496edacd..b5c57dbed8 100644 --- a/src/gui/kernel/qevent.cpp +++ b/src/gui/kernel/qevent.cpp @@ -362,7 +362,7 @@ void QPointerEvent::setTimestamp(quint64 timestamp) { QInputEvent::setTimestamp(timestamp); for (auto &p : m_points) - QMutableEventPoint::from(p).setTimestamp(timestamp); + QMutableEventPoint::setTimestamp(p, timestamp); } /*! @@ -2563,9 +2563,9 @@ QTabletEvent::QTabletEvent(Type type, const QPointingDevice *dev, const QPointF m_yTilt(yTilt), m_z(z) { - QMutableEventPoint &mut = QMutableEventPoint::from(point(0)); - mut.setPressure(pressure); - mut.setRotation(rotation); + QEventPoint &p = point(0); + QMutableEventPoint::setPressure(p, pressure); + QMutableEventPoint::setRotation(p, rotation); } /*! @@ -4548,7 +4548,7 @@ QTouchEvent::QTouchEvent(QEvent::Type eventType, { for (QEventPoint &point : m_points) { m_touchPointStates |= point.state(); - QMutableEventPoint::from(point).setDevice(device); + QMutableEventPoint::setDevice(point, device); } } @@ -4569,7 +4569,7 @@ QTouchEvent::QTouchEvent(QEvent::Type eventType, m_touchPointStates(touchPointStates) { for (QEventPoint &point : m_points) - QMutableEventPoint::from(point).setDevice(device); + QMutableEventPoint::setDevice(point, device); } /*! @@ -4837,7 +4837,7 @@ void QMutableTouchEvent::addPoint(const QEventPoint &point) m_points.append(point); auto &added = m_points.last(); if (!added.device()) - QMutableEventPoint::from(added).setDevice(pointingDevice()); + QMutableEventPoint::setDevice(added, pointingDevice()); m_touchPointStates |= point.state(); } diff --git a/src/gui/kernel/qeventpoint.cpp b/src/gui/kernel/qeventpoint.cpp index 249569a853..fc72099c19 100644 --- a/src/gui/kernel/qeventpoint.cpp +++ b/src/gui/kernel/qeventpoint.cpp @@ -515,12 +515,12 @@ QPointF QEventPoint::lastNormalizedPos() const modify an independent copy. (The independent copy can then be used in a subsequent event.) */ -void QMutableEventPoint::detach() +void QMutableEventPoint::detach(QEventPoint &p) { - if (d) - d.detach(); + if (p.d) + p.d.detach(); else - d.reset(new QEventPointPrivate(-1, nullptr)); + p.d.reset(new QEventPointPrivate(-1, nullptr)); } /*! \internal @@ -591,33 +591,33 @@ void QMutableEventPoint::updateFrom(const QEventPoint &other) The velocity calculation is skipped if the platform has promised to provide velocities already by setting the QInputDevice::Velocity capability. */ -void QMutableEventPoint::setTimestamp(const ulong t) +void QMutableEventPoint::setTimestamp(QEventPoint &p, ulong t) { // On mouse press, if the mouse has moved from its last-known location, // QGuiApplicationPrivate::processMouseEvent() sends first a mouse move and // 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, // but skip setting lastTimestamp and velocity because those need a time delta. - if (d) { - if (state() == QEventPoint::State::Pressed) { - d->pressTimestamp = t; - d->globalPressPos = d->globalPos; + if (p.d) { + if (p.state() == QEventPoint::State::Pressed) { + p.d->pressTimestamp = t; + p.d->globalPressPos = p.d->globalPos; } - if (d->timestamp == t) + if (p.d->timestamp == t) return; } - detach(); - if (device()) { + detach(p); + if (p.device()) { // get the persistent instance out of QPointingDevicePrivate::activePoints // (which sometimes might be the same as this instance) QEventPointPrivate *pd = QPointingDevicePrivate::get( - const_cast(d->device))->pointById(id())->eventPoint.d.get(); + const_cast(p.d->device))->pointById(p.id())->eventPoint.d.get(); if (t > pd->timestamp) { pd->lastTimestamp = pd->timestamp; pd->timestamp = t; - if (state() == QEventPoint::State::Pressed) + if (p.state() == QEventPoint::State::Pressed) pd->pressTimestamp = t; - if (pd->lastTimestamp > 0 && !device()->capabilities().testFlag(QInputDevice::Capability::Velocity)) { + if (pd->lastTimestamp > 0 && !p.device()->capabilities().testFlag(QInputDevice::Capability::Velocity)) { // calculate instantaneous velocity according to time and distance moved since the previous point QVector2D newVelocity = QVector2D(pd->globalPos - pd->globalLastPos) / (t - pd->lastTimestamp) * 1000; // VERY simple kalman filter: does a weighted average @@ -628,17 +628,17 @@ void QMutableEventPoint::setTimestamp(const ulong t) "based on movement" << pd->globalLastPos << "->" << pd->globalPos << "over time" << pd->lastTimestamp << "->" << pd->timestamp; } - if (d != pd) { - d->lastTimestamp = pd->lastTimestamp; - d->velocity = pd->velocity; + if (p.d != pd) { + p.d->lastTimestamp = pd->lastTimestamp; + p.d->velocity = pd->velocity; } } } - d->timestamp = t; + p.d->timestamp = t; } /*! - \fn void QMutableEventPoint::setPosition(const QPointF &pos) + \fn void QMutableEventPoint::setPosition(QPointF pos) \internal Sets the localized position. diff --git a/src/gui/kernel/qeventpoint_p.h b/src/gui/kernel/qeventpoint_p.h index f10739343f..1cd32fd536 100644 --- a/src/gui/kernel/qeventpoint_p.h +++ b/src/gui/kernel/qeventpoint_p.h @@ -141,25 +141,27 @@ public: static const QMutableEventPoint &constFrom(const QEventPoint &me) { return static_cast(me); } - void detach(); + void detach() { detach(*this); } + static void detach(QEventPoint &p); - void setId(int pointId) { d->pointId = pointId; } +#define TRIVIAL_SETTER(type, field, Field) \ + void set##Field (type arg) { d->field = std::move(arg); } \ + static void set##Field (QEventPoint &p, type arg) { p.d->field = std::move(arg); } \ + /* end */ - void setDevice(const QPointingDevice *device) { d->device = device; } + TRIVIAL_SETTER(int, pointId, Id) + TRIVIAL_SETTER(const QPointingDevice *, device, Device) - void setTimestamp(const ulong t); + // not trivial: + void setTimestamp(const ulong t) { setTimestamp(*this, t); } + static void setTimestamp(QEventPoint &p, ulong t); - void setPressTimestamp(const ulong t) { d->pressTimestamp = t; } - - void setState(QEventPoint::State state) { d->state = state; } - - void setUniqueId(const QPointingDeviceUniqueId &uid) { d->uniqueId = uid; } - - void setPosition(const QPointF &pos) { d->pos = pos; } - - void setScenePosition(const QPointF &pos) { d->scenePos = pos; } - - void setGlobalPosition(const QPointF &pos) { d->globalPos = pos; } + TRIVIAL_SETTER(ulong, pressTimestamp, PressTimestamp) + TRIVIAL_SETTER(QEventPoint::State, state, State) + TRIVIAL_SETTER(QPointingDeviceUniqueId, uniqueId, UniqueId) + TRIVIAL_SETTER(QPointF, pos, Position) + TRIVIAL_SETTER(QPointF, scenePos, ScenePosition) + TRIVIAL_SETTER(QPointF, globalPos, GlobalPosition) #if QT_DEPRECATED_SINCE(6, 0) // temporary replacements for QTouchEvent::TouchPoint setters, mainly to make porting easier @@ -171,27 +173,25 @@ public: void setScreenPos(const QPointF &pos) { d->globalPos = pos; } #endif - void setGlobalPressPosition(const QPointF &pos) { d->globalPressPos = pos; } - - void setGlobalGrabPosition(const QPointF &pos) { d->globalGrabPos = pos; } - - void setGlobalLastPosition(const QPointF &pos) { d->globalLastPos = pos; } - - void setEllipseDiameters(const QSizeF &diams) { d->ellipseDiameters = diams; } - - void setPressure(qreal v) { d->pressure = v; } - - void setRotation(qreal v) { d->rotation = v; } - - void setVelocity(const QVector2D &v) { d->velocity = v; } + TRIVIAL_SETTER(QPointF, globalPressPos, GlobalPressPosition) + TRIVIAL_SETTER(QPointF, globalGrabPos, GlobalGrabPosition) + TRIVIAL_SETTER(QPointF, globalLastPos, GlobalLastPosition) + TRIVIAL_SETTER(QSizeF, ellipseDiameters, EllipseDiameters) + TRIVIAL_SETTER(qreal, pressure, Pressure) + TRIVIAL_SETTER(qreal, rotation, Rotation) + TRIVIAL_SETTER(QVector2D, velocity, Velocity) QWindow *window() const { return d->window.data(); } + static QWindow *window(const QEventPoint &p) { return p.d->window.data(); } - void setWindow(const QPointer &w) { d->window = w; } + TRIVIAL_SETTER(QWindow *, window, Window) QObject *target() const { return d->target.data(); } + static QObject *target(const QEventPoint &p) { return p.d->target.data(); } - void setTarget(const QPointer &t) { d->target = t; } + TRIVIAL_SETTER(QObject *, target, Target) + +#undef TRIVIAL_SETTER }; static_assert(sizeof(QMutableEventPoint) == sizeof(QEventPoint)); diff --git a/src/gui/kernel/qguiapplication.cpp b/src/gui/kernel/qguiapplication.cpp index de1b8d725f..5ace7049c7 100644 --- a/src/gui/kernel/qguiapplication.cpp +++ b/src/gui/kernel/qguiapplication.cpp @@ -2232,7 +2232,7 @@ void QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::Mo QMouseEvent ev(type, localPoint, localPoint, globalPoint, button, e->buttons, e->modifiers, e->source, device); // restore globalLastPosition to avoid invalidating the velocity calculations, // because the QPlatformCursor mouse event above was in native coordinates - QMutableEventPoint::from(persistentEPD->eventPoint).setGlobalLastPosition(lastGlobalPosition); + QMutableEventPoint::setGlobalLastPosition(persistentEPD->eventPoint, lastGlobalPosition); // ev now contains a detached copy of the QEventPoint from QPointingDevicePrivate::activePoints ev.setTimestamp(e->timestamp); if (window->d_func()->blockedByModalWindow && !qApp->d_func()->popupActive()) { @@ -2410,7 +2410,7 @@ void QGuiApplicationPrivate::processEnterEvent(QWindowSystemInterfacePrivate::En const QPointingDevicePrivate *devPriv = QPointingDevicePrivate::get(event.pointingDevice()); auto epd = devPriv->queryPointById(event.points().first().id()); Q_ASSERT(epd); - QMutableEventPoint::from(epd->eventPoint).setVelocity({}); + QMutableEventPoint::setVelocity(epd->eventPoint, {}); QCoreApplication::sendSpontaneousEvent(e->enter.data(), &event); } @@ -2800,9 +2800,7 @@ void QGuiApplicationPrivate::processTouchEvent(QWindowSystemInterfacePrivate::To QSet windowsNeedingCancel; for (auto &epd : devPriv->activePoints.values()) { - auto &mut = QMutableEventPoint::from(const_cast(epd.eventPoint)); - QWindow *w = mut.window(); - if (w) + if (QWindow *w = QMutableEventPoint::window(epd.eventPoint)) windowsNeedingCancel.insert(w); } diff --git a/src/gui/kernel/qpointingdevice.cpp b/src/gui/kernel/qpointingdevice.cpp index 3675cec2dd..32ca48b23d 100644 --- a/src/gui/kernel/qpointingdevice.cpp +++ b/src/gui/kernel/qpointingdevice.cpp @@ -426,8 +426,8 @@ QPointingDevicePrivate::EventPointData *QPointingDevicePrivate::pointById(int id if (it == activePoints.end()) { Q_Q(const QPointingDevice); QPointingDevicePrivate::EventPointData epd; - QMutableEventPoint::from(epd.eventPoint).setId(id); - QMutableEventPoint::from(epd.eventPoint).setDevice(q); + QMutableEventPoint::setId(epd.eventPoint, id); + QMutableEventPoint::setDevice(epd.eventPoint, q); return &activePoints.insert(id, epd).first.value(); } return &it.value(); @@ -451,7 +451,7 @@ void QPointingDevicePrivate::removePointById(int id) QObject *QPointingDevicePrivate::firstActiveTarget() const { for (auto &pt : activePoints.values()) { - if (auto target = QMutableEventPoint::constFrom(pt.eventPoint).target()) + if (auto target = QMutableEventPoint::target(pt.eventPoint)) return target; } return nullptr; @@ -466,7 +466,7 @@ QObject *QPointingDevicePrivate::firstActiveTarget() const QWindow *QPointingDevicePrivate::firstActiveWindow() const { for (auto &pt : activePoints.values()) { - if (auto window = QMutableEventPoint::constFrom(pt.eventPoint).window()) + if (auto window = QMutableEventPoint::window(pt.eventPoint)) return window; } return nullptr; @@ -505,7 +505,7 @@ void QPointingDevicePrivate::setExclusiveGrabber(const QPointerEvent *event, con << "@" << point.scenePosition() << ": grab" << oldGrabber << "->" << exclusiveGrabber; } - QMutableEventPoint::from(persistentPoint->eventPoint).setGlobalGrabPosition(point.globalPosition()); + QMutableEventPoint::setGlobalGrabPosition(persistentPoint->eventPoint, point.globalPosition()); if (exclusiveGrabber) emit q->grabChanged(exclusiveGrabber, QPointingDevice::GrabExclusive, event, point); else diff --git a/src/gui/kernel/qtestsupport_gui.cpp b/src/gui/kernel/qtestsupport_gui.cpp index ab48111fb5..41798d650d 100644 --- a/src/gui/kernel/qtestsupport_gui.cpp +++ b/src/gui/kernel/qtestsupport_gui.cpp @@ -101,29 +101,29 @@ QTouchEventSequence::~QTouchEventSequence() } QTouchEventSequence& QTouchEventSequence::press(int touchId, const QPoint &pt, QWindow *window) { - auto &p = QMutableEventPoint::from(point(touchId)); - p.setGlobalPosition(mapToScreen(window, pt)); - p.setState(QEventPoint::State::Pressed); + auto &p = point(touchId); + QMutableEventPoint::setGlobalPosition(p, mapToScreen(window, pt)); + QMutableEventPoint::setState(p, QEventPoint::State::Pressed); return *this; } QTouchEventSequence& QTouchEventSequence::move(int touchId, const QPoint &pt, QWindow *window) { - auto &p = QMutableEventPoint::from(point(touchId)); - p.setGlobalPosition(mapToScreen(window, pt)); - p.setState(QEventPoint::State::Updated); + auto &p = point(touchId); + QMutableEventPoint::setGlobalPosition(p, mapToScreen(window, pt)); + QMutableEventPoint::setState(p, QEventPoint::State::Updated); return *this; } QTouchEventSequence& QTouchEventSequence::release(int touchId, const QPoint &pt, QWindow *window) { - auto &p = QMutableEventPoint::from(point(touchId)); - p.setGlobalPosition(mapToScreen(window, pt)); - p.setState(QEventPoint::State::Released); + auto &p = point(touchId); + QMutableEventPoint::setGlobalPosition(p, mapToScreen(window, pt)); + QMutableEventPoint::setState(p, QEventPoint::State::Released); return *this; } QTouchEventSequence& QTouchEventSequence::stationary(int touchId) { - auto &p = QMutableEventPoint::from(pointOrPreviousPoint(touchId)); - p.setState(QEventPoint::State::Stationary); + auto &p = pointOrPreviousPoint(touchId); + QMutableEventPoint::setState(p, QEventPoint::State::Stationary); return *this; } diff --git a/src/widgets/graphicsview/qgraphicsscene.cpp b/src/widgets/graphicsview/qgraphicsscene.cpp index 0eceaeacb2..5e30f114eb 100644 --- a/src/widgets/graphicsview/qgraphicsscene.cpp +++ b/src/widgets/graphicsview/qgraphicsscene.cpp @@ -5840,8 +5840,8 @@ void QGraphicsScenePrivate::updateTouchPointsForItem(QGraphicsItem *item, QTouch item->d_ptr->genericMapFromSceneTransform(static_cast(touchEvent->target())); for (int i = 0; i < touchEvent->pointCount(); ++i) { - auto &pt = QMutableEventPoint::from(touchEvent->point(i)); - QMutableEventPoint::from(pt).setPosition(mapFromScene.map(pt.scenePosition())); + auto &pt = touchEvent->point(i); + QMutableEventPoint::setPosition(pt, mapFromScene.map(pt.scenePosition())); } } diff --git a/src/widgets/graphicsview/qgraphicsview.cpp b/src/widgets/graphicsview/qgraphicsview.cpp index f4b9228e51..1a147e5e40 100644 --- a/src/widgets/graphicsview/qgraphicsview.cpp +++ b/src/widgets/graphicsview/qgraphicsview.cpp @@ -314,7 +314,7 @@ void QGraphicsViewPrivate::translateTouchEvent(QGraphicsViewPrivate *d, QTouchEv auto &pt = touchEvent->point(i); // the scene will set the item local pos, startPos, lastPos, and rect before delivering to // an item, but for now those functions are returning the view's local coordinates - QMutableEventPoint::from(pt).setScenePosition(d->mapToScene(pt.position())); + QMutableEventPoint::setScenePosition(pt, d->mapToScene(pt.position())); // screenPos, startScreenPos, and lastScreenPos are already set } } diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp index fa381e796c..b1ec997e04 100644 --- a/src/widgets/kernel/qapplication.cpp +++ b/src/widgets/kernel/qapplication.cpp @@ -3152,8 +3152,8 @@ bool QApplication::notify(QObject *receiver, QEvent *e) w = w->parentWidget(); touchEvent->setTarget(w); for (int i = 0; i < touchEvent->pointCount(); ++i) { - auto &pt = QMutableEventPoint::from(touchEvent->point(i)); - pt.setPosition(pt.position() + offset); + auto &pt = touchEvent->point(i); + QMutableEventPoint::setPosition(pt, pt.position() + offset); } } @@ -3802,8 +3802,8 @@ bool QApplicationPrivate::updateTouchPointsForWidget(QWidget *widget, QTouchEven bool containsPress = false; for (int i = 0; i < touchEvent->pointCount(); ++i) { - auto &pt = QMutableEventPoint::from(touchEvent->point(i)); - pt.setPosition(widget->mapFromGlobal(pt.globalPosition())); + auto &pt = touchEvent->point(i); + QMutableEventPoint::setPosition(pt, widget->mapFromGlobal(pt.globalPosition())); if (pt.state() == QEventPoint::State::Pressed) containsPress = true; @@ -3862,9 +3862,9 @@ void QApplicationPrivate::activateImplicitTouchGrab(QWidget *widget, QTouchEvent // there might already be an implicit grabber. Don't override that. A widget that // has partially recognized a gesture needs to grab all points. for (int i = 0; i < touchEvent->pointCount(); ++i) { - auto &mep = QMutableEventPoint::from(touchEvent->point(i)); - if (!mep.target() && (mep.isAccepted() || grabMode == GrabAllPoints)) - mep.setTarget(widget); + auto &ep = touchEvent->point(i); + if (!QMutableEventPoint::target(ep) && (ep.isAccepted() || grabMode == GrabAllPoints)) + QMutableEventPoint::setTarget(ep, widget); } // TODO setExclusiveGrabber() to be consistent with Qt Quick? } @@ -3913,9 +3913,9 @@ bool QApplicationPrivate::translateRawTouchEvent(QWidget *window, const QTouchEv // on touch pads, implicitly grab all touch points // on touch screens, grab touch points that are redirected to the closest widget if (device->type() == QInputDevice::DeviceType::TouchPad || usingClosestWidget) - QMutableEventPoint::from(touchPoint).setTarget(target); + QMutableEventPoint::setTarget(touchPoint, target); } else { - target = QMutableEventPoint::from(touchPoint).target(); + target = QMutableEventPoint::target(touchPoint); if (!target) continue; } diff --git a/src/widgets/kernel/qtestsupport_widgets.cpp b/src/widgets/kernel/qtestsupport_widgets.cpp index c9116fcef6..9c5603fc5a 100644 --- a/src/widgets/kernel/qtestsupport_widgets.cpp +++ b/src/widgets/kernel/qtestsupport_widgets.cpp @@ -100,30 +100,30 @@ QTouchEventWidgetSequence::~QTouchEventWidgetSequence() QTouchEventWidgetSequence& QTouchEventWidgetSequence::press(int touchId, const QPoint &pt, QWidget *widget) { - auto &p = QMutableEventPoint::from(point(touchId)); - p.setGlobalPosition(mapToScreen(widget, pt)); - p.setState(QEventPoint::State::Pressed); + auto &p = point(touchId); + QMutableEventPoint::setGlobalPosition(p, mapToScreen(widget, pt)); + QMutableEventPoint::setState(p, QEventPoint::State::Pressed); return *this; } QTouchEventWidgetSequence& QTouchEventWidgetSequence::move(int touchId, const QPoint &pt, QWidget *widget) { - auto &p = QMutableEventPoint::from(point(touchId)); - p.setGlobalPosition(mapToScreen(widget, pt)); - p.setState(QEventPoint::State::Updated); + auto &p = point(touchId); + QMutableEventPoint::setGlobalPosition(p, mapToScreen(widget, pt)); + QMutableEventPoint::setState(p, QEventPoint::State::Updated); return *this; } QTouchEventWidgetSequence& QTouchEventWidgetSequence::release(int touchId, const QPoint &pt, QWidget *widget) { - auto &p = QMutableEventPoint::from(point(touchId)); - p.setGlobalPosition(mapToScreen(widget, pt)); - p.setState(QEventPoint::State::Released); + auto &p = point(touchId); + QMutableEventPoint::setGlobalPosition(p, mapToScreen(widget, pt)); + QMutableEventPoint::setState(p, QEventPoint::State::Released); return *this; } QTouchEventWidgetSequence& QTouchEventWidgetSequence::stationary(int touchId) { - auto &p = QMutableEventPoint::from(pointOrPreviousPoint(touchId)); - p.setState(QEventPoint::State::Stationary); + auto &p = pointOrPreviousPoint(touchId); + QMutableEventPoint::setState(p, QEventPoint::State::Stationary); return *this; }