From 4e628397b0ef6f8d5e9294e85507488cebe740f3 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 30 Nov 2015 12:19:31 +0100 Subject: QGraphicsView: replace some Q_FOREACH loops over rvalues with C++11 range-for This needs to be handled a bit carefully, because Qt containers will detach upon being iterated over using range-for. In the cases of this patch, that trivially cannot happen, because all containers are marked as const when being assigned the rvalues previously found on the rhs of the Q_FOREACH. The new code thus does exactly what the old code did: take a const copy, then iterate over it. Separate patches will deal with other situations. Range-for loops are much more efficient than foreach loops. This patch shaves almost 4K of text size off an optimized Linux AMD64 GCC 4.9 build. Change-Id: Ida868b77d078cbfa0516d17e98e6f0a86fcdb7a3 Reviewed-by: Olivier Goffart (Woboq GmbH) --- .../graphicsview/qgraphicsanchorlayout_p.cpp | 13 ++-- src/widgets/graphicsview/qgraphicsitem.cpp | 24 +++++--- src/widgets/graphicsview/qgraphicsproxywidget.cpp | 3 +- src/widgets/graphicsview/qgraphicsscene.cpp | 71 ++++++++++++++-------- src/widgets/graphicsview/qgraphicsview.cpp | 9 ++- src/widgets/graphicsview/qgraphicswidget.cpp | 3 +- 6 files changed, 78 insertions(+), 45 deletions(-) (limited to 'src/widgets/graphicsview') diff --git a/src/widgets/graphicsview/qgraphicsanchorlayout_p.cpp b/src/widgets/graphicsview/qgraphicsanchorlayout_p.cpp index e16b427448..adc77ded47 100644 --- a/src/widgets/graphicsview/qgraphicsanchorlayout_p.cpp +++ b/src/widgets/graphicsview/qgraphicsanchorlayout_p.cpp @@ -2318,9 +2318,9 @@ void QGraphicsAnchorLayoutPrivate::findPaths(Orientation orientation) graphPaths[orientation].insert(root, GraphPath()); - foreach (AnchorVertex *v, graph[orientation].adjacentVertices(root)) { + const auto adjacentVertices = graph[orientation].adjacentVertices(root); + for (AnchorVertex *v : adjacentVertices) queue.enqueue(qMakePair(root, v)); - } while(!queue.isEmpty()) { QPair pair = queue.dequeue(); @@ -2339,10 +2339,9 @@ void QGraphicsAnchorLayoutPrivate::findPaths(Orientation orientation) graphPaths[orientation].insert(pair.second, current); - foreach (AnchorVertex *v, - graph[orientation].adjacentVertices(pair.second)) { + const auto adjacentVertices = graph[orientation].adjacentVertices(pair.second); + for (AnchorVertex *v : adjacentVertices) queue.enqueue(qMakePair(pair.second, v)); - } } // We will walk through every reachable items (non-float) store them in a temporary set. @@ -2703,9 +2702,9 @@ void QGraphicsAnchorLayoutPrivate::calculateVertexPositions( visited.insert(root); // Add initial edges to the queue - foreach (AnchorVertex *v, graph[orientation].adjacentVertices(root)) { + const auto adjacentVertices = graph[orientation].adjacentVertices(root); + for (AnchorVertex *v : adjacentVertices) queue.enqueue(qMakePair(root, v)); - } // Do initial calculation required by "interpolateEdge()" setupEdgesInterpolation(orientation); diff --git a/src/widgets/graphicsview/qgraphicsitem.cpp b/src/widgets/graphicsview/qgraphicsitem.cpp index 3197a5bf6c..31b2778251 100644 --- a/src/widgets/graphicsview/qgraphicsitem.cpp +++ b/src/widgets/graphicsview/qgraphicsitem.cpp @@ -1519,7 +1519,8 @@ QGraphicsItem::~QGraphicsItem() if (d_ptr->isObject && !d_ptr->gestureContext.isEmpty()) { QGraphicsObject *o = static_cast(this); if (QGestureManager *manager = QGestureManager::instance()) { - foreach (Qt::GestureType type, d_ptr->gestureContext.keys()) + const auto types = d_ptr->gestureContext.keys(); // FIXME: iterate over the map directly? + for (Qt::GestureType type : types) manager->cleanupCachedGestures(o, type); } } @@ -2243,11 +2244,13 @@ void QGraphicsItem::setCursor(const QCursor &cursor) d_ptr->hasCursor = 1; if (d_ptr->scene) { d_ptr->scene->d_func()->allItemsUseDefaultCursor = false; - foreach (QGraphicsView *view, d_ptr->scene->views()) { + const auto views = d_ptr->scene->views(); + for (QGraphicsView *view : views) { view->viewport()->setMouseTracking(true); // Note: Some of this logic is duplicated in QGraphicsView's mouse events. if (view->underMouse()) { - foreach (QGraphicsItem *itemUnderCursor, view->items(view->mapFromGlobal(QCursor::pos()))) { + const auto itemsUnderCursor = view->items(view->mapFromGlobal(QCursor::pos())); + for (QGraphicsItem *itemUnderCursor : itemsUnderCursor) { if (itemUnderCursor->hasCursor()) { QMetaObject::invokeMethod(view, "_q_setViewportCursor", Q_ARG(QCursor, itemUnderCursor->cursor())); @@ -2286,7 +2289,8 @@ void QGraphicsItem::unsetCursor() d_ptr->unsetExtra(QGraphicsItemPrivate::ExtraCursor); d_ptr->hasCursor = 0; if (d_ptr->scene) { - foreach (QGraphicsView *view, d_ptr->scene->views()) { + const auto views = d_ptr->scene->views(); + for (QGraphicsView *view : views) { if (view->underMouse() && view->itemAt(view->mapFromGlobal(QCursor::pos())) == this) { QMetaObject::invokeMethod(view, "_q_unsetViewportCursor"); break; @@ -2922,7 +2926,8 @@ QRectF QGraphicsItemPrivate::effectiveBoundingRect(const QRectF &rect) const return effect->boundingRectFor(rect); QRectF sceneRect = q->mapRectToScene(rect); QRectF sceneEffectRect; - foreach (QGraphicsView *view, scene->views()) { + const auto views = scene->views(); + for (QGraphicsView *view : views) { QRectF deviceRect = view->d_func()->mapRectFromScene(sceneRect); QRect deviceEffectRect = effect->boundingRectFor(deviceRect).toAlignedRect(); sceneEffectRect |= view->d_func()->mapRectToScene(deviceEffectRect); @@ -5216,7 +5221,8 @@ bool QGraphicsItem::isObscured(const QRectF &rect) const QRectF br = boundingRect(); QRectF testRect = rect.isNull() ? br : rect; - foreach (QGraphicsItem *item, d->scene->items(mapToScene(br), Qt::IntersectsItemBoundingRect)) { + const auto items = d->scene->items(mapToScene(br), Qt::IntersectsItemBoundingRect); + for (QGraphicsItem *item : items) { if (item == this) break; if (qt_QGraphicsItem_isObscured(this, item, testRect)) @@ -5338,7 +5344,8 @@ QRegion QGraphicsItem::boundingRegion(const QTransform &itemToDeviceTransform) c QTransform unscale = QTransform::fromScale(1 / granularity, 1 / granularity); QRegion r; QBitmap colorMask = QBitmap::fromImage(mask.createMaskFromColor(0)); - foreach (const QRect &rect, QRegion( colorMask ).rects()) { + const auto rects = QRegion(colorMask).rects(); + for (const QRect &rect : rects) { QRect xrect = unscale.mapRect(rect).translated(deviceRect.topLeft() - QPoint(pad, pad)); r += xrect.adjusted(-1, -1, 1, 1) & deviceRect; } @@ -6599,7 +6606,8 @@ bool QGraphicsItem::isUnderMouse() const return false; QPoint cursorPos = QCursor::pos(); - foreach (QGraphicsView *view, d->scene->views()) { + const auto views = d->scene->views(); + for (QGraphicsView *view : views) { if (contains(mapFromScene(view->mapToScene(view->mapFromGlobal(cursorPos))))) return true; } diff --git a/src/widgets/graphicsview/qgraphicsproxywidget.cpp b/src/widgets/graphicsview/qgraphicsproxywidget.cpp index 3ea04efc6b..ea9fc0a0d5 100644 --- a/src/widgets/graphicsview/qgraphicsproxywidget.cpp +++ b/src/widgets/graphicsview/qgraphicsproxywidget.cpp @@ -595,7 +595,8 @@ void QGraphicsProxyWidgetPrivate::setWidget_helper(QWidget *newWidget, bool auto resolvePalette(inheritedPaletteResolveMask); widget->update(); - foreach (QGraphicsItem *child, q->childItems()) { + const auto childItems = q->childItems(); + for (QGraphicsItem *child : childItems) { if (child->d_ptr->isProxyWidget()) { QGraphicsProxyWidget *childProxy = static_cast(child); QWidget * parent = childProxy->widget(); diff --git a/src/widgets/graphicsview/qgraphicsscene.cpp b/src/widgets/graphicsview/qgraphicsscene.cpp index 2dc48a8065..3993138f3f 100644 --- a/src/widgets/graphicsview/qgraphicsscene.cpp +++ b/src/widgets/graphicsview/qgraphicsscene.cpp @@ -756,8 +756,9 @@ void QGraphicsScenePrivate::setActivePanelHelper(QGraphicsItem *item, bool durin q->sendEvent(activePanel, &event); } else if (panel && !duringActivationEvent) { // Deactivate the scene if changing activation to a panel. + const auto items = q->items(); QEvent event(QEvent::WindowDeactivate); - foreach (QGraphicsItem *item, q->items()) { + for (QGraphicsItem *item : items) { if (item->isVisible() && !item->isPanel() && !item->parentItem()) q->sendEvent(item, &event); } @@ -791,9 +792,10 @@ void QGraphicsScenePrivate::setActivePanelHelper(QGraphicsItem *item, bool durin } while (fw != panel); } } else if (q->isActive()) { + const auto items = q->items(); // Activate the scene QEvent event(QEvent::WindowActivate); - foreach (QGraphicsItem *item, q->items()) { + for (QGraphicsItem *item : items) { if (item->isVisible() && !item->isPanel() && !item->parentItem()) q->sendEvent(item, &event); } @@ -1545,7 +1547,8 @@ void QGraphicsScenePrivate::updateFont(const QFont &font) // Resolve the fonts of all top-level widget items, or widget items // whose parent is not a widget. - foreach (QGraphicsItem *item, q->items()) { + const auto items = q->items(); + for (QGraphicsItem *item : items) { if (!item->parentItem()) { // Resolvefont for an item is a noop operation, but // every item can be a widget, or can have a widget @@ -1601,7 +1604,8 @@ void QGraphicsScenePrivate::updatePalette(const QPalette &palette) // Resolve the palettes of all top-level widget items, or widget items // whose parent is not a widget. - foreach (QGraphicsItem *item, q->items()) { + const auto items = q->items(); + for (QGraphicsItem *item : items) { if (!item->parentItem()) { // ResolvePalette for an item is a noop operation, but // every item can be a widget, or can have a widget @@ -1947,7 +1951,8 @@ QRectF QGraphicsScene::itemsBoundingRect() const { // Does not take untransformable items into account. QRectF boundingRect; - foreach (QGraphicsItem *item, items()) + const auto items_ = items(); + for (QGraphicsItem *item : items_) boundingRect |= item->sceneBoundingRect(); return boundingRect; } @@ -2116,7 +2121,8 @@ QList QGraphicsScene::collidingItems(const QGraphicsItem *item, // Does not support ItemIgnoresTransformations. QList tmp; - foreach (QGraphicsItem *itemInVicinity, d->index->estimateItems(item->sceneBoundingRect(), Qt::DescendingOrder)) { + const auto itemsInVicinity = d->index->estimateItems(item->sceneBoundingRect(), Qt::DescendingOrder); + for (QGraphicsItem *itemInVicinity : itemsInVicinity) { if (item != itemInVicinity && item->collidesWithItem(itemInVicinity, mode)) tmp << itemInVicinity; } @@ -2303,7 +2309,8 @@ void QGraphicsScene::setSelectionArea(const QPainterPath &path, bool changed = false; // Set all items in path to selected. - foreach (QGraphicsItem *item, items(path, mode, Qt::DescendingOrder, deviceTransform)) { + const auto items = this->items(path, mode, Qt::DescendingOrder, deviceTransform); + for (QGraphicsItem *item : items) { if (item->flags() & QGraphicsItem::ItemIsSelectable) { if (!item->isSelected()) changed = true; @@ -2444,7 +2451,8 @@ QGraphicsItemGroup *QGraphicsScene::createItemGroup(const QList */ void QGraphicsScene::destroyItemGroup(QGraphicsItemGroup *group) { - foreach (QGraphicsItem *item, group->childItems()) + const auto items = group->childItems(); + for (QGraphicsItem *item : items) group->removeFromGroup(item); removeItem(group); delete group; @@ -2559,7 +2567,8 @@ void QGraphicsScene::addItem(QGraphicsItem *item) } #ifndef QT_NO_GESTURES - foreach (Qt::GestureType gesture, item->d_ptr->gestureContext.keys()) + const auto gestures = item->d_ptr->gestureContext.keys(); // FIXME: iterate over hash directly? + for (Qt::GestureType gesture : gestures) d->grabGesture(item, gesture); #endif @@ -3130,7 +3139,8 @@ void QGraphicsScene::setForegroundBrush(const QBrush &brush) { Q_D(QGraphicsScene); d->foregroundBrush = brush; - foreach (QGraphicsView *view, views()) + const auto views_ = views(); + for (QGraphicsView *view : views_) view->viewport()->update(); update(); } @@ -3238,7 +3248,8 @@ void QGraphicsScene::update(const QRectF &rect) */ void QGraphicsScene::invalidate(const QRectF &rect, SceneLayers layers) { - foreach (QGraphicsView *view, views()) + const auto views_ = views(); + for (QGraphicsView *view : views_) view->invalidateScene(rect, layers); update(rect); } @@ -3279,7 +3290,8 @@ QList QGraphicsScene::views() const void QGraphicsScene::advance() { for (int i = 0; i < 2; ++i) { - foreach (QGraphicsItem *item, items()) + const auto items_ = items(); + for (QGraphicsItem *item : items_) item->advance(i); } } @@ -3434,7 +3446,8 @@ bool QGraphicsScene::event(QEvent *event) } else { // Activate all toplevel items. QEvent event(QEvent::WindowActivate); - foreach (QGraphicsItem *item, items()) { + const auto items_ = items(); + for (QGraphicsItem *item : items_) { if (item->isVisible() && !item->isPanel() && !item->parentItem()) sendEvent(item, &event); } @@ -3452,7 +3465,8 @@ bool QGraphicsScene::event(QEvent *event) } else { // Activate all toplevel items. QEvent event(QEvent::WindowDeactivate); - foreach (QGraphicsItem *item, items()) { + const auto items_ = items(); + for (QGraphicsItem *item : items_) { if (item->isVisible() && !item->isPanel() && !item->parentItem()) sendEvent(item, &event); } @@ -3547,9 +3561,10 @@ void QGraphicsScene::contextMenuEvent(QGraphicsSceneContextMenuEvent *contextMen // Send the event to all items at this position until one item accepts the // event. - foreach (QGraphicsItem *item, d->itemsAtPosition(contextMenuEvent->screenPos(), - contextMenuEvent->scenePos(), - contextMenuEvent->widget())) { + const auto items = d->itemsAtPosition(contextMenuEvent->screenPos(), + contextMenuEvent->scenePos(), + contextMenuEvent->widget()); + for (QGraphicsItem *item : items) { contextMenuEvent->setPos(item->d_ptr->genericMapFromScene(contextMenuEvent->scenePos(), contextMenuEvent->widget())); contextMenuEvent->accept(); @@ -3605,9 +3620,10 @@ void QGraphicsScene::dragMoveEvent(QGraphicsSceneDragDropEvent *event) // Find the topmost enabled items under the cursor. They are all // candidates for accepting drag & drop events. - foreach (QGraphicsItem *item, d->itemsAtPosition(event->screenPos(), - event->scenePos(), - event->widget())) { + const auto items = d->itemsAtPosition(event->screenPos(), + event->scenePos(), + event->widget()); + for (QGraphicsItem *item : items) { if (!item->isEnabled() || !item->acceptDrops()) continue; @@ -4635,7 +4651,8 @@ void QGraphicsScenePrivate::drawItemHelper(QGraphicsItem *item, QPainter *painte for (int i = 0; i < exposed.size(); ++i) br |= exposed.at(i); QTransform pixmapToItem = itemToPixmap.inverted(); - foreach (const QRect &r, scrollExposure.rects()) + const auto rects = scrollExposure.rects(); + for (const QRect &r : rects) br |= pixmapToItem.mapRect(r); } styleOptionTmp = *option; @@ -5564,7 +5581,8 @@ void QGraphicsScene::setStyle(QStyle *style) QApplication::sendEvent(this, &event); // Notify all widgets that don't have a style explicitly set. - foreach (QGraphicsItem *item, items()) { + const auto items_ = items(); + for (QGraphicsItem *item : items_) { if (item->isWidget()) { QGraphicsWidget *widget = static_cast(item); if (!widget->testAttribute(Qt::WA_SetStyle)) @@ -5733,7 +5751,8 @@ void QGraphicsScene::setActiveWindow(QGraphicsWidget *widget) // Find the highest z value. qreal z = panel->zValue(); - foreach (QGraphicsItem *sibling, parent ? parent->childItems() : items()) { + const auto siblings = parent ? parent->childItems() : items(); + for (QGraphicsItem *sibling : siblings) { if (sibling != panel && sibling->isWindow()) z = qMax(z, sibling->zValue()); } @@ -5819,7 +5838,8 @@ void QGraphicsScenePrivate::addView(QGraphicsView *view) { views << view; #ifndef QT_NO_GESTURES - foreach (Qt::GestureType gesture, grabbedGestures.keys()) + const auto gestures = grabbedGestures.keys(); + for (Qt::GestureType gesture : gestures) view->viewport()->grabGesture(gesture); #endif } @@ -6304,7 +6324,8 @@ void QGraphicsScenePrivate::gestureEventHandler(QGestureEvent *event) QGraphicsObject *item = cachedTargetItems.at(i); // get gestures to deliver to the current item - foreach (QGesture *g, cachedItemGestures.value(item)) { + const auto gestures = cachedItemGestures.value(item); + for (QGesture *g : gestures) { if (!gestureTargets.contains(g)) { gestureTargets.insert(g, item); normalGestures.remove(g); diff --git a/src/widgets/graphicsview/qgraphicsview.cpp b/src/widgets/graphicsview/qgraphicsview.cpp index ac8cd45f9e..928d826323 100644 --- a/src/widgets/graphicsview/qgraphicsview.cpp +++ b/src/widgets/graphicsview/qgraphicsview.cpp @@ -799,7 +799,8 @@ void QGraphicsViewPrivate::_q_setViewportCursor(const QCursor &cursor) void QGraphicsViewPrivate::_q_unsetViewportCursor() { Q_Q(QGraphicsView); - foreach (QGraphicsItem *item, q->items(lastMouseEvent.pos())) { + const auto items = q->items(lastMouseEvent.pos()); + for (QGraphicsItem *item : items) { if (item->hasCursor()) { _q_setViewportCursor(item->cursor()); return; @@ -1139,7 +1140,8 @@ QList QGraphicsViewPrivate::findItems(const QRegion &exposedReg // 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()) + const auto rects = exposedRegion.rects(); + for (const QRect &r : rects) adjustedRegion += r.adjusted(-1, -1, 1, 1); const QPainterPath exposedScenePath(q->mapToScene(qt_regionToPath(adjustedRegion))); @@ -2791,7 +2793,8 @@ void QGraphicsView::setupViewport(QWidget *widget) #ifndef QT_NO_GESTURES if (d->scene) { - foreach (Qt::GestureType gesture, d->scene->d_func()->grabbedGestures.keys()) + const auto gestures = d->scene->d_func()->grabbedGestures.keys(); + for (Qt::GestureType gesture : gestures) widget->grabGesture(gesture); } #endif diff --git a/src/widgets/graphicsview/qgraphicswidget.cpp b/src/widgets/graphicsview/qgraphicswidget.cpp index ef086aeab0..db1cdd42ab 100644 --- a/src/widgets/graphicsview/qgraphicswidget.cpp +++ b/src/widgets/graphicsview/qgraphicswidget.cpp @@ -251,7 +251,8 @@ QGraphicsWidget::~QGraphicsWidget() //we check if we have a layout previously if (d->layout) { QGraphicsLayout *temp = d->layout; - foreach (QGraphicsItem * item, childItems()) { + const auto items = childItems(); + for (QGraphicsItem *item : items) { // In case of a custom layout which doesn't remove and delete items, we ensure that // the parent layout item does not point to the deleted layout. This code is here to // avoid regression from 4.4 to 4.5, because according to 4.5 docs it is not really needed. -- cgit v1.2.3