From 1c114fe03a6af18060529fe0604acf89dc595bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Nilsen?= Date: Mon, 3 Aug 2009 12:56:55 +0200 Subject: Artifacts when moving a child when the parent has a graphics effect. We have to use the effectiveBoundingRect() when finding out which items to repaint within a specific area. However, we don't want items to be clickable on the shadow, so we shouldn't use effectiveBoundingRect for normal item-lookup. Solution to this is to only use effectiveBoundingRect() in the BSP (used by estimateTopLevels) and in drawSubtreeRecursive. Auto-test included. --- src/gui/graphicsview/qgraphicsitem.cpp | 27 +++++-- .../graphicsview/qgraphicsscenebsptreeindex.cpp | 4 +- tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 90 ++++++++++++++++++++++ 3 files changed, 113 insertions(+), 8 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index b1c19d8..78a5410 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -2227,8 +2227,11 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect) // Unset current effect. QGraphicsEffectPrivate *oldEffectPrivate = d_ptr->graphicsEffect->d_func(); d_ptr->graphicsEffect = 0; - if (oldEffectPrivate) + if (oldEffectPrivate) { oldEffectPrivate->setGraphicsEffectSource(0); // deletes the current source. + if (d_ptr->scene) // Update the views directly. + d_ptr->scene->d_func()->markDirty(this, QRectF(), false, false, false, false, true); + } } else { // Set new effect. QGraphicsEffectSourcePrivate *sourced = new QGraphicsItemEffectSourcePrivate(this); @@ -2237,8 +2240,7 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect) effect->d_func()->setGraphicsEffectSource(source); } - if (d_ptr->scene) - d_ptr->scene->d_func()->markDirty(this, QRectF(), false, false, false, false, !effect); + prepareGeometryChange(); } /*! @@ -2252,9 +2254,22 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect) */ QRectF QGraphicsItem::effectiveBoundingRect() const { - if (d_ptr->graphicsEffect && d_ptr->graphicsEffect->isEnabled()) - return d_ptr->graphicsEffect->boundingRect(); - return boundingRect(); + QGraphicsEffect *effect = d_ptr->graphicsEffect; + QRectF brect = effect && effect->isEnabled() ? effect->boundingRect() : boundingRect(); + if (d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren) + return brect; + + const QGraphicsItem *effectParent = d_ptr->parent; + while (effectParent) { + effect = effectParent->d_ptr->graphicsEffect; + if (effect && effect->isEnabled()) + brect = effect->boundingRectFor(brect); + if (effectParent->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren) + return brect; + effectParent = effectParent->d_ptr->parent; + } + + return brect; } /*! diff --git a/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp b/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp index 3cb33d1..78a77aa 100644 --- a/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp +++ b/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp @@ -173,7 +173,7 @@ void QGraphicsSceneBspTreeIndexPrivate::_q_updateIndex() if (item->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren) continue; - bsp.insertItem(item, item->sceneBoundingRect()); + bsp.insertItem(item, item->sceneEffectiveBoundingRect()); } } unindexedItems.clear(); @@ -353,7 +353,7 @@ void QGraphicsSceneBspTreeIndexPrivate::removeItem(QGraphicsItem *item, bool rec purgePending = true; removedItems << item; } else if (!(item->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)) { - bsp.removeItem(item, item->sceneBoundingRect()); + bsp.removeItem(item, item->sceneEffectiveBoundingRect()); } } else { unindexedItems.removeOne(item); diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp index 011e480..ceb033e 100644 --- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp @@ -57,6 +57,7 @@ #include #include #include +#include //TESTED_CLASS= //TESTED_FILES= @@ -274,6 +275,7 @@ private slots: void sorting(); void itemHasNoContents(); void hitTestUntransformableItem(); + void hitTestGraphicsEffectItem(); void focusProxy(); // task specific tests below me @@ -7385,6 +7387,94 @@ void tst_QGraphicsItem::hitTestUntransformableItem() QCOMPARE(items.at(0), static_cast(item3)); } +void tst_QGraphicsItem::hitTestGraphicsEffectItem() +{ + QGraphicsScene scene; + scene.setSceneRect(-100, -100, 200, 200); + + QGraphicsView view(&scene); + view.show(); +#ifdef Q_WS_X11 + qt_x11_wait_for_window_manager(&view); +#endif + QTest::qWait(100); + + // Confuse the BSP with dummy items. + QGraphicsRectItem *dummy = new QGraphicsRectItem(0, 0, 20, 20); + dummy->setPos(-100, -100); + scene.addItem(dummy); + for (int i = 0; i < 100; ++i) { + QGraphicsItem *parent = dummy; + dummy = new QGraphicsRectItem(0, 0, 20, 20); + dummy->setPos(-100 + i, -100 + i); + dummy->setParentItem(parent); + } + + const QRectF itemBoundingRect(0, 0, 20, 20); + EventTester *item1 = new EventTester; + item1->br = itemBoundingRect; + item1->setPos(-200, -200); + + EventTester *item2 = new EventTester; + item2->br = itemBoundingRect; + item2->setFlag(QGraphicsItem::ItemIgnoresTransformations); + item2->setParentItem(item1); + item2->setPos(200, 200); + + EventTester *item3 = new EventTester; + item3->br = itemBoundingRect; + item3->setParentItem(item2); + item3->setPos(80, 80); + + scene.addItem(item1); + QTest::qWait(100); + + item1->repaints = 0; + item2->repaints = 0; + item3->repaints = 0; + + // Apply shadow effect to the entire sub-tree. + QGraphicsShadowEffect *shadow = new QGraphicsShadowEffect; + shadow->setShadowOffset(-20, -20); + item1->setGraphicsEffect(shadow); + QTest::qWait(50); + + // Make sure all items are repainted. + QCOMPARE(item1->repaints, 1); + QCOMPARE(item2->repaints, 1); + QCOMPARE(item3->repaints, 1); + + // Make sure an item doesn't respond to a click on its shadow. + QList items = scene.items(QPointF(75, 75)); + QVERIFY(items.isEmpty()); + items = scene.items(QPointF(80, 80)); + QCOMPARE(items.size(), 1); + QCOMPARE(items.at(0), static_cast(item3)); + + item1->repaints = 0; + item2->repaints = 0; + item3->repaints = 0; + + view.viewport()->update(75, 75, 20, 20); + QTest::qWait(50); + + // item1 is the effect source and must therefore be repainted. + // item2 intersects with the exposed region + // item3 is just another child outside the exposed region + QCOMPARE(item1->repaints, 1); + QCOMPARE(item2->repaints, 1); + QCOMPARE(item3->repaints, 0); + + scene.setItemIndexMethod(QGraphicsScene::NoIndex); + QTest::qWait(100); + + items = scene.items(QPointF(75, 75)); + QVERIFY(items.isEmpty()); + items = scene.items(QPointF(80, 80)); + QCOMPARE(items.size(), 1); + QCOMPARE(items.at(0), static_cast(item3)); +} + void tst_QGraphicsItem::focusProxy() { QGraphicsScene scene; -- cgit v0.12