AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QGraphicsView: fix jumping graphic items when dragging out of view

Browse Source 
The algorithms for calculating the scene's position within the view
did not compensate for scrollbars showing. The scrollbars should be
ignored when positioning hte scene within the view, as alignment
only cares about the positioning of the scene when the view is
larger than the scene anyway.

Add a test case that verifies that items don't jump up or down when
dragging horizontally, and not left or right when dragging
vertically.

Mark variables in the modified function as const where applicable to
make it easier to follow the code.

Done-with: Volker Hilsheimer <volker.hilsheimer@qt.io>
Fixes: QTBUG-46757
Change-Id: If205637dfe124e0034f68201b23f174d6863084d
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Taras Kachmaryk 2022-11-25 15:58:43 +02:00
parent 3ee7a9f85c
commit f0cd18706f
2 changed files with 137 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
src/widgets/graphicsview/qgraphicsview.cpp
View File

@ -335,10 +335,10 @@ void QGraphicsViewPrivate::recalculateContentSize()
{
Q_Q(QGraphicsView);
QSize maxSize = q->maximumViewportSize();
const QSize maxSize = q->maximumViewportSize();
int width = maxSize.width();
int height = maxSize.height();
QRectF viewRect = matrix.mapRect(q->sceneRect());
const QRectF viewRect = matrix.mapRect(q->sceneRect());
bool frameOnlyAround = (q->style()->styleHint(QStyle::SH_ScrollView_FrameOnlyAroundContents, nullptr, q));
if (frameOnlyAround) {
@ -350,9 +350,8 @@ void QGraphicsViewPrivate::recalculateContentSize()
// Adjust the maximum width and height of the viewport based on the width
// of visible scroll bars.
int scrollBarExtent = q->style()->pixelMetric(QStyle::PM_ScrollBarExtent, nullptr, q);
if (frameOnlyAround)
scrollBarExtent += frameWidth * 2;
const int scrollBarExtent = q->style()->pixelMetric(QStyle::PM_ScrollBarExtent, nullptr, q)
+ (frameOnlyAround ? frameWidth * 2 : 0);
// We do not need to subtract the width scrollbars whose policy is
// Qt::ScrollBarAlwaysOn, this was already done by maximumViewportSize().
@ -374,62 +373,70 @@ void QGraphicsViewPrivate::recalculateContentSize()
// Setting the ranges of these scroll bars can/will cause the values to
// change, and scrollContentsBy() will be called correspondingly. This
// will reset the last center point.
QPointF savedLastCenterPoint = lastCenterPoint;
const QPointF savedLastCenterPoint = lastCenterPoint;
// Remember the former indent settings
qreal oldLeftIndent = leftIndent;
qreal oldTopIndent = topIndent;
const qreal oldLeftIndent = leftIndent;
const qreal oldTopIndent = topIndent;
// If the whole scene fits horizontally, we center the scene horizontally,
// and ignore the horizontal scroll bars.
int left = q_round_bound(viewRect.left());
int right = q_round_bound(viewRect.right() - width);
const int left = q_round_bound(viewRect.left());
const int right = q_round_bound(viewRect.right() - width);
if (left >= right) {
hbar->setRange(0, 0);
switch (alignment & Qt::AlignHorizontal_Mask) {
case Qt::AlignLeft:
leftIndent = -viewRect.left();
break;
case Qt::AlignRight:
leftIndent = width - viewRect.width() - viewRect.left() - 1;
leftIndent = maxSize.width() - viewRect.width() - viewRect.left() - 1;
break;
case Qt::AlignHCenter:
default:
leftIndent = width / 2 - (viewRect.left() + viewRect.right()) / 2;
leftIndent = maxSize.width() / 2 - (viewRect.left() + viewRect.right()) / 2;
break;
}
hbar->setRange(0, 0);
} else {
leftIndent = 0;
hbar->setRange(left, right);
hbar->setPageStep(width);
hbar->setSingleStep(width / 20);
leftIndent = 0;
if (oldLeftIndent != 0)
hbar->setValue(-oldLeftIndent);
}
// If the whole scene fits vertically, we center the scene vertically, and
// ignore the vertical scroll bars.
int top = q_round_bound(viewRect.top());
int bottom = q_round_bound(viewRect.bottom() - height);
const int top = q_round_bound(viewRect.top());
const int bottom = q_round_bound(viewRect.bottom() - height);
if (top >= bottom) {
vbar->setRange(0, 0);
switch (alignment & Qt::AlignVertical_Mask) {
case Qt::AlignTop:
topIndent = -viewRect.top();
break;
case Qt::AlignBottom:
topIndent = height - viewRect.height() - viewRect.top() - 1;
topIndent = maxSize.height() - viewRect.height() - viewRect.top() - 1;
break;
case Qt::AlignVCenter:
default:
topIndent = height / 2 - (viewRect.top() + viewRect.bottom()) / 2;
topIndent = maxSize.height() / 2 - (viewRect.top() + viewRect.bottom()) / 2;
break;
}
vbar->setRange(0, 0);
} else {
topIndent = 0;
vbar->setRange(top, bottom);
vbar->setPageStep(height);
vbar->setSingleStep(height / 20);
topIndent = 0;
if (oldTopIndent != 0)
vbar->setValue(-oldTopIndent);
}
// Restorethe center point from before the ranges changed.

108
tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp
View File

@ -248,6 +248,8 @@ private slots:
#ifndef QT_NO_CURSOR
void QTBUG_7438_cursor();
#endif
void resizeContentsOnItemDrag_data();
void resizeContentsOnItemDrag();
public slots:
void dummySlot() {}
@ -4960,5 +4962,111 @@ void tst_QGraphicsView::QTBUG_70255_scrollTo()
QCOMPARE(point, QPoint(0, -500));
}
void tst_QGraphicsView::resizeContentsOnItemDrag_data()
{
QTest::addColumn<Qt::Alignment>("alignment");
QTest::addColumn<Qt::Orientation>("orientation");
QTest::addRow("Center right") << Qt::Alignment(Qt::AlignCenter) << Qt::Horizontal;
QTest::addRow("Center down") << Qt::Alignment(Qt::AlignCenter) << Qt::Vertical;
QTest::addRow("BottomLeft right") << (Qt::AlignBottom | Qt::AlignLeft) << Qt::Horizontal;
QTest::addRow("TopRight down") << (Qt::AlignTop | Qt::AlignRight) << Qt::Vertical;
}
void tst_QGraphicsView::resizeContentsOnItemDrag()
{
QFETCH(Qt::Alignment, alignment);
QFETCH(Qt::Orientation, orientation);
QGraphicsView view;
QGraphicsScene scene;
view.setFixedSize(200, 200);
view.setScene(&scene);
view.setAlignment(alignment);
class MovableItem : public QGraphicsEllipseItem
{
public:
using QGraphicsEllipseItem::QGraphicsEllipseItem;
QList<QPointF> scenePositions;
protected:
void mousePressEvent(QGraphicsSceneMouseEvent *event) override
{
scenePositions << event->scenePos();
}
void mouseMoveEvent(QGraphicsSceneMouseEvent *event) override
{
scenePositions << event->scenePos();
QGraphicsEllipseItem::mouseMoveEvent(event);
}
};
MovableItem item(-10, -10, 20, 20);
item.setFlags(QGraphicsItem::ItemIsMovable);
scene.addItem(&item);
view.show();
QVERIFY(QTest::qWaitForWindowExposed(&view));
// Position the item near the relevant edge of the view, with a few pixels
// to go until the scrollbars should be showing.
if (orientation == Qt::Horizontal)
item.setPos(view.width() - item.rect().width() - 5, 0);
else
item.setPos(0, view.height() - item.rect().height() - 5);
QApplication::processEvents(); // queued connection used to trigger recalculateContentSize
QPoint mousePos = view.mapFromScene(item.pos());
QTest::mousePress(view.viewport(), Qt::LeftButton, {}, mousePos);
QCOMPARE(item.scenePositions.count(), 1);
QCOMPARE(item.scenePositions.takeLast(), view.mapToScene(mousePos));
auto lastItemPos = item.pos();
auto lastScenePos = view.mapToScene(mousePos);
int overshoot = 0;
const QScrollBar *scrollBar = orientation == Qt::Horizontal
? view.horizontalScrollBar()
: view.verticalScrollBar();
// Drag the item until the scroll bars become visible, and then for a few more pixels.
// Verify that the item doesn't jump when the scrollbar shows.
while (overshoot < 10) {
if (orientation == Qt::Horizontal)
mousePos.rx() += 1;
else
mousePos.ry() += 1;
QTest::mouseMove(view.viewport(), mousePos);
QApplication::processEvents(); // queued connection used to trigger recalculateContentSize
const bool scrollbarAvailable = scrollBar->maximum() > scrollBar->minimum();
bool allowMoreEvents = false;
if (scrollbarAvailable) {
if (!overshoot) {
QTRY_VERIFY(scrollBar->isVisible());
// scrollbar becoming visible triggers event replay, so we get more than one
allowMoreEvents = true;
}
++overshoot;
}
if (allowMoreEvents)
QCOMPARE_GE(item.scenePositions.count(), 1);
else
QCOMPARE(item.scenePositions.count(), 1);
const auto scenePos = item.scenePositions.takeLast();
item.scenePositions.clear();
const auto same = orientation == Qt::Horizontal ? &QPointF::y : &QPointF::x;
const auto moved = orientation == Qt::Horizontal ? &QPointF::x : &QPointF::y;
QCOMPARE((item.pos().*same)(), (lastItemPos.*same)());
QCOMPARE_GE((item.pos().*moved)() - (lastItemPos.*moved)(), 1);
QCOMPARE_LE((item.pos().*moved)() - (lastItemPos.*moved)(), 2);
lastItemPos = item.pos();
QCOMPARE((scenePos.*same)(), (lastScenePos.*same)());
QCOMPARE_GE((scenePos.*moved)() - (lastScenePos.*moved)(), 1);
QCOMPARE_LE((scenePos.*moved)() - (lastScenePos.*moved)(), 2);
lastScenePos = scenePos;
}
}
QTEST_MAIN(tst_QGraphicsView)
#include "tst_qgraphicsview.moc"