Skip to content

Commit

Permalink
Fix touch point positions of non-transformable QGraphicsItems
Browse files Browse the repository at this point in the history 
TouchEvent::TouchPoint::pos was not updated in
QGraphicsScenePrivate::updateTouchPointsForItem().

To prevent the transformation being calculated repeatedly for each touch
point member, extract a function genericMapFromSceneTransform()
from genericMapFromScene() returning the transformation and use
that whereever multiple points are transformed.

Add a test, extracting helper functionality from
tst_QGraphicsItem::touchEventPropagation().
In addition, fold tst_QGraphicsScene::checkTouchPointsEllipseDiameters() from
c48f4bd into this test, so that
it is testing all transformations.

Task-number: QTBUG-66192
Change-Id: If71886d2c14c4e216f7781ea2f22f1adc444e6cf
Reviewed-by: Andy Shaw <[email protected]>
Reviewed-by: Richard Moe Gustavsen <[email protected]>
  • Loading branch information
@FriedemannKleint
FriedemannKleint committed Feb 20, 2018
1 parent 76617fd commit ed3ed0b
Show file tree
Hide file tree
Showing 5 changed files with 225 additions and 128 deletions.
25 changes: 16 additions & 9 deletions src/widgets/graphicsview/qgraphicsitem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1134,19 +1134,26 @@ void QGraphicsItemPrivate::remapItemPos(QEvent *event, QGraphicsItem *item)
is untransformable, this function will correctly map \a pos from the scene using the
view's transformation.
*/
QPointF QGraphicsItemPrivate::genericMapFromScene(const QPointF &pos,
const QWidget *viewport) const

QTransform QGraphicsItemPrivate::genericMapFromSceneTransform(const QWidget *viewport) const
{
Q_Q(const QGraphicsItem);
if (!itemIsUntransformable())
return q->mapFromScene(pos);
QGraphicsView *view = 0;
if (viewport)
view = qobject_cast<QGraphicsView *>(viewport->parentWidget());
if (!view)
return q->mapFromScene(pos);
return sceneTransform.inverted();
const QGraphicsView *view = viewport
? qobject_cast<QGraphicsView *>(viewport->parentWidget())
: nullptr;
if (view == nullptr)
return sceneTransform.inverted();
// ### More ping pong than needed.
return q->deviceTransform(view->viewportTransform()).inverted().map(view->mapFromScene(pos));
const QTransform viewportTransform = view->viewportTransform();
return viewportTransform * q->deviceTransform(viewportTransform).inverted();
}

QPointF QGraphicsItemPrivate::genericMapFromScene(const QPointF &pos,
const QWidget *viewport) const
{
return genericMapFromSceneTransform(viewport).map(pos);
}

/*!
Expand Down
1 change: 1 addition & 0 deletions src/widgets/graphicsview/qgraphicsitem_p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ class Q_WIDGETS_EXPORT QGraphicsItemPrivate
void updateAncestorFlags();
void setIsMemberOfGroup(bool enabled);
void remapItemPos(QEvent *event, QGraphicsItem *item);
QTransform genericMapFromSceneTransform(const QWidget *viewport = nullptr) const;
QPointF genericMapFromScene(const QPointF &pos, const QWidget *viewport) const;
inline bool itemIsUntransformable() const
{
Expand Down
26 changes: 18 additions & 8 deletions src/widgets/graphicsview/qgraphicsscene.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1286,10 +1286,11 @@ void QGraphicsScenePrivate::sendHoverEvent(QEvent::Type type, QGraphicsItem *ite
{
QGraphicsSceneHoverEvent event(type);
event.setWidget(hoverEvent->widget());
event.setPos(item->d_ptr->genericMapFromScene(hoverEvent->scenePos(), hoverEvent->widget()));
const QTransform mapFromScene = item->d_ptr->genericMapFromSceneTransform(hoverEvent->widget());
event.setPos(mapFromScene.map(hoverEvent->scenePos()));
event.setScenePos(hoverEvent->scenePos());
event.setScreenPos(hoverEvent->screenPos());
event.setLastPos(item->d_ptr->genericMapFromScene(hoverEvent->lastScenePos(), hoverEvent->widget()));
event.setLastPos(mapFromScene.map(hoverEvent->lastScenePos()));
event.setLastScenePos(hoverEvent->lastScenePos());
event.setLastScreenPos(hoverEvent->lastScreenPos());
event.setModifiers(hoverEvent->modifiers());
Expand All @@ -1312,14 +1313,16 @@ void QGraphicsScenePrivate::sendMouseEvent(QGraphicsSceneMouseEvent *mouseEvent)
if (item->isBlockedByModalPanel())
return;

const QTransform mapFromScene = item->d_ptr->genericMapFromSceneTransform(mouseEvent->widget());
const QPointF itemPos = mapFromScene.map(mouseEvent->scenePos());
for (int i = 0x1; i <= 0x10; i <<= 1) {
Qt::MouseButton button = Qt::MouseButton(i);
mouseEvent->setButtonDownPos(button, mouseGrabberButtonDownPos.value(button, item->d_ptr->genericMapFromScene(mouseEvent->scenePos(), mouseEvent->widget())));
mouseEvent->setButtonDownPos(button, mouseGrabberButtonDownPos.value(button, itemPos));
mouseEvent->setButtonDownScenePos(button, mouseGrabberButtonDownScenePos.value(button, mouseEvent->scenePos()));
mouseEvent->setButtonDownScreenPos(button, mouseGrabberButtonDownScreenPos.value(button, mouseEvent->screenPos()));
}
mouseEvent->setPos(item->d_ptr->genericMapFromScene(mouseEvent->scenePos(), mouseEvent->widget()));
mouseEvent->setLastPos(item->d_ptr->genericMapFromScene(mouseEvent->lastScenePos(), mouseEvent->widget()));
mouseEvent->setPos(itemPos);
mouseEvent->setLastPos(mapFromScene.map(mouseEvent->lastScenePos()));
sendEvent(item, mouseEvent);
}

Expand Down Expand Up @@ -5858,10 +5861,17 @@ void QGraphicsScenePrivate::removeView(QGraphicsView *view)

void QGraphicsScenePrivate::updateTouchPointsForItem(QGraphicsItem *item, QTouchEvent *touchEvent)
{
const QTransform mapFromScene =
item->d_ptr->genericMapFromSceneTransform(static_cast<const QWidget *>(touchEvent->target()));

for (auto &touchPoint : touchEvent->_touchPoints) {
touchPoint.setRect(item->mapFromScene(touchPoint.sceneRect()).boundingRect());
touchPoint.setStartPos(item->d_ptr->genericMapFromScene(touchPoint.startScenePos(), static_cast<QWidget *>(touchEvent->target())));
touchPoint.setLastPos(item->d_ptr->genericMapFromScene(touchPoint.lastScenePos(), static_cast<QWidget *>(touchEvent->target())));
// Deprecated TouchPoint::setRect clobbers ellipseDiameters, restore
const QSizeF ellipseDiameters = touchPoint.ellipseDiameters();
touchPoint.setRect(mapFromScene.map(touchPoint.sceneRect()).boundingRect());
touchPoint.setEllipseDiameters(ellipseDiameters);
touchPoint.setPos(mapFromScene.map(touchPoint.scenePos()));
touchPoint.setStartPos(mapFromScene.map(touchPoint.startScenePos()));
touchPoint.setLastPos(mapFromScene.map(touchPoint.lastScenePos()));
}
}

Expand Down
225 changes: 190 additions & 35 deletions tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <QAbstractTextDocumentLayout>
#include <QBitmap>
#include <QCursor>
#include <QDesktopWidget>
#include <QScreen>
#include <QLabel>
#include <QDial>
Expand All @@ -54,10 +55,13 @@
#include <QPushButton>
#include <QLineEdit>
#include <QGraphicsLinearLayout>
#include <QTransform>
#include <float.h>
#include <QStyleHints>

Q_DECLARE_METATYPE(QPainterPath)
Q_DECLARE_METATYPE(QSizeF)
Q_DECLARE_METATYPE(QTransform)

#if defined(Q_OS_WIN) && !defined(Q_OS_WINRT)
#include <windows.h>
Expand Down Expand Up @@ -435,6 +439,8 @@ private slots:
void focusHandling();
void touchEventPropagation_data();
void touchEventPropagation();
void touchEventTransformation_data();
void touchEventTransformation();
void deviceCoordinateCache_simpleRotations();
void resolvePaletteForItemChildren();

Expand Down Expand Up @@ -465,6 +471,7 @@ private slots:

private:
QList<QGraphicsItem *> paintedItems;
QTouchDevice *m_touchDevice = nullptr;
};

void tst_QGraphicsItem::construction()
Expand Down Expand Up @@ -10945,6 +10952,95 @@ void tst_QGraphicsItem::focusHandling()
QCOMPARE(scene.focusItem(), focusableUnder);
}

class TouchEventTestee : public QGraphicsRectItem
{
public:
TouchEventTestee(const QSizeF &size = QSizeF(100, 100)) :
QGraphicsRectItem(QRectF(QPointF(), size))
{
setAcceptTouchEvents(true);
setFlag(QGraphicsItem::ItemIsFocusable, false);
}

QList<QTouchEvent::TouchPoint> touchBeginPoints() const { return m_touchBeginPoints; }
int touchBeginEventCount() const { return m_touchBeginPoints.size(); }

QList<QTouchEvent::TouchPoint> touchUpdatePoints() const { return m_touchUpdatePoints; }
int touchUpdateEventCount() const { return m_touchUpdatePoints.size(); }

protected:
bool sceneEvent(QEvent *ev) override
{
switch (ev->type()) {
case QEvent::TouchBegin:
m_touchBeginPoints.append(static_cast<const QTouchEvent *>(ev)->touchPoints().constFirst());
ev->accept();
return true;
case QEvent::TouchUpdate:
m_touchUpdatePoints.append(static_cast<const QTouchEvent *>(ev)->touchPoints().constFirst());
ev->accept();
return true;
default:
break;
}

return QGraphicsRectItem::sceneEvent(ev);
}

private:
QList<QTouchEvent::TouchPoint> m_touchBeginPoints;
QList<QTouchEvent::TouchPoint> m_touchUpdatePoints;
};

static QList<QTouchEvent::TouchPoint>
createTouchPoints(const QGraphicsView &view,
const QPointF &scenePos,
const QSizeF &ellipseDiameters,
Qt::TouchPointState state = Qt::TouchPointPressed)
{
QTouchEvent::TouchPoint tp(0);
tp.setState(state);
tp.setScenePos(scenePos);
tp.setStartScenePos(scenePos);
tp.setLastScenePos(scenePos);
const QPointF screenPos = view.viewport()->mapToGlobal(view.mapFromScene(scenePos));
tp.setScreenPos(screenPos);
tp.setStartScreenPos(screenPos);
tp.setLastScreenPos(screenPos);
tp.setEllipseDiameters(ellipseDiameters);
const QSizeF screenSize = QApplication::desktop()->screenGeometry(&view).size();
tp.setNormalizedPos(QPointF(screenPos.x() / screenSize.width(), screenPos.y() / screenSize.height()));
return QList<QTouchEvent::TouchPoint>() << tp;
}

static bool comparePointF(const QPointF &p1, const QPointF &p2)
{
return qFuzzyCompare(p1.x(), p2.x()) && qFuzzyCompare(p1.y(), p2.y());
}

static bool compareSizeF(const QSizeF &s1, const QSizeF &s2)
{
return qFuzzyCompare(s1.width(), s2.width()) && qFuzzyCompare(s1.height(), s2.height());
}

static QByteArray msgPointFComparisonFailed(const QPointF &p1, const QPointF &p2)
{
return QByteArray::number(p1.x()) + ", " + QByteArray::number(p1.y())
+ " != " + QByteArray::number(p2.x()) + ", " + QByteArray::number(p2.y());
}

static QByteArray msgSizeFComparisonFailed(const QSizeF &s1, const QSizeF &s2)
{
return QByteArray::number(s1.width()) + 'x' + QByteArray::number(s1.height())
+ " != " + QByteArray::number(s2.width()) + 'x' + QByteArray::number(s2.height());
}

#define COMPARE_POINTF(ACTUAL, EXPECTED) \
QVERIFY2(comparePointF(ACTUAL, EXPECTED), msgPointFComparisonFailed(ACTUAL, EXPECTED).constData())

#define COMPARE_SIZEF(ACTUAL, EXPECTED) \
QVERIFY2(compareSizeF(ACTUAL, EXPECTED), msgSizeFComparisonFailed(ACTUAL, EXPECTED).constData())

void tst_QGraphicsItem::touchEventPropagation_data()
{
QTest::addColumn<QGraphicsItem::GraphicsItemFlag>("flag");
Expand All @@ -10963,29 +11059,7 @@ void tst_QGraphicsItem::touchEventPropagation()
QFETCH(QGraphicsItem::GraphicsItemFlag, flag);
QFETCH(int, expectedCount);

class Testee : public QGraphicsRectItem
{
public:
int touchBeginEventCount;

Testee()
: QGraphicsRectItem(0, 0, 100, 100)
, touchBeginEventCount(0)
{
setAcceptTouchEvents(true);
setFlag(QGraphicsItem::ItemIsFocusable, false);
}

bool sceneEvent(QEvent *ev)
{
if (ev->type() == QEvent::TouchBegin)
++touchBeginEventCount;

return QGraphicsRectItem::sceneEvent(ev);
}
};

Testee *touchEventReceiver = new Testee;
TouchEventTestee *touchEventReceiver = new TouchEventTestee;
QGraphicsItem *topMost = new QGraphicsRectItem(touchEventReceiver->boundingRect());

QGraphicsScene scene;
Expand All @@ -10998,26 +11072,107 @@ void tst_QGraphicsItem::touchEventPropagation()
topMost->setFlag(flag, true);

QGraphicsView view(&scene);
view.setWindowTitle(QLatin1String(QTest::currentTestFunction()) + QLatin1String("::")
+ QLatin1String(QTest::currentDataTag()));
view.setSceneRect(touchEventReceiver->boundingRect());
view.show();
QVERIFY(QTest::qWaitForWindowExposed(&view));

QCOMPARE(touchEventReceiver->touchBeginEventCount, 0);
QCOMPARE(touchEventReceiver->touchBeginEventCount(), 0);

QTouchEvent::TouchPoint tp(0);
tp.setState(Qt::TouchPointPressed);
tp.setScenePos(view.sceneRect().center());
tp.setLastScenePos(view.sceneRect().center());
const QPointF scenePos = view.sceneRect().center();
sendMousePress(&scene, scenePos);
if (m_touchDevice == nullptr)
m_touchDevice = QTest::createTouchDevice();
QTouchEvent touchBegin(QEvent::TouchBegin, m_touchDevice, Qt::NoModifier, Qt::TouchPointPressed,
createTouchPoints(view, scenePos, QSizeF(10, 10)));
touchBegin.setTarget(view.viewport());

QList<QTouchEvent::TouchPoint> touchPoints;
touchPoints << tp;
qApp->sendEvent(&scene, &touchBegin);
QCOMPARE(touchEventReceiver->touchBeginEventCount(), expectedCount);
}

sendMousePress(&scene, tp.scenePos());
QTouchDevice *device = QTest::createTouchDevice();
QTouchEvent touchBegin(QEvent::TouchBegin, device, Qt::NoModifier, Qt::TouchPointPressed, touchPoints);
void tst_QGraphicsItem::touchEventTransformation_data()
{
QTest::addColumn<QGraphicsItem::GraphicsItemFlag>("flag");
QTest::addColumn<QTransform>("viewTransform");
QTest::addColumn<QPointF>("touchScenePos");
QTest::addColumn<QSizeF>("ellipseDiameters");
QTest::addColumn<QPointF>("expectedItemPos");

QTest::newRow("notransform")
<< QGraphicsItem::ItemIsSelectable << QTransform()
<< QPointF(150, 150) << QSizeF(7, 8) << QPointF(50, 50);
QTest::newRow("scaled")
<< QGraphicsItem::ItemIsSelectable << QTransform::fromScale(0.5, 0.5)
<< QPointF(150, 150) << QSizeF(7, 8) << QPointF(50, 50);
// QTBUG-66192: When the item ignores the downscaling transformation,
// it will receive the touch point at 25,25 instead of 50,50.
QTest::newRow("scaled/ItemIgnoresTransformations")
<< QGraphicsItem::ItemIgnoresTransformations << QTransform::fromScale(0.5, 0.5)
<< QPointF(150, 150) << QSizeF(7, 8) << QPointF(25, 25);
}

void tst_QGraphicsItem::touchEventTransformation()
{
QFETCH(QGraphicsItem::GraphicsItemFlag, flag);
QFETCH(QTransform, viewTransform);
QFETCH(QPointF, touchScenePos);
QFETCH(QSizeF, ellipseDiameters);
QFETCH(QPointF, expectedItemPos);

TouchEventTestee *touchEventReceiver = new TouchEventTestee;

QGraphicsScene scene;
scene.addItem(touchEventReceiver);
const QPointF itemPos(100, 100);

touchEventReceiver->setPos(itemPos);

touchEventReceiver->setFlag(flag, true);

QGraphicsView view(&scene);
view.setWindowTitle(QLatin1String(QTest::currentTestFunction()) + QLatin1String("::")
+ QLatin1String(QTest::currentDataTag()));
view.setSceneRect(QRectF(QPointF(0, 0), QSizeF(300, 300)));
view.setTransform(viewTransform);
view.show();
QVERIFY(QTest::qWaitForWindowExposed(&view));

QCOMPARE(touchEventReceiver->touchBeginEventCount(), 0);

if (m_touchDevice == nullptr)
m_touchDevice = QTest::createTouchDevice();
QTouchEvent touchBegin(QEvent::TouchBegin, m_touchDevice, Qt::NoModifier, Qt::TouchPointPressed,
createTouchPoints(view, touchScenePos, ellipseDiameters));
touchBegin.setTarget(view.viewport());

QCoreApplication::sendEvent(&scene, &touchBegin);
QCOMPARE(touchEventReceiver->touchBeginEventCount(), 1);

const QTouchEvent::TouchPoint touchBeginPoint = touchEventReceiver->touchBeginPoints().constFirst();

COMPARE_POINTF(touchBeginPoint.scenePos(), touchScenePos);
COMPARE_POINTF(touchBeginPoint.startScenePos(), touchScenePos);
COMPARE_POINTF(touchBeginPoint.lastScenePos(), touchScenePos);
COMPARE_POINTF(touchBeginPoint.pos(), expectedItemPos);
COMPARE_SIZEF(touchBeginPoint.ellipseDiameters(), ellipseDiameters); // Must remain untransformed

QTouchEvent touchUpdate(QEvent::TouchUpdate, m_touchDevice, Qt::NoModifier, Qt::TouchPointMoved,
createTouchPoints(view, touchScenePos, ellipseDiameters, Qt::TouchPointMoved));
touchUpdate.setTarget(view.viewport());

QCoreApplication::sendEvent(&scene, &touchUpdate);
QCOMPARE(touchEventReceiver->touchUpdateEventCount(), 1);

const QTouchEvent::TouchPoint touchUpdatePoint = touchEventReceiver->touchUpdatePoints().constFirst();

COMPARE_POINTF(touchUpdatePoint.scenePos(), touchScenePos);
COMPARE_POINTF(touchBeginPoint.startScenePos(), touchScenePos);
COMPARE_POINTF(touchUpdatePoint.lastScenePos(), touchScenePos);
COMPARE_POINTF(touchUpdatePoint.pos(), expectedItemPos);
COMPARE_SIZEF(touchUpdatePoint.ellipseDiameters(), ellipseDiameters); // Must remain untransformed

qApp->sendEvent(&scene, &touchBegin);
QCOMPARE(touchEventReceiver->touchBeginEventCount, expectedCount);
}

void tst_QGraphicsItem::deviceCoordinateCache_simpleRotations()
Expand Down
Loading

0 comments on commit ed3ed0b

Please sign in to comment.