QGraphicsView: remove (last remaining?) user of QEvent::op=
Instead of aggregating a QMutableSinglePointEvent by value, which
depends on the QEvent copy assignment operator, and casting it to a
QMouseEvent that it isn't, introduce QEventStorage which is a bit like
std::optional, but, by each event subclass befriending it, can store
event copies by-value, unlike clone(), which is restricted to the
heap. We could have befriended std::optional<QMouseEvent>, too, but by
adding our own type in _p.h, we can better control which code uses
this dangerous construct.
Added a guard to avoid clobbering lastMouseEvent with a copy of itself
in storeMouseEvent(). Before, we'd self-assign to lastMouseEvent,
which didn't invalidate the reference. Now, a store() is the
equivalent of a dtor + copy constructor, so we need to be a bit more
careful.
Fixes UBSan reports when running tst_qgraphicsview:
qgraphicsview/qgraphicsview.cpp:612:27: runtime error: downcast of address 0x61a000035e90 which does not point to an object of type 'QMouseEvent'
0x61a000035e90: note: object is of type 'QMutableSinglePointEvent'
00 00 00 00 30 47 ef 8b 99 7f 00 00 02 00 00 00 00 00 00 e0 d0 91 00 00 20 60 00 00 00 00 00 00
^~~~~~~~~~~~~~~~~~~~~~~
vptr for 'QMutableSinglePointEvent'
#0 0x7f99a27c7a47 in QGraphicsViewPrivate::replayLastMouseEvent() qgraphicsview.cpp:612
qgraphicsview.cpp:653:39: runtime error: member call on address 0x61a0000fe290 which does not point to an object of type 'QMouseEvent'
0x61a0000fe290: note: object is of type 'QMutableSinglePointEvent'
00 00 00 00 30 47 ef 8b 99 7f 00 00 05 00 00 00 00 00 00 e0 d0 91 00 00 20 60 00 00 00 00 00 00
^~~~~~~~~~~~~~~~~~~~~~~
vptr for 'QMutableSinglePointEvent'
#0 0x7f99a27c3609 in QGraphicsViewPrivate::mouseMoveEventHandler(QMouseEvent*) qgraphicsview.cpp:653
#1 0x7f99a27c7832 in QGraphicsViewPrivate::replayLastMouseEvent() qgraphicsview.cpp:612
qgraphicsview.cpp:654:37: runtime error: member call on address 0x61a0000fe290 which does not point to an object of type 'QMouseEvent'
0x61a0000fe290: note: object is of type 'QMutableSinglePointEvent'
00 00 00 00 30 47 ef 8b 99 7f 00 00 05 00 00 00 00 00 00 e0 d0 91 00 00 20 60 00 00 00 00 00 00
^~~~~~~~~~~~~~~~~~~~~~~
vptr for 'QMutableSinglePointEvent'
#0 0x7f99a27c358b in QGraphicsViewPrivate::mouseMoveEventHandler(QMouseEvent*) qgraphicsview.cpp:654
#1 0x7f99a27c7832 in QGraphicsViewPrivate::replayLastMouseEvent() qgraphicsview.cpp:612
Pick-to: 6.9
Task-number: QTBUG-99563
Change-Id: Ib642d416b8aef98c7fd8b1fa164ec2449189992a
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
This commit is contained in:
Marc Mutz
parent
e07b4d4394
commit
dd264cc9c0
@ -11,6 +11,8 @@
|
||||
|
||||
QT_BEGIN_NAMESPACE
|
||||
|
||||
template <typename Event> class QEventStorage;
|
||||
|
||||
#define Q_EVENT_DISABLE_COPY(Class) \
|
||||
protected: \
|
||||
Class(const Class &) = default; \
|
||||
@ -19,6 +21,7 @@ protected: \
|
||||
Class &operator=(Class &&) = delete
|
||||
|
||||
#define Q_DECL_EVENT_COMMON(Class) \
|
||||
friend class QEventStorage<Class>; \
|
||||
protected: \
|
||||
Class(const Class &); \
|
||||
Class(Class &&) = delete; \
|
||||
|
||||
@ -24,6 +24,100 @@ QT_BEGIN_NAMESPACE
|
||||
|
||||
class QPointingDevice;
|
||||
|
||||
/*!
|
||||
\internal
|
||||
\since 6.9
|
||||
|
||||
This class provides a way to store copies of QEvents. QEvents can otherwise
|
||||
only be copied by \l{QEvent::clone()}{cloning}, which allocates the copy on
|
||||
the heap. By befriending concrete QEvent subclasses, QEventStorage gains
|
||||
access to their protected copy constructors.
|
||||
|
||||
It is similar to std::optional, but with a more targeted API.
|
||||
|
||||
Note that by storing an event in QEventStorage, it is always sliced.
|
||||
*/
|
||||
template <typename Event>
|
||||
class QEventStorage
|
||||
{
|
||||
Q_DISABLE_COPY_MOVE(QEventStorage)
|
||||
static_assert(std::is_base_of_v<QEvent, Event>);
|
||||
union {
|
||||
char m_eventNotSet; // could be std::monostate, but don't want to pay for <variant> include
|
||||
Event m_event;
|
||||
};
|
||||
bool m_engaged = false;
|
||||
|
||||
void engage(const Event &e)
|
||||
{
|
||||
Q_ASSERT(!m_engaged);
|
||||
new (&m_event) Event(e);
|
||||
m_engaged = true;
|
||||
}
|
||||
|
||||
void disengage()
|
||||
{
|
||||
Q_ASSERT(m_engaged);
|
||||
m_event.~Event();
|
||||
m_engaged = false;
|
||||
}
|
||||
|
||||
public:
|
||||
QEventStorage() noexcept : m_eventNotSet{} {}
|
||||
explicit QEventStorage(const Event &e)
|
||||
{
|
||||
engage(e);
|
||||
}
|
||||
|
||||
~QEventStorage()
|
||||
{
|
||||
if (m_engaged)
|
||||
disengage();
|
||||
}
|
||||
|
||||
explicit operator bool() const noexcept { return m_engaged; }
|
||||
bool operator!() const noexcept { return !m_engaged; }
|
||||
|
||||
Event &store(const Event &e)
|
||||
{
|
||||
if (m_engaged)
|
||||
disengage();
|
||||
engage(e);
|
||||
return m_event;
|
||||
}
|
||||
|
||||
Event &storeUnlessAlias(const Event &e)
|
||||
{
|
||||
if (m_engaged && &e == &m_event)
|
||||
return m_event;
|
||||
return store(e);
|
||||
}
|
||||
|
||||
const Event &operator*() const
|
||||
{
|
||||
Q_PRE(m_engaged);
|
||||
return m_event;
|
||||
}
|
||||
|
||||
Event &operator*()
|
||||
{
|
||||
Q_PRE(m_engaged);
|
||||
return m_event;
|
||||
}
|
||||
|
||||
const Event *operator->() const
|
||||
{
|
||||
Q_PRE(m_engaged);
|
||||
return &m_event;
|
||||
}
|
||||
|
||||
Event *operator->()
|
||||
{
|
||||
Q_PRE(m_engaged);
|
||||
return &m_event;
|
||||
}
|
||||
};
|
||||
|
||||
class Q_GUI_EXPORT QMutableTouchEvent : public QTouchEvent
|
||||
{
|
||||
public:
|
||||
|
||||
@ -608,8 +608,7 @@ void QGraphicsViewPrivate::replayLastMouseEvent()
|
||||
{
|
||||
if (!useLastMouseEvent || !scene)
|
||||
return;
|
||||
QSinglePointEvent *spe = static_cast<QSinglePointEvent *>(&lastMouseEvent);
|
||||
mouseMoveEventHandler(static_cast<QMouseEvent *>(spe));
|
||||
mouseMoveEventHandler(&*lastMouseEvent);
|
||||
}
|
||||
|
||||
/*!
|
||||
@ -618,7 +617,8 @@ void QGraphicsViewPrivate::replayLastMouseEvent()
|
||||
void QGraphicsViewPrivate::storeMouseEvent(QMouseEvent *event)
|
||||
{
|
||||
useLastMouseEvent = true;
|
||||
lastMouseEvent = *event;
|
||||
// *event may alias *lastMouseEvent
|
||||
lastMouseEvent.storeUnlessAlias(*event);
|
||||
}
|
||||
|
||||
void QGraphicsViewPrivate::mouseMoveEventHandler(QMouseEvent *event)
|
||||
@ -630,7 +630,7 @@ void QGraphicsViewPrivate::mouseMoveEventHandler(QMouseEvent *event)
|
||||
#endif
|
||||
|
||||
storeMouseEvent(event);
|
||||
lastMouseEvent.setAccepted(false);
|
||||
lastMouseEvent->setAccepted(false);
|
||||
|
||||
if (!sceneInteractionAllowed)
|
||||
return;
|
||||
@ -662,7 +662,7 @@ void QGraphicsViewPrivate::mouseMoveEventHandler(QMouseEvent *event)
|
||||
QCoreApplication::sendEvent(scene, &mouseEvent);
|
||||
|
||||
// Remember whether the last event was accepted or not.
|
||||
lastMouseEvent.setAccepted(mouseEvent.isAccepted());
|
||||
lastMouseEvent->setAccepted(mouseEvent.isAccepted());
|
||||
|
||||
if (mouseEvent.isAccepted() && mouseEvent.buttons() != 0) {
|
||||
// The event was delivered to a mouse grabber; the press is likely to
|
||||
@ -819,7 +819,7 @@ void QGraphicsViewPrivate::_q_setViewportCursor(const QCursor &cursor)
|
||||
void QGraphicsViewPrivate::_q_unsetViewportCursor()
|
||||
{
|
||||
Q_Q(QGraphicsView);
|
||||
const auto items = q->items(lastMouseEvent.position().toPoint());
|
||||
const auto items = q->items(lastMouseEvent->position().toPoint());
|
||||
for (QGraphicsItem *item : items) {
|
||||
if (item->isEnabled() && item->hasCursor()) {
|
||||
_q_setViewportCursor(item->cursor());
|
||||
@ -3183,7 +3183,7 @@ void QGraphicsView::mouseDoubleClickEvent(QMouseEvent *event)
|
||||
event->setAccepted(isAccepted);
|
||||
|
||||
// Update the last mouse event accepted state.
|
||||
d->lastMouseEvent.setAccepted(isAccepted);
|
||||
d->lastMouseEvent->setAccepted(isAccepted);
|
||||
}
|
||||
|
||||
/*!
|
||||
@ -3197,7 +3197,7 @@ void QGraphicsView::mousePressEvent(QMouseEvent *event)
|
||||
// scroll-dragging; even in non-interactive mode, scroll hand dragging is
|
||||
// allowed, so we store the event at the very top of this function.
|
||||
d->storeMouseEvent(event);
|
||||
d->lastMouseEvent.setAccepted(false);
|
||||
d->lastMouseEvent->setAccepted(false);
|
||||
|
||||
if (d->sceneInteractionAllowed) {
|
||||
// Store some of the event's button-down data.
|
||||
@ -3235,7 +3235,7 @@ void QGraphicsView::mousePressEvent(QMouseEvent *event)
|
||||
event->setAccepted(isAccepted);
|
||||
|
||||
// Update the last mouse event accepted state.
|
||||
d->lastMouseEvent.setAccepted(isAccepted);
|
||||
d->lastMouseEvent->setAccepted(isAccepted);
|
||||
|
||||
if (isAccepted)
|
||||
return;
|
||||
@ -3284,7 +3284,7 @@ void QGraphicsView::mouseMoveEvent(QMouseEvent *event)
|
||||
if (d->handScrolling) {
|
||||
QScrollBar *hBar = horizontalScrollBar();
|
||||
QScrollBar *vBar = verticalScrollBar();
|
||||
QPoint delta = event->position().toPoint() - d->lastMouseEvent.position().toPoint();
|
||||
QPoint delta = event->position().toPoint() - d->lastMouseEvent->position().toPoint();
|
||||
hBar->setValue(hBar->value() + (isRightToLeft() ? delta.x() : -delta.x()));
|
||||
vBar->setValue(vBar->value() - delta.y());
|
||||
|
||||
@ -3318,7 +3318,7 @@ void QGraphicsView::mouseReleaseEvent(QMouseEvent *event)
|
||||
#endif
|
||||
d->handScrolling = false;
|
||||
|
||||
if (d->scene && d->sceneInteractionAllowed && !d->lastMouseEvent.isAccepted() && d->handScrollMotions <= 6) {
|
||||
if (d->scene && d->sceneInteractionAllowed && !d->lastMouseEvent->isAccepted() && d->handScrollMotions <= 6) {
|
||||
// If we've detected very little motion during the hand drag, and
|
||||
// no item accepted the last event, we'll interpret that as a
|
||||
// click to the scene, and reset the selection.
|
||||
@ -3355,7 +3355,7 @@ void QGraphicsView::mouseReleaseEvent(QMouseEvent *event)
|
||||
QCoreApplication::sendEvent(d->scene, &mouseEvent);
|
||||
|
||||
// Update the last and current mouse events' accepted state.
|
||||
d->lastMouseEvent.setAccepted(mouseEvent.isAccepted());
|
||||
d->lastMouseEvent->setAccepted(mouseEvent.isAccepted());
|
||||
event->setAccepted(mouseEvent.isAccepted());
|
||||
|
||||
#ifndef QT_NO_CURSOR
|
||||
|
||||
@ -89,7 +89,7 @@ public:
|
||||
qreal topIndent;
|
||||
|
||||
// Replaying mouse events
|
||||
QMutableSinglePointEvent lastMouseEvent;
|
||||
QEventStorage<QMouseEvent> lastMouseEvent;
|
||||
void replayLastMouseEvent();
|
||||
void storeMouseEvent(QMouseEvent *event);
|
||||
void mouseMoveEventHandler(QMouseEvent *event);
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user