From 5423ba187c62ea861ccfcc013fb15fcc4a5ae28d Mon Sep 17 00:00:00 2001 From: Andreas Aardal Hanssen Date: Thu, 23 Apr 2009 17:46:54 +0200 Subject: Fix bugs in QGraphicsItem::childrenBoundingRect() While working on layering in Graphics View I stumbled over this bug. The QGraphicsItem::childrenBoundingRect() function had an accumulating error caused by recursive adding of rectangles that individually were mapped to the local parent using QGraphicsItem::mapRectToParent() / QTransform::mapRect. This caused the brect to be way too large for items with children that are rotated (fex, alternating 45 and -45 degrees). The new version should be just as fast, but with no loss of precision. Reviewed-by: bnilsen --- src/gui/graphicsview/qgraphicsitem.cpp | 41 +++++++++++++++++---- src/gui/graphicsview/qgraphicsitem_p.h | 1 + tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 49 ++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index 8a7a080..65d1e0a 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -885,6 +885,38 @@ void QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem *newParent, bool de /*! \internal + Returns the bounding rect of this item's children (excluding itself). +*/ +void QGraphicsItemPrivate::childrenBoundingRectHelper(QTransform *x, QRectF *rect) +{ + for (int i = 0; i < children.size(); ++i) { + QGraphicsItem *child = children.at(i); + QGraphicsItemPrivate *childd = child->d_ptr; + bool hasX = childd->hasTransform; + bool hasPos = !childd->pos.isNull(); + if (hasPos || hasX) { + QTransform matrix; + if (hasX) + matrix = child->transform(); + if (hasPos) { + const QPointF &p = childd->pos; + matrix *= QTransform::fromTranslate(p.x(), p.y()); + } + matrix *= *x; + *rect |= matrix.mapRect(child->boundingRect()); + if (!childd->children.isEmpty()) + childd->childrenBoundingRectHelper(&matrix, rect); + } else { + *rect |= x->mapRect(child->boundingRect()); + if (!childd->children.isEmpty()) + childd->childrenBoundingRectHelper(x, rect); + } + } +} + +/*! + \internal + Empty all cached pixmaps from the pixmap cache. */ void QGraphicsItemCache::purge() @@ -3019,13 +3051,8 @@ void QGraphicsItem::setZValue(qreal z) QRectF QGraphicsItem::childrenBoundingRect() const { QRectF childRect; - foreach (QGraphicsItem *child, children()) { - QPointF childPos = child->pos(); - QTransform matrix = child->transform(); - if (!childPos.isNull()) - matrix *= QTransform::fromTranslate(childPos.x(), childPos.y()); - childRect |= matrix.mapRect(child->boundingRect() | child->childrenBoundingRect()); - } + QTransform x; + d_ptr->childrenBoundingRectHelper(&x, &childRect); return childRect; } diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h index 078c543..940e566 100644 --- a/src/gui/graphicsview/qgraphicsitem_p.h +++ b/src/gui/graphicsview/qgraphicsitem_p.h @@ -178,6 +178,7 @@ public: void addChild(QGraphicsItem *child); void removeChild(QGraphicsItem *child); void setParentItemHelper(QGraphicsItem *parent, bool deleting); + void childrenBoundingRectHelper(QTransform *x, QRectF *rect); virtual void resolveFont(uint inheritedMask) { diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp index 57e441b..88c64d3 100644 --- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp @@ -172,6 +172,7 @@ private slots: void boundingRects2(); void sceneBoundingRect(); void childrenBoundingRect(); + void childrenBoundingRectTransformed(); void group(); void setGroup(); void nestedGroups(); @@ -2917,9 +2918,57 @@ void tst_QGraphicsItem::childrenBoundingRect() childChild->setParentItem(child); childChild->setPos(500, 500); child->rotate(90); + + + scene.addPolygon(parent->mapToScene(parent->boundingRect() | parent->childrenBoundingRect()))->setPen(QPen(Qt::red));; + + QGraphicsView view(&scene); + view.show(); + + QTest::qWait(5000); + QCOMPARE(parent->childrenBoundingRect(), QRectF(-500, -100, 600, 800)); } +void tst_QGraphicsItem::childrenBoundingRectTransformed() +{ + QGraphicsScene scene; + + QGraphicsRectItem *rect = scene.addRect(QRectF(0, 0, 100, 100)); + QGraphicsRectItem *rect2 = scene.addRect(QRectF(0, 0, 100, 100)); + QGraphicsRectItem *rect3 = scene.addRect(QRectF(0, 0, 100, 100)); + QGraphicsRectItem *rect4 = scene.addRect(QRectF(0, 0, 100, 100)); + QGraphicsRectItem *rect5 = scene.addRect(QRectF(0, 0, 100, 100)); + rect2->setParentItem(rect); + rect3->setParentItem(rect2); + rect4->setParentItem(rect3); + rect5->setParentItem(rect4); + + rect2->setTransform(QTransform().translate(50, 50).rotate(45)); + rect2->setPos(25, 25); + rect3->setTransform(QTransform().translate(50, 50).rotate(45)); + rect3->setPos(25, 25); + rect4->setTransform(QTransform().translate(50, 50).rotate(45)); + rect4->setPos(25, 25); + rect5->setTransform(QTransform().translate(50, 50).rotate(45)); + rect5->setPos(25, 25); + + QRectF subTreeRect = rect->childrenBoundingRect(); + QCOMPARE(subTreeRect.left(), qreal(-206.0660171779821)); + QCOMPARE(subTreeRect.top(), qreal(75.0)); + QCOMPARE(subTreeRect.width(), qreal(351.7766952966369)); + QCOMPARE(subTreeRect.height(), qreal(251.7766952966369)); + + rect->rotate(45); + rect2->rotate(-45); + rect3->rotate(45); + rect4->rotate(-45); + rect5->rotate(45); + + subTreeRect = rect->childrenBoundingRect(); + QCOMPARE(rect->childrenBoundingRect(), QRectF(-100, 75, 275, 250)); +} + void tst_QGraphicsItem::group() { QGraphicsScene scene; -- cgit v0.12