Fix QGraphicsScene::drawItems() to use the recursive path as well.

This change also changes the direct painting path to be opt-in as a
temporary testing measure to see what's broken when using the old code
path.

5 files changed, 34 insertions, 157 deletions

diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 1d33361..30c7f97 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -5048,6 +5048,7 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
                                                  const QTransform &viewTransform,
                                                  const QRegion &exposedRegion, QWidget *widget,
                                                  QGraphicsView::OptimizationFlags optimizationFlags,
+                                                 QList<QGraphicsItem *> *topLevelItems,
                                                  qreal parentOpacity)
 {
     // Calculate opacity.
@@ -5111,7 +5112,7 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
     }
 
     // Find and sort children.
-    QList<QGraphicsItem *> &children = item ? item->d_ptr->children : topLevelItems;
+    QList<QGraphicsItem *> &children = item ? item->d_ptr->children : (topLevelItems ? *topLevelItems : this->topLevelItems);
     if (!dontDrawChildren) {
         if (item && item->d_ptr->needSortChildren) {
             item->d_ptr->needSortChildren = 0;
@@ -5123,7 +5124,7 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
     }
 
     // Draw children behind
-    int i;
+    int i = 0;
     if (!dontDrawChildren) {
         for (i = 0; i < children.size(); ++i) {
             QGraphicsItem *child = children.at(i);
@@ -5131,7 +5132,7 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
                 break;
             drawSubtreeRecursive(child, painter, transform, viewTransform,
                                  exposedRegion, widget, optimizationFlags,
-                                 opacity);
+                                 0, opacity);
         }
     }
 
@@ -5160,7 +5161,7 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
     if (!dontDrawChildren) {
         for (; i < children.size(); ++i) {
             drawSubtreeRecursive(children.at(i), painter, transform, viewTransform,
-                                 exposedRegion, widget, optimizationFlags, opacity);
+                                 exposedRegion, widget, optimizationFlags, 0, opacity);
         }
     }
 
@@ -5321,118 +5322,27 @@ void QGraphicsScene::drawItems(QPainter *painter,
                                const QStyleOptionGraphicsItem options[], QWidget *widget)
 {
     Q_D(QGraphicsScene);
-
-    // Detect if painter state protection is disabled.
     QTransform viewTransform = painter->worldTransform();
-    QVarLengthArray<QGraphicsItem *, 16> childClippers;
+    Q_UNUSED(options);
 
+    // Draw each toplevel recursively.
+    QGraphicsView *view = widget ? qobject_cast<QGraphicsView *>(widget->parentWidget()) : 0;
+    QList<QGraphicsItem *> topLevelItems;
     for (int i = 0; i < numItems; ++i) {
-        QGraphicsItem *item = items[i];
-        if (!(item->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)) {
-            if (!childClippers.isEmpty()) {
-                // Item is not clipped to any ancestor: pop all current clippers.
-                for (int i = 0; i < childClippers.size(); ++i)
-                    painter->restore();
-                childClippers.clear();
-            }
-        } else {
-            // Item is clipped to an ancestor, which may or may not be in our
-            // child clipper list. Let's start by finding the item's closest
-            // clipping ancestor.
-            QGraphicsItem *clipParent = item->parentItem();
-            while (clipParent && !(clipParent->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape))
-                clipParent = clipParent->parentItem();
-
-            // Pop any in-between clippers. If the clipper is unknown, pop
-            // them all.  ### QVarLengthArray::lastIndexOf().
-            int index = -1;
-            for (int n = childClippers.size() - 1; n >= 0; --n) {
-                if (childClippers[n] == clipParent) {
-                    index = n;
-                    break;
-                }
-            }
-            if (index != -1) {
-                int toPop = childClippers.size() - index - 1;
-                if (toPop > 0) {
-                    for (int i = 0; i < toPop; ++i)
-                        painter->restore();
-                    childClippers.resize(index + 1);
-                }
-            }
-
-            // Sanity check
-            if (!childClippers.isEmpty())
-                Q_ASSERT(childClippers[childClippers.size() - 1] == clipParent);
-
-            // If the clipper list is empty at this point, but we're still
-            // clipped to an ancestor, then we need to build the clip chain
-            // ourselves. There is only one case that can produce this issue:
-            // This item is stacked behind an ancestor:
-            // ItemStacksBehindParent.
-            if (childClippers.isEmpty()) {
-                Q_ASSERT(clipParent != 0);
-                // Build a stack of clippers.
-                QVarLengthArray<QGraphicsItem *, 16> clippers;
-                QGraphicsItem *p = clipParent;
-                do {
-                    if (p->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape)
-                        clippers.append(p);
-                } while ((p = p->parentItem()) && (p->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren));
-
-                // ### This code path can also use the itemTransform
-                // optimization, but it's hit very rarely.
-                for (int i = clippers.size() - 1; i >= 0; --i) {
-                    QGraphicsItem *clipper = clippers[i];
-                    painter->setWorldTransform(clipper->deviceTransform(viewTransform), false);
-
-                    childClippers.append(clipper);
-                    painter->save();
-                    painter->setClipPath(clipper->shape(), Qt::IntersectClip);
-                }
-                Q_ASSERT(childClippers[childClippers.size() - 1] == clipParent);
-            }
-        }
-
-        // Set up the painter transform
-        if (item->d_ptr->hasValidDeviceTransform) {
-            painter->setWorldTransform(item->d_ptr->deviceTransform);
-            item->d_ptr->hasValidDeviceTransform = 0;
-        } else {
-            painter->setWorldTransform(item->deviceTransform(viewTransform), false);
-        }
-
-        // Save painter
-        bool saveState = (d->painterStateProtection || (item->flags() & QGraphicsItem::ItemClipsToShape));
-        if (saveState)
-            painter->save();
-
-        // Set local clip
-        if (item->flags() & QGraphicsItem::ItemClipsToShape)
-            painter->setClipPath(item->shape(), Qt::IntersectClip);
-
-        // Setup opacity
-        painter->setOpacity(item->effectiveOpacity());
-
-        // Draw the item
-        d->drawItemHelper(item, painter, &options[i], widget, d->painterStateProtection);
-        const QRect paintedViewBoundingRect = painter->worldTransform().mapRect(options[i].rect).adjusted(-1, -1, 1, 1);
-        item->d_ptr->paintedViewBoundingRects.insert(widget, paintedViewBoundingRect);
-
-        if (saveState)
-            painter->restore();
-
-        if (item->flags() & QGraphicsItem::ItemClipsChildrenToShape) {
-            // Clip descendents to this item's shape, and keep the painter
-            // saved.
-            childClippers.append(item);
-            painter->save();
-            painter->setClipPath(item->shape(), Qt::IntersectClip);
+        QGraphicsItem *item = items[i]->topLevelItem();
+        topLevelItems << item;
+
+        if (!item->d_ptr->itemDiscovered) {
+            item->d_ptr->itemDiscovered = 1;
+            d->drawSubtreeRecursive(item, painter, viewTransform, viewTransform,
+                                    view->d_func()->exposedRegion, widget,
+                                    view->optimizationFlags());
         }
     }
 
-    for (int i = 0; i < childClippers.size(); ++i)
-        painter->restore();
+    // Reset discovery bits.
+    for (int i = 0; i < topLevelItems.size(); ++i)
+        topLevelItems.at(i)->d_ptr->itemDiscovered = 0;
 
     painter->setWorldTransform(viewTransform);
 }
diff --git a/src/gui/graphicsview/qgraphicsscene_p.h b/src/gui/graphicsview/qgraphicsscene_p.h
index 4facec3..2a036e3 100644
--- a/src/gui/graphicsview/qgraphicsscene_p.h
+++ b/src/gui/graphicsview/qgraphicsscene_p.h
@@ -257,7 +257,7 @@ public:
     void drawSubtreeRecursive(QGraphicsItem *item, QPainter *painter, const QTransform &parentTransform,
                               const QTransform &viewTransform,
                               const QRegion &exposedRegion, QWidget *widget, QGraphicsView::OptimizationFlags optimizationFlags,
-                              qreal parentOpacity = qreal(1.0));
+                              QList<QGraphicsItem *> *topLevelItems = 0, qreal parentOpacity = qreal(1.0));
     void markDirty(QGraphicsItem *item, const QRectF &rect = QRectF(), bool invalidateChildren = false,
                    bool maybeDirtyClipPath = false, bool force = false, bool ignoreOpacity = false);
     void processDirtyItemsRecursive(QGraphicsItem *item, const QTransform &);
diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp
index 2515069..8bafe50 100644
--- a/src/gui/graphicsview/qgraphicsview.cpp
+++ b/src/gui/graphicsview/qgraphicsview.cpp
@@ -39,8 +39,6 @@
 **
 ****************************************************************************/
 
-//#define QGRAPHICSVIEW_DEBUG
-
 static const int QGRAPHICSVIEW_REGION_RECT_THRESHOLD = 50;
 
 static const int QGRAPHICSVIEW_PREALLOC_STYLE_OPTIONS = 503; // largest prime < 2^9
@@ -3236,12 +3234,12 @@ void QGraphicsView::paintEvent(QPaintEvent *event)
     d->scene->d_func()->painterStateProtection = !(d->optimizationFlags & DontSavePainterState);
 
     // Determine the exposed region
-    QRegion exposedRegion = event->region();
+    d->exposedRegion = event->region();
     if (!d->accelerateScrolling)
-        exposedRegion = viewport()->rect();
+        d->exposedRegion = viewport()->rect();
     else if (d->viewportUpdateMode == BoundingRectViewportUpdate)
-        exposedRegion = event->rect();
-    QRectF exposedSceneRect = mapToScene(exposedRegion.boundingRect()).boundingRect();
+        d->exposedRegion = event->rect();
+    QRectF exposedSceneRect = mapToScene(d->exposedRegion.boundingRect()).boundingRect();
 
     // Set up the painter
     QPainter painter(viewport());
@@ -3258,16 +3256,7 @@ void QGraphicsView::paintEvent(QPaintEvent *event)
     const QTransform viewTransform = viewportTransform();
     painter.setTransform(viewTransform, true);
 
-#ifdef QGRAPHICSVIEW_DEBUG
-    QTime stopWatch;
-    stopWatch.start();
-    qDebug() << "QGraphicsView::paintEvent(" << exposedRegion << ')';
-#endif
-
-    // Find all exposed items
-    bool allItems = false;
-    QList<QGraphicsItem *> itemList = d->findItems(exposedRegion, &allItems);
-
+    // Draw background
     if ((d->cacheMode & CacheBackground)
 #ifdef Q_WS_X11
         && X11->use_xrender
@@ -3309,18 +3298,14 @@ void QGraphicsView::paintEvent(QPaintEvent *event)
             painter.restore();
     }
 
-#ifdef QGRAPHICSVIEW_DEBUG
-    int backgroundTime = stopWatch.elapsed() - exposedTime;
-#endif
-
-    static int directEnv = qgetenv("QGRAPHICSVIEW_DIRECT").toInt();
-    if (directEnv || (d->optimizationFlags & BypassDrawItems)) {
-        d->scene->d_func()->drawSubtreeRecursive(0, &painter, viewTransform, viewTransform, exposedRegion,
-                                                 viewport(), d->optimizationFlags);
+    // Items
+    if ((d->optimizationFlags & IndirectPainting)) {
+        d->scene->d_func()->drawSubtreeRecursive(0, &painter, viewTransform, viewTransform, d->exposedRegion,
+                                                 viewport(), d->optimizationFlags, 0);
     } else {
         // Find all exposed items
         bool allItems = false;
-        QList<QGraphicsItem *> itemList = d->findItems(exposedRegion, &allItems);
+        QList<QGraphicsItem *> itemList = d->findItems(d->exposedRegion, &allItems);
         
         if (!itemList.isEmpty()) {
             // Generate the style options.
@@ -3328,24 +3313,16 @@ void QGraphicsView::paintEvent(QPaintEvent *event)
             QGraphicsItem **itemArray = &itemList[0]; // Relies on QList internals, but is perfectly valid.
             QStyleOptionGraphicsItem *styleOptionArray = d->allocStyleOptionsArray(numItems);
             for (int i = 0; i < numItems; ++i)
-                itemArray[i]->d_ptr->initStyleOption(&styleOptionArray[i], viewTransform, exposedRegion, allItems);
+                itemArray[i]->d_ptr->initStyleOption(&styleOptionArray[i], viewTransform, d->exposedRegion, allItems);
             // Draw the items.
             drawItems(&painter, numItems, itemArray, styleOptionArray);
             d->freeStyleOptionsArray(styleOptionArray);
         }
     }
 
-#ifdef QGRAPHICSVIEW_DEBUG
-    int itemsTime = stopWatch.elapsed() - exposedTime - backgroundTime;
-#endif
-
     // Foreground
     drawForeground(&painter, exposedSceneRect);
 
-#ifdef QGRAPHICSVIEW_DEBUG
-    int foregroundTime = stopWatch.elapsed() - exposedTime - backgroundTime - itemsTime;
-#endif
-
 #ifndef QT_NO_RUBBERBAND
     // Rubberband
     if (d->rubberBanding && !d->rubberBandRect.isEmpty()) {
@@ -3367,17 +3344,6 @@ void QGraphicsView::paintEvent(QPaintEvent *event)
 
     painter.end();
 
-#ifdef QGRAPHICSVIEW_DEBUG
-    qDebug() << "\tItem discovery....... " << exposedTime << "msecs (" << itemList.size() << "items,"
-             << (exposedTime > 0 ? (itemList.size() * 1000.0 / exposedTime) : -1) << "/ sec )";
-    qDebug() << "\tDrawing background... " << backgroundTime << "msecs (" << exposedRegion.numRects() << "segments )";
-    qDebug() << "\tDrawing items........ " << itemsTime << "msecs ("
-             << (itemsTime > 0 ? (itemList.size() * 1000.0 / itemsTime) : -1) << "/ sec )";
-    qDebug() << "\tDrawing foreground... " << foregroundTime << "msecs (" << exposedRegion.numRects() << "segments )";
-    qDebug() << "\tTotal rendering time: " << stopWatch.elapsed() << "msecs ("
-             << (stopWatch.elapsed() > 0 ? (1000.0 / stopWatch.elapsed()) : -1.0) << "fps )";
-#endif
-
     // Restore painter state protection.
     d->scene->d_func()->painterStateProtection = true;
 }
diff --git a/src/gui/graphicsview/qgraphicsview.h b/src/gui/graphicsview/qgraphicsview.h
index deed1d0..b7a9906 100644
--- a/src/gui/graphicsview/qgraphicsview.h
+++ b/src/gui/graphicsview/qgraphicsview.h
@@ -113,7 +113,7 @@ public:
         DontClipPainter = 0x1, // obsolete
         DontSavePainterState = 0x2,
         DontAdjustForAntialiasing = 0x4,
-        BypassDrawItems = 0x8
+        IndirectPainting = 0x8
     };
     Q_DECLARE_FLAGS(OptimizationFlags, OptimizationFlag)
 
diff --git a/src/gui/graphicsview/qgraphicsview_p.h b/src/gui/graphicsview/qgraphicsview_p.h
index 637687b..814e476 100644
--- a/src/gui/graphicsview/qgraphicsview_p.h
+++ b/src/gui/graphicsview/qgraphicsview_p.h
@@ -167,6 +167,7 @@ public:
     void updateRect(const QRect &rect);
     void updateRegion(const QRegion &region);
     bool updateSceneSlotReimplementedChecked;
+    QRegion exposedRegion;
 
     QList<QGraphicsItem *> findItems(const QRegion &exposedRegion, bool *allItems) const;
 };