From 481cea71043dabf6d5ff33101a66693af86438e1 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Wed, 4 Sep 2019 10:12:32 +0200 Subject: Compositor: Fix various input-related rounding errors QPointF::toPoint (which is what the QPoint versions of the events use internally) uses qRound, which is not the right kind of rounding to use with the QRegion we use for the input region. This switches to use QPointF variants instead of QPoint wherever possible, and then the correct conversion (with qFloor) is done once in the new QPointF version of QWaylandSurface::inputRegionContains(). The compositor inputRegion test has now been updated to test the new API. [ChangeLog][Compositor] Fixed various rounding errors related to touch and mouse input. Fixes: QTBUG-77457 Change-Id: Ife2365abd56a239c34eee91310ab0e698a50d4ff Reviewed-by: Paul Olav Tvete (cherry picked from commit 7f189ec10a9b3e9825dda30d3a8f86ee2e07b97f) Reviewed-by: Pier Luigi Fiorini --- .../compositor_api/qwaylandquickitem.cpp | 28 +++++++++++----------- .../compositor_api/qwaylandquickitem_p.h | 2 +- src/compositor/compositor_api/qwaylandsurface.cpp | 18 ++++++++++++++ src/compositor/compositor_api/qwaylandsurface.h | 7 ++++++ 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/compositor/compositor_api/qwaylandquickitem.cpp b/src/compositor/compositor_api/qwaylandquickitem.cpp index 2b24c13b7..5010247f9 100644 --- a/src/compositor/compositor_api/qwaylandquickitem.cpp +++ b/src/compositor/compositor_api/qwaylandquickitem.cpp @@ -465,7 +465,7 @@ void QWaylandQuickItem::mousePressEvent(QMouseEvent *event) return; } - if (!inputRegionContains(event->pos())) { + if (!inputRegionContains(event->localPos())) { event->ignore(); return; } @@ -477,7 +477,7 @@ void QWaylandQuickItem::mousePressEvent(QMouseEvent *event) seat->sendMouseMoveEvent(d->view.data(), mapToSurface(event->localPos()), event->windowPos()); seat->sendMousePressEvent(event->button()); - d->hoverPos = event->pos(); + d->hoverPos = event->localPos(); } /*! @@ -503,7 +503,7 @@ void QWaylandQuickItem::mouseMoveEvent(QMouseEvent *event) #endif // QT_CONFIG(draganddrop) { seat->sendMouseMoveEvent(d->view.data(), mapToSurface(event->localPos()), event->windowPos()); - d->hoverPos = event->pos(); + d->hoverPos = event->localPos(); } } else { emit mouseMove(event->windowPos()); @@ -540,14 +540,14 @@ void QWaylandQuickItem::mouseReleaseEvent(QMouseEvent *event) void QWaylandQuickItem::hoverEnterEvent(QHoverEvent *event) { Q_D(QWaylandQuickItem); - if (!inputRegionContains(event->pos())) { + if (!inputRegionContains(event->posF())) { event->ignore(); return; } if (d->shouldSendInputEvents()) { QWaylandSeat *seat = compositor()->seatFor(event); - seat->sendMouseMoveEvent(d->view.data(), event->pos(), mapToScene(event->pos())); - d->hoverPos = event->pos(); + seat->sendMouseMoveEvent(d->view.data(), event->posF(), mapToScene(event->posF())); + d->hoverPos = event->posF(); } else { event->ignore(); } @@ -560,16 +560,16 @@ void QWaylandQuickItem::hoverMoveEvent(QHoverEvent *event) { Q_D(QWaylandQuickItem); if (surface()) { - if (!inputRegionContains(event->pos())) { + if (!inputRegionContains(event->posF())) { event->ignore(); return; } } if (d->shouldSendInputEvents()) { QWaylandSeat *seat = compositor()->seatFor(event); - if (event->pos() != d->hoverPos) { - seat->sendMouseMoveEvent(d->view.data(), mapToSurface(event->pos()), mapToScene(event->pos())); - d->hoverPos = event->pos(); + if (event->posF() != d->hoverPos) { + seat->sendMouseMoveEvent(d->view.data(), mapToSurface(event->posF()), mapToScene(event->posF())); + d->hoverPos = event->posF(); } } else { event->ignore(); @@ -598,7 +598,7 @@ void QWaylandQuickItem::wheelEvent(QWheelEvent *event) { Q_D(QWaylandQuickItem); if (d->shouldSendInputEvents()) { - if (!inputRegionContains(event->pos())) { + if (!inputRegionContains(event->posF())) { event->ignore(); return; } @@ -651,10 +651,10 @@ void QWaylandQuickItem::touchEvent(QTouchEvent *event) if (d->shouldSendInputEvents() && d->touchEventsEnabled) { QWaylandSeat *seat = compositor()->seatFor(event); - QPoint pointPos; + QPointF pointPos; const QList &points = event->touchPoints(); if (!points.isEmpty()) - pointPos = points.at(0).pos().toPoint(); + pointPos = points.at(0).pos(); if (event->type() == QEvent::TouchBegin && !inputRegionContains(pointPos)) { event->ignore(); @@ -1039,7 +1039,7 @@ void QWaylandQuickItem::setFocusOnClick(bool focus) bool QWaylandQuickItem::inputRegionContains(const QPointF &localPosition) const { if (QWaylandSurface *s = surface()) - return s->inputRegionContains(mapToSurface(localPosition).toPoint()); + return s->inputRegionContains(mapToSurface(localPosition)); return false; } diff --git a/src/compositor/compositor_api/qwaylandquickitem_p.h b/src/compositor/compositor_api/qwaylandquickitem_p.h index 3d710d71b..352a130dc 100644 --- a/src/compositor/compositor_api/qwaylandquickitem_p.h +++ b/src/compositor/compositor_api/qwaylandquickitem_p.h @@ -171,7 +171,7 @@ public: bool focusOnClick = true; bool sizeFollowsSurface = true; bool belowParent = false; - QPoint hoverPos; + QPointF hoverPos; QMatrix4x4 lastMatrix; QQuickWindow *connectedWindow = nullptr; diff --git a/src/compositor/compositor_api/qwaylandsurface.cpp b/src/compositor/compositor_api/qwaylandsurface.cpp index f457c372c..e7bb0d7de 100644 --- a/src/compositor/compositor_api/qwaylandsurface.cpp +++ b/src/compositor/compositor_api/qwaylandsurface.cpp @@ -66,6 +66,7 @@ #include #include +#include QT_BEGIN_NAMESPACE @@ -601,6 +602,23 @@ bool QWaylandSurface::inputRegionContains(const QPoint &p) const return d->inputRegion.contains(p); } +//TODO: Add appropriate \since version when this is made public. +/*! + * Returns \c true if the QWaylandSurface's input region contains the point \a position. + * Otherwise returns \c false. + */ +bool QWaylandSurface::inputRegionContains(const QPointF &position) const +{ + Q_D(const QWaylandSurface); + // QRegion::contains operates in integers. If a region has a rect (0,0,10,10), (0,0) is + // inside while (10,10) is outside. Therefore, we can't use QPoint::toPoint(), which will + // round upwards, meaning the point (-0.25,-0.25) would be rounded to (0,0) and count as + // being inside the region, and similarly, a point (9.75,9.75) inside the region would be + // rounded upwards and count as being outside the region. + const QPoint floored(qFloor(position.x()), qFloor(position.y())); + return d->inputRegion.contains(floored); +} + /*! * \qmlmethod void QtWaylandCompositor::WaylandSurface::destroy() * diff --git a/src/compositor/compositor_api/qwaylandsurface.h b/src/compositor/compositor_api/qwaylandsurface.h index a138b2af5..c4d80d2bf 100644 --- a/src/compositor/compositor_api/qwaylandsurface.h +++ b/src/compositor/compositor_api/qwaylandsurface.h @@ -120,6 +120,13 @@ public: QWaylandCompositor *compositor() const; bool inputRegionContains(const QPoint &p) const; +private: + // TODO: Making this private now since it's added in a patch release, and we want to ensure + // compatibility with older patch releases. + // This should simply be made public (and the friend removed) in the next minor release. + friend class QWaylandQuickItem; + bool inputRegionContains(const QPointF &position) const; +public: Q_INVOKABLE void destroy(); Q_INVOKABLE bool isDestroyed() const; -- cgit v1.2.3