From cc18633fe45d599bfeac2a8b2737d155f1dd5564 Mon Sep 17 00:00:00 2001 From: Andreas Aardal Hanssen Date: Mon, 6 Apr 2009 13:31:02 +0200 Subject: Fixup update rect regression by adjusting expose rectangles. This change shows a limitation in Graphics View caused by QPen's default width being 0 (cosmetic), while Graphics View actually does not support cosmetic pens at all. Because items are at risk of drawing lines that poke 1 pixel outside their bounding rect, QGraphicsView must look for items that are up to one pixel larger than their bounding rect mapped to viewport coordinates. Furthermore, mapToScene(QRect) forces us to adjust the input rectangle by (0, 0, 1, 1), because it uses QRect::bottomRight() (etc) when mapping the rectangle to a polygon (which is _wrong_). Since this behavior has been there since 4.2, we don't want to fix it in a 4.5 patch release... The only _proper_ fix to this problem is for the view to know the item's "adjust" in device coordinates, allowing items to use cosmetic pens freely. Fex, we could introduce QGraphicsItem::viewportMargins() or so. Added an autotest to ensure this doesn't break again. Reviewed-by: bnilsen --- src/gui/graphicsview/qgraphicsview.cpp | 42 ++++++++++++++---- tests/auto/qgraphicsview/tst_qgraphicsview.cpp | 61 ++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 8 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp index c4297df..418638f 100644 --- a/src/gui/graphicsview/qgraphicsview.cpp +++ b/src/gui/graphicsview/qgraphicsview.cpp @@ -1046,13 +1046,24 @@ void QGraphicsViewPrivate::freeStyleOptionsArray(QStyleOptionGraphicsItem *array extern QPainterPath qt_regionToPath(const QRegion ®ion); +/*! + ### Adjustments in findItems: mapToScene(QRect) forces us to adjust the + input rectangle by (0, 0, 1, 1), because it uses QRect::bottomRight() + (etc) when mapping the rectangle to a polygon (which is _wrong_). In + addition, as QGraphicsItem::boundingRect() is defined in logical space, + but the default pen for QPainter is cosmetic with a width of 0, QPainter + is at risk of painting 1 pixel outside the bounding rect. Therefore we + must search for items with an adjustment of (-1, -1, 1, 1). +*/ QList QGraphicsViewPrivate::findItems(const QRegion &exposedRegion, bool *allItems) const { Q_Q(const QGraphicsView); - const QPainterPath exposedPath(qt_regionToPath(exposedRegion)); - const QPainterPath exposedScenePath(q->mapToScene(exposedPath)); - if (exposedScenePath.contains(scene->d_func()->growingItemsBoundingRect)) { + // Step 1) If all items are contained within the expose region, then + // return a list of all visible items. + const QRectF exposedRegionSceneBounds = q->mapToScene(exposedRegion.boundingRect().adjusted(-1, -1, 2, 2)) + .boundingRect(); + if (exposedRegionSceneBounds.contains(scene->d_func()->growingItemsBoundingRect)) { Q_ASSERT(allItems); *allItems = true; @@ -1072,12 +1083,27 @@ QList QGraphicsViewPrivate::findItems(const QRegion &exposedReg return itemList; } + // Step 2) If the expose region is a simple rect and the view is only + // translated or scaled, search for items using + // QGraphicsScene::items(QRectF). + bool simpleRectLookup = (scene->d_func()->largestUntransformableItem.isNull() + && exposedRegion.numRects() == 1 && matrix.type() <= QTransform::TxScale); + if (simpleRectLookup) { + return scene->d_func()->items_helper(exposedRegionSceneBounds, + Qt::IntersectsItemBoundingRect, + Qt::DescendingOrder); + } + + // If the region is complex or the view has a complex transform, adjust + // the expose region, convert it to a path, and then search for items + // using QGraphicsScene::items(QPainterPath); + QRegion adjustedRegion; + foreach (const QRect &r, exposedRegion.rects()) + adjustedRegion += r.adjusted(-1, -1, 1, 1); + + const QPainterPath exposedPath(qt_regionToPath(adjustedRegion)); if (scene->d_func()->largestUntransformableItem.isNull()) { - if (exposedRegion.numRects() == 1 && matrix.type() <= QTransform::TxScale) { - return scene->d_func()->items_helper(exposedScenePath.controlPointRect(), - Qt::IntersectsItemBoundingRect, - Qt::DescendingOrder); - } + const QPainterPath exposedScenePath(q->mapToScene(exposedPath)); return scene->d_func()->items_helper(exposedScenePath, Qt::IntersectsItemBoundingRect, Qt::DescendingOrder); diff --git a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp index 4368e76..412c6c5 100644 --- a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp +++ b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp @@ -155,6 +155,8 @@ private slots: void fitInView(); void itemsAtPoint(); void itemsInRect(); + void itemsInRect_cosmeticAdjust_data(); + void itemsInRect_cosmeticAdjust(); void itemsInPoly(); void itemsInPath(); void itemAt(); @@ -1310,6 +1312,65 @@ void tst_QGraphicsView::itemsInRect() QCOMPARE(items.takeFirst()->zValue(), qreal(3)); } +class CountPaintItem : public QGraphicsRectItem +{ +public: + int numPaints; + + CountPaintItem(const QRectF &rect) + : QGraphicsRectItem(rect), numPaints(0) + { } + + void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget = 0) + { + ++numPaints; + QGraphicsRectItem::paint(painter, option, widget); + } +}; + +void tst_QGraphicsView::itemsInRect_cosmeticAdjust_data() +{ + QTest::addColumn("updateRect"); + QTest::addColumn("numPaints"); + + QTest::newRow("nil") << QRect() << 1; + QTest::newRow("0, 0, 300, 100") << QRect(0, 0, 300, 100) << 1; + QTest::newRow("0, 0, 100, 300") << QRect(0, 0, 100, 300) << 1; + QTest::newRow("200, 0, 100, 300") << QRect(200, 0, 100, 300) << 1; + QTest::newRow("0, 200, 300, 100") << QRect(0, 200, 300, 100) << 1; + QTest::newRow("0, 0, 300, 99") << QRect(0, 0, 300, 99) << 0; + QTest::newRow("0, 0, 99, 300") << QRect(0, 0, 99, 300) << 0; + QTest::newRow("201, 0, 99, 300") << QRect(201, 0, 99, 300) << 0; + QTest::newRow("0, 201, 300, 99") << QRect(0, 201, 300, 99) << 0; +} + +void tst_QGraphicsView::itemsInRect_cosmeticAdjust() +{ + QFETCH(QRect, updateRect); + QFETCH(int, numPaints); + + QGraphicsScene scene(-100, -100, 200, 200); + CountPaintItem *rect = new CountPaintItem(QRectF(-50, -50, 100, 100)); + scene.addItem(rect); + + QGraphicsView view(&scene); + view.setFrameStyle(0); + view.resize(300, 300); + view.show(); +#ifdef Q_WS_X11 + qt_x11_wait_for_window_manager(&view); +#endif + QTest::qWait(125); + + rect->numPaints = 0; + if (updateRect.isNull()) + view.viewport()->update(); + else + view.viewport()->update(updateRect); + qApp->processEvents(); + QCOMPARE(rect->numPaints, numPaints); +} + void tst_QGraphicsView::itemsInPoly() { QGraphicsScene scene; -- cgit v0.12