From a1786a441e9101500ae2c28a5226372ba0c7b4a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Nilsen?= Date: Thu, 11 Jun 2009 16:24:40 +0200 Subject: QGraphicsView: Replace update() with updateAll(). We have some nice cut-offs when there's a full update pending, but we don't know about it if we call update() directly on the viewport. Instead call QGraphicsViewPrivate::updateAll() which has the same effect, except that it also sets a flag telling us a full update is pending. Reviewed-by: Andreas --- src/gui/graphicsview/qgraphicsview.cpp | 34 +++++++++++++--------------------- src/gui/graphicsview/qgraphicsview_p.h | 8 +++++++- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp index de7b9f4..4a32ee5 100644 --- a/src/gui/graphicsview/qgraphicsview.cpp +++ b/src/gui/graphicsview/qgraphicsview.cpp @@ -442,7 +442,7 @@ void QGraphicsViewPrivate::recalculateContentSize() // scroll instead. if (oldLeftIndent != leftIndent || oldTopIndent != topIndent) { dirtyScroll = true; - viewport->update(); + updateAll(); } else if (q->isRightToLeft() && !leftIndent) { // In reverse mode, the horizontal scroll always changes after the content // size has changed, as the scroll is calculated by summing the min and @@ -823,14 +823,6 @@ void QGraphicsViewPrivate::processPendingUpdates() dirtyRegion = QRegion(); } -void QGraphicsViewPrivate::updateAll() -{ - viewport->update(); - fullUpdatePending = true; - dirtyBoundingRect = QRect(); - dirtyRegion = QRegion(); -} - void QGraphicsViewPrivate::updateRegion(const QRegion &r) { if (r.isEmpty() || fullUpdatePending) @@ -1084,7 +1076,7 @@ void QGraphicsView::setRenderHints(QPainter::RenderHints hints) if (hints == d->renderHints) return; d->renderHints = hints; - viewport()->update(); + d->updateAll(); } /*! @@ -1102,7 +1094,7 @@ void QGraphicsView::setRenderHint(QPainter::RenderHint hint, bool enabled) else d->renderHints &= ~hint; if (oldHints != d->renderHints) - viewport()->update(); + d->updateAll(); } /*! @@ -1389,7 +1381,7 @@ void QGraphicsView::resetCachedContent() if (d->cacheMode & CacheBackground) { // Background caching is enabled. d->mustResizeBackgroundPixmap = true; - viewport()->update(); + d->updateAll(); } else if (d->mustResizeBackgroundPixmap) { // Background caching is disabled. // Cleanup, free some resources. @@ -1478,7 +1470,7 @@ void QGraphicsView::setScene(QGraphicsScene *scene) return; // Always update the viewport when the scene changes. - viewport()->update(); + d->updateAll(); // Remove the previously assigned scene. if (d->scene) { @@ -2434,7 +2426,7 @@ void QGraphicsView::setBackgroundBrush(const QBrush &brush) { Q_D(QGraphicsView); d->backgroundBrush = brush; - viewport()->update(); + d->updateAll(); if (d->cacheMode & CacheBackground) { // Invalidate the background pixmap @@ -2464,7 +2456,7 @@ void QGraphicsView::setForegroundBrush(const QBrush &brush) { Q_D(QGraphicsView); d->foregroundBrush = brush; - viewport()->update(); + d->updateAll(); } /*! @@ -3062,7 +3054,7 @@ void QGraphicsView::mouseMoveEvent(QMouseEvent *event) if (d->viewportUpdateMode != FullViewportUpdate) viewport()->update(d->rubberBandRegion(viewport(), d->rubberBandRect)); else - viewport()->update(); + d->updateAll(); } // Stop rubber banding if the user has let go of all buttons (even @@ -3084,7 +3076,7 @@ void QGraphicsView::mouseMoveEvent(QMouseEvent *event) if (d->viewportUpdateMode != FullViewportUpdate) viewport()->update(d->rubberBandRegion(viewport(), d->rubberBandRect)); else - viewport()->update(); + d->updateAll(); } // Set the new selection area QPainterPath selectionArea; @@ -3127,7 +3119,7 @@ void QGraphicsView::mouseReleaseEvent(QMouseEvent *event) if (d->viewportUpdateMode != FullViewportUpdate) viewport()->update(d->rubberBandRegion(viewport(), d->rubberBandRect)); else - viewport()->update(); + d->updateAll(); } d->rubberBanding = false; d->rubberBandRect = QRect(); @@ -3395,10 +3387,10 @@ void QGraphicsView::scrollContentsBy(int dx, int dy) #endif viewport()->scroll(dx, dy); } else { - viewport()->update(); + d->updateAll(); } } else { - viewport()->update(); + d->updateAll(); } } @@ -3611,7 +3603,7 @@ void QGraphicsView::setTransform(const QTransform &matrix, bool combine ) d->transforming = false; // Any matrix operation requires a full update. - viewport()->update(); + d->updateAll(); } /*! diff --git a/src/gui/graphicsview/qgraphicsview_p.h b/src/gui/graphicsview/qgraphicsview_p.h index 814e476..00f3035 100644 --- a/src/gui/graphicsview/qgraphicsview_p.h +++ b/src/gui/graphicsview/qgraphicsview_p.h @@ -163,7 +163,13 @@ public: QRegion dirtyRegion; QRect dirtyBoundingRect; void processPendingUpdates(); - void updateAll(); + inline void updateAll() + { + viewport->update(); + fullUpdatePending = true; + dirtyBoundingRect = QRect(); + dirtyRegion = QRegion(); + } void updateRect(const QRect &rect); void updateRegion(const QRegion ®ion); bool updateSceneSlotReimplementedChecked; -- cgit v0.12 From 5918d108579d53e9c41ee674379a8c60124e9838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Nilsen?= Date: Thu, 11 Jun 2009 17:29:46 +0200 Subject: Painting artifacts in QGraphicsView. Problem appears in the chip demo when clicking an item while scrolling the view using the mouse wheel. The problem was that we didn't translate the the item's old painted view rect. There was also a problem when enabling the DontAdjustForAntialiasing flag, causing an item to not redraw its edges. We have to adjust the rectangle by (-1, -1, 1, 1) since QRect() and QRectF() behaves differently. Auto-test (made by Andreas) included. Reviewed-by: Andreas --- src/gui/graphicsview/qgraphicsscene.cpp | 8 +++- src/gui/graphicsview/qgraphicsview.cpp | 21 +++++---- src/gui/graphicsview/qgraphicsview_p.h | 1 + tests/auto/qgraphicsview/tst_qgraphicsview.cpp | 63 ++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp index 1c48675..55d8a1d 100644 --- a/src/gui/graphicsview/qgraphicsscene.cpp +++ b/src/gui/graphicsview/qgraphicsscene.cpp @@ -5269,7 +5269,9 @@ void QGraphicsScenePrivate::markDirty(QGraphicsItem *item, const QRectF &rect, b for (int i = 0; i < views.size(); ++i) { QGraphicsViewPrivate *viewPrivate = views.at(i)->d_func(); - viewPrivate->updateRect(item->d_ptr->paintedViewBoundingRects.value(viewPrivate->viewport)); + QRect rect = item->d_ptr->paintedViewBoundingRects.value(viewPrivate->viewport); + rect.translate(viewPrivate->dirtyScrollOffset); + viewPrivate->updateRect(rect); } return; } @@ -5350,7 +5352,9 @@ void QGraphicsScenePrivate::processDirtyItemsRecursive(QGraphicsItem *item, bool if (item->d_ptr->paintedViewBoundingRectsNeedRepaint) { wasDirtyParentViewBoundingRects = true; - viewPrivate->updateRect(item->d_ptr->paintedViewBoundingRects.value(viewPrivate->viewport)); + QRect rect = item->d_ptr->paintedViewBoundingRects.value(viewPrivate->viewport); + rect.translate(viewPrivate->dirtyScrollOffset); + viewPrivate->updateRect(rect); } if (!item->d_ptr->dirty) diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp index 4a32ee5..a72aa5a 100644 --- a/src/gui/graphicsview/qgraphicsview.cpp +++ b/src/gui/graphicsview/qgraphicsview.cpp @@ -812,7 +812,7 @@ void QGraphicsViewPrivate::processPendingUpdates() if (viewportUpdateMode == QGraphicsView::BoundingRectViewportUpdate) { if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing) - viewport->update(dirtyBoundingRect); + viewport->update(dirtyBoundingRect.adjusted(-1, -1, 1, 1)); else viewport->update(dirtyBoundingRect.adjusted(-2, -2, 2, 2)); } else { @@ -843,14 +843,16 @@ void QGraphicsViewPrivate::updateRegion(const QRegion &r) break; case QGraphicsView::SmartViewportUpdate: // ### DEPRECATE case QGraphicsView::MinimalViewportUpdate: - if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing) { - dirtyRegion += r; - } else { - const QVector &rects = r.rects(); - for (int i = 0; i < rects.size(); ++i) + { + const QVector &rects = r.rects(); + for (int i = 0; i < rects.size(); ++i) { + if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing) + dirtyRegion += rects.at(i).adjusted(-1, -1, 1, 1); + else dirtyRegion += rects.at(i).adjusted(-2, -2, 2, 2); } break; + } case QGraphicsView::NoViewportUpdate: // Unreachable break; @@ -878,7 +880,7 @@ void QGraphicsViewPrivate::updateRect(const QRect &r) case QGraphicsView::SmartViewportUpdate: // ### DEPRECATE case QGraphicsView::MinimalViewportUpdate: if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing) - dirtyRegion += r; + dirtyRegion += r.adjusted(-1, -1, 1, 1); else dirtyRegion += r.adjusted(-2, -2, 2, 2); break; @@ -2678,6 +2680,7 @@ bool QGraphicsView::viewportEvent(QEvent *event) case QEvent::Paint: // Reset full update d->fullUpdatePending = false; + d->dirtyScrollOffset = QPoint(); if (d->scene) { // Check if this view reimplements the updateScene slot; if it // does, we can't do direct update delivery and have to fall back @@ -3375,7 +3378,6 @@ void QGraphicsView::scrollContentsBy(int dx, int dy) if (d->viewportUpdateMode != QGraphicsView::NoViewportUpdate) { if (d->viewportUpdateMode != QGraphicsView::FullViewportUpdate) { - d->dirtyRegion.translate(dx, dy); if (d->accelerateScrolling) { #ifndef QT_NO_RUBBERBAND // Update new and old rubberband regions @@ -3385,6 +3387,9 @@ void QGraphicsView::scrollContentsBy(int dx, int dy) viewport()->update(rubberBandRegion); } #endif + d->dirtyScrollOffset.rx() += dx; + d->dirtyScrollOffset.ry() += dy; + d->dirtyRegion.translate(dx, dy); viewport()->scroll(dx, dy); } else { d->updateAll(); diff --git a/src/gui/graphicsview/qgraphicsview_p.h b/src/gui/graphicsview/qgraphicsview_p.h index 00f3035..a6f0d04 100644 --- a/src/gui/graphicsview/qgraphicsview_p.h +++ b/src/gui/graphicsview/qgraphicsview_p.h @@ -94,6 +94,7 @@ public: QPoint mousePressScreenPoint; QPointF lastMouseMoveScenePoint; QPoint lastMouseMoveScreenPoint; + QPoint dirtyScrollOffset; Qt::MouseButton mousePressButton; QTransform matrix; bool identityMatrix; diff --git a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp index 5167682..06b4a78 100644 --- a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp +++ b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp @@ -188,6 +188,8 @@ private slots: void embeddedViews(); void scrollAfterResize_data(); void scrollAfterResize(); + void moveItemWhileScrolling_data(); + void moveItemWhileScrolling(); void centerOnDirtyItem(); void mouseTracking(); void mouseTracking2(); @@ -3045,6 +3047,67 @@ void tst_QGraphicsView::scrollAfterResize() QCOMPARE(view.viewportTransform(), x3); } +void tst_QGraphicsView::moveItemWhileScrolling_data() +{ + QTest::addColumn("adjustForAntialiasing"); + + QTest::newRow("no adjust") << false; + QTest::newRow("adjust") << true; +} + +void tst_QGraphicsView::moveItemWhileScrolling() +{ + QFETCH(bool, adjustForAntialiasing); + + class MoveItemScrollView : public QGraphicsView + { + public: + MoveItemScrollView() + { + setScene(new QGraphicsScene(0, 0, 1000, 1000)); + rect = scene()->addRect(0, 0, 10, 10); + rect->setPos(50, 50); + } + QRegion lastPaintedRegion; + QGraphicsItem *rect; + protected: + void paintEvent(QPaintEvent *event) + { + lastPaintedRegion = event->region(); + QGraphicsView::paintEvent(event); + } + }; + + MoveItemScrollView view; + view.setFrameStyle(0); + view.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + view.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + view.setResizeAnchor(QGraphicsView::NoAnchor); + view.setTransformationAnchor(QGraphicsView::NoAnchor); + if (!adjustForAntialiasing) + view.setOptimizationFlag(QGraphicsView::DontAdjustForAntialiasing); + view.show(); + view.resize(200, 200); +#ifdef Q_WS_X11 + qt_x11_wait_for_window_manager(&view); +#endif + QTest::qWait(100); + + view.lastPaintedRegion = QRegion(); + view.horizontalScrollBar()->setValue(view.horizontalScrollBar()->value() + 10); + view.rect->moveBy(0, 10); + QTest::qWait(100); + + QRegion expectedRegion; + expectedRegion += QRect(0, 0, 200, 200); + expectedRegion -= QRect(0, 0, 190, 200); + int a = adjustForAntialiasing ? 2 : 1; + expectedRegion += QRect(40, 50, 10, 10).adjusted(-a, -a, a, a); + expectedRegion += QRect(40, 60, 10, 10).adjusted(-a, -a, a, a); + + QCOMPARE(view.lastPaintedRegion, expectedRegion); +} + void tst_QGraphicsView::centerOnDirtyItem() { QGraphicsView view; -- cgit v0.12