From f9ca7ba523c2cd975a9935ce619c26d0faa9c4d8 Mon Sep 17 00:00:00 2001 From: vicquick Date: Fri, 10 Apr 2026 01:15:52 +0200 Subject: [PATCH 01/14] Fix Layer Styling panel crash when switching saved styles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit styleClicked() was connected to QAbstractItemView::clicked with a direct connection. When the user clicks a saved style in the Layer Styling panel's Style Manager tab, styleClicked() calls setCurrentStyle() synchronously from inside QListView::mouseReleaseEvent(). setCurrentStyle() emits currentStyleChanged(), which triggers currentStyleChanged() → mStyleList->setCurrentIndex(). This modifies the view's selection model while mouseReleaseEvent still holds a QPersistentModelIndex (d->pressedIndex). On Qt 6, the invalidated persistent index causes a use-after-free when mouseReleaseEvent continues after the clicked() handler returns. On Windows: access violation (0xbaadf00d = freed heap). On Linux: SIGSEGV → SIGABRT. The crash does NOT happen when switching styles via: - Right-click layer → Styles submenu (QAction::triggered fires from the menu event loop, outside any view event handler) - Layer Properties dialog (QComboBox::currentIndexChanged, also outside a view event handler) Fix: change the clicked → styleClicked connection to Qt::QueuedConnection, so styleClicked() runs on the next event loop iteration — after mouseReleaseEvent has fully completed and released d->pressedIndex. Assisted-by: Claude (Anthropic) --- src/gui/qgsmaplayerstylemanagerwidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/qgsmaplayerstylemanagerwidget.cpp b/src/gui/qgsmaplayerstylemanagerwidget.cpp index 61166ba851e5..472928c43378 100644 --- a/src/gui/qgsmaplayerstylemanagerwidget.cpp +++ b/src/gui/qgsmaplayerstylemanagerwidget.cpp @@ -70,7 +70,7 @@ QgsMapLayerStyleManagerWidget::QgsMapLayerStyleManagerWidget( QgsMapLayer *layer QAction *loadDefaultAction = toolbar->addAction( tr( "Restore Default" ) ); connect( loadDefaultAction, &QAction::triggered, this, &QgsMapLayerStyleManagerWidget::loadDefault ); - connect( mStyleList, &QAbstractItemView::clicked, this, &QgsMapLayerStyleManagerWidget::styleClicked ); + connect( mStyleList, &QAbstractItemView::clicked, this, &QgsMapLayerStyleManagerWidget::styleClicked, Qt::QueuedConnection ); setLayout( new QVBoxLayout() ); layout()->setContentsMargins( 0, 0, 0, 0 ); From d86d3e5c2f065bcda1d6ed082014acdb7f00f62f Mon Sep 17 00:00:00 2001 From: Victor <88567707+vicquick@users.noreply.github.com> Date: Wed, 8 Apr 2026 16:18:11 +0200 Subject: [PATCH 02/14] Fix layout items list selection update race causing crash and duplicate rows QgsLayoutItemsListView::updateSelection() was connected to QItemSelectionModel::selectionChanged with a direct connection. When the source QgsLayoutModel begins a row insertion (for example when creating a new group via QgsLayoutView::groupSelectedItems -> addLayoutItem -> rebuildZList -> beginInsertRows) and there are persistent indices held by the items list view's selection model, those indices shift and QItemSelectionModel emits selectionChanged synchronously, from inside the still-open beginInsertRows transaction. updateSelection() then runs while the source model is mid-transaction and calls setSelected() on layout items, which emits dataChanged() on the source model. Emitting dataChanged from inside a beginInsertRows/endInsertRows bracket is a QAbstractItemModel contract violation. The observable consequences differed by Qt version: - Qt 6.8: QTreeView::dataChanged eagerly touches the selection model, which re-emits selectionChanged, re-enters updateSelection(), and the cycle recurses until the stack overflows. - Qt 6.9+: The mid-transaction dataChanged corrupts QSortFilterProxyModel's internal source-to-proxy mapping cache. The newly inserted row ends up with two proxy rows mapping to the same source row, so the Items list panel visibly shows a duplicate entry for the newly created group. Closing and reopening the layout rebuilds the proxy from scratch and the duplicate disappears. Both symptoms share the same root cause: updateSelection() running synchronously inside an open source-model transaction. Making the selectionChanged -> updateSelection connection a Qt::QueuedConnection defers the slot to the next event loop iteration, by which time endInsertRows() has fired and the proxy is in a consistent state. The mUpdatingSelection re-entry guard is also added to updateSelection()'s early return, matching the guard that onItemFocused() already uses. With Qt::QueuedConnection this is defense in depth, but it is cheap and protects against future refactors that might reintroduce a synchronous path into the slot. Fixes #61702 Assisted-by: Claude (Anthropic) --- src/gui/layout/qgslayoutitemslistview.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index be41409a6c85..95ab68c72de0 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -119,7 +119,10 @@ void QgsLayoutItemsListView::setCurrentLayout( QgsLayout *layout ) setColumnWidth( 1, Qgis::UI_SCALE_FACTOR * fontMetrics().horizontalAdvance( 'x' ) * 4 ); header()->setSectionsMovable( false ); - connect( selectionModel(), &QItemSelectionModel::selectionChanged, this, &QgsLayoutItemsListView::updateSelection ); + // Queued so updateSelection() does not run synchronously from inside + // a source model row insertion (e.g. when grouping items), which + // would emit dataChanged mid-transaction and corrupt the proxy. + connect( selectionModel(), &QItemSelectionModel::selectionChanged, this, &QgsLayoutItemsListView::updateSelection, Qt::QueuedConnection ); } void QgsLayoutItemsListView::keyPressEvent( QKeyEvent *event ) @@ -150,8 +153,8 @@ void QgsLayoutItemsListView::updateSelection() { // Do nothing if we are currently updating the selection // because user has selected/deselected some items in the - // graphics view - if ( !mModel || mUpdatingFromView ) + // graphics view, or if we are already inside this method. + if ( !mModel || mUpdatingFromView || mUpdatingSelection ) return; // Set the updating flag From 9bbb9ed184fb21f3dbc3e2a2fe935f6c0f5378ad Mon Sep 17 00:00:00 2001 From: vicquick Date: Sat, 25 Apr 2026 16:42:38 +0200 Subject: [PATCH 03/14] feat: hierarchical layout items model + tree view - QgsLayoutModel: implement parent(), restructure index()/rowCount() so child items of a QgsLayoutItemGroup appear nested under their group instead of flat at the root. Two helpers, topLevelItemsInScene() and childItemsInScene(group), filter the existing scene-order list by parentGroup() membership. The root sentinel at top-level row 0 is preserved (proxy filters hide it from the user-facing panel). - QgsLayoutModel::indexForItem(): walk up parentGroup() chain to build the hierarchical index, instead of the flat row+1 lookup. - QgsLayoutItemsListView: setIndentation(16), setRootIsDecorated(true), setAnimated(true) so the QTreeView actually renders the hierarchy with disclosure arrows now that the model exposes it. Drag-and-drop reorder still rejects non-root parents pending per-group local z-stack support in QgsLayoutItemGroup. Assisted-by: Claude Opus 4.7 --- src/core/layout/qgslayoutmodel.cpp | 129 +++++++++++++++++----- src/core/layout/qgslayoutmodel.h | 14 +++ src/gui/layout/qgslayoutitemslistview.cpp | 4 +- 3 files changed, 116 insertions(+), 31 deletions(-) diff --git a/src/core/layout/qgslayoutmodel.cpp b/src/core/layout/qgslayoutmodel.cpp index 9ac23e913b2e..0cace12e0eda 100644 --- a/src/core/layout/qgslayoutmodel.cpp +++ b/src/core/layout/qgslayoutmodel.cpp @@ -44,13 +44,39 @@ QgsLayoutModel::QgsLayoutModel( QgsLayout *layout, QObject *parent ) QgsLayoutItem *QgsLayoutModel::itemFromIndex( const QModelIndex &index ) const { //try to return the QgsLayoutItem corresponding to a QModelIndex - if ( !index.isValid() || index.row() == 0 ) + if ( !index.isValid() ) { return nullptr; } - QgsLayoutItem *item = static_cast( index.internalPointer() ); - return item; + // Internal pointer is the QgsLayoutItem * (or nullptr for the root sentinel + // at top-level row 0). The static_cast naturally yields nullptr for that case. + return static_cast( index.internalPointer() ); +} + +QList QgsLayoutModel::topLevelItemsInScene() const +{ + QList result; + result.reserve( mItemsInScene.size() ); + for ( QgsLayoutItem *item : mItemsInScene ) + { + if ( !item->parentGroup() ) + result.append( item ); + } + return result; +} + +QList QgsLayoutModel::childItemsInScene( QgsLayoutItemGroup *group ) const +{ + QList result; + if ( !group ) + return result; + for ( QgsLayoutItem *item : mItemsInScene ) + { + if ( item->parentGroup() == group ) + result.append( item ); + } + return result; } QModelIndex QgsLayoutModel::index( int row, int column, const QModelIndex &parent ) const @@ -61,17 +87,33 @@ QModelIndex QgsLayoutModel::index( int row, int column, const QModelIndex &paren return QModelIndex(); } - if ( !parent.isValid() && row == 0 ) + if ( !parent.isValid() ) { - return createIndex( row, column, nullptr ); + if ( row == 0 ) + { + // root sentinel — paper item placeholder, hidden by the items panel proxy + return createIndex( row, column, nullptr ); + } + + const QList top = topLevelItemsInScene(); + if ( row >= 1 && row <= top.size() ) + { + return createIndex( row, column, top.at( row - 1 ) ); + } + return QModelIndex(); } - else if ( !parent.isValid() && row >= 1 && row < mItemsInScene.size() + 1 ) + + // parent must be a group for there to be children + QgsLayoutItem *parentItem = itemFromIndex( parent ); + QgsLayoutItemGroup *group = qobject_cast( parentItem ); + if ( !group ) + return QModelIndex(); + + const QList children = childItemsInScene( group ); + if ( row >= 0 && row < children.size() ) { - //return an index for the layout item at this position - return createIndex( row, column, mItemsInScene.at( row - 1 ) ); + return createIndex( row, column, children.at( row ) ); } - - //only top level supported for now return QModelIndex(); } @@ -93,31 +135,50 @@ void QgsLayoutModel::refreshItemsInScene() QModelIndex QgsLayoutModel::parent( const QModelIndex &index ) const { - Q_UNUSED( index ) + if ( !index.isValid() ) + return QModelIndex(); - //all items are top level for now - return QModelIndex(); + QgsLayoutItem *item = static_cast( index.internalPointer() ); + if ( !item ) + return QModelIndex(); + + QgsLayoutItemGroup *parentGroup = item->parentGroup(); + if ( !parentGroup ) + return QModelIndex(); + + // Find parent's row in ITS parent's child list + QgsLayoutItemGroup *grandparent = parentGroup->parentGroup(); + int parentRow = -1; + if ( grandparent ) + { + const QList siblings = childItemsInScene( grandparent ); + parentRow = siblings.indexOf( parentGroup ); + } + else + { + const QList top = topLevelItemsInScene(); + parentRow = top.indexOf( parentGroup ); + if ( parentRow >= 0 ) + parentRow += 1; // shift past the root sentinel at top level + } + if ( parentRow < 0 ) + return QModelIndex(); + return createIndex( parentRow, 0, parentGroup ); } int QgsLayoutModel::rowCount( const QModelIndex &parent ) const { if ( !parent.isValid() ) { - return mItemsInScene.size() + 1; + // top-level rows = sentinel + items with no parent group + return topLevelItemsInScene().size() + 1; } -#if 0 - QGraphicsItem *parentItem = itemFromIndex( parent ); - - if ( parentItem ) - { - // return child count for item + QgsLayoutItem *parentItem = itemFromIndex( parent ); + QgsLayoutItemGroup *group = qobject_cast( parentItem ); + if ( !group ) return 0; - } -#endif - - //no children for now - return 0; + return childItemsInScene( group ).size(); } int QgsLayoutModel::columnCount( const QModelIndex &parent ) const @@ -906,14 +967,22 @@ QModelIndex QgsLayoutModel::indexForItem( QgsLayoutItem *item, const int column return QModelIndex(); } - int row = mItemsInScene.indexOf( item ); - if ( row == -1 ) + QgsLayoutItemGroup *parentGroup = item->parentGroup(); + if ( !parentGroup ) { - //not found - return QModelIndex(); + const QList top = topLevelItemsInScene(); + int row = top.indexOf( item ); + if ( row < 0 ) + return QModelIndex(); + return index( row + 1, column ); // +1 for sentinel } - return index( row + 1, column ); + const QList siblings = childItemsInScene( parentGroup ); + int row = siblings.indexOf( item ); + if ( row < 0 ) + return QModelIndex(); + QModelIndex parentIdx = indexForItem( parentGroup, 0 ); + return index( row, column, parentIdx ); } ///@cond PRIVATE diff --git a/src/core/layout/qgslayoutmodel.h b/src/core/layout/qgslayoutmodel.h index 2386738611fb..27ce70973998 100644 --- a/src/core/layout/qgslayoutmodel.h +++ b/src/core/layout/qgslayoutmodel.h @@ -289,6 +289,20 @@ class CORE_EXPORT QgsLayoutModel : public QAbstractItemModel */ void rebuildSceneItemList(); + /** + * Returns items from mItemsInScene that have no parent group. + * Order preserves the existing global z-order slice. + */ + QList topLevelItemsInScene() const; + + /** + * Returns items from mItemsInScene whose parentGroup() is \a group. + * Order preserves the existing global z-order slice — once + * QgsLayoutItemGroup gains its own local z-stack this will defer + * to that ordering. + */ + QList childItemsInScene( class QgsLayoutItemGroup *group ) const; + friend class TestQgsLayoutModel; friend class TestQgsLayoutGui; }; diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index 95ab68c72de0..1c41a466afe5 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -97,7 +97,9 @@ QgsLayoutItemsListView::QgsLayoutItemsListView( QWidget *parent, QgsLayoutDesign setDropIndicatorShown( true ); setDragDropMode( QAbstractItemView::InternalMove ); setContextMenuPolicy( Qt::CustomContextMenu ); - setIndentation( 0 ); + setIndentation( 16 ); + setRootIsDecorated( true ); + setAnimated( true ); // Allow multi selection from the list view setSelectionMode( QAbstractItemView::ExtendedSelection ); From 0d7e3391eca3a4e93356b4569fc9c8959a6a775b Mon Sep 17 00:00:00 2001 From: vicquick Date: Sat, 25 Apr 2026 16:44:45 +0200 Subject: [PATCH 04/14] feat: per-group local z-stack + auto-nest by z on group creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - QgsLayoutItemGroup gains reorderItemUp/Down/ToTop/ToBottom that mutate the group's mItems QList (which now has documented semantics: index 0 = topmost member, last index = bottommost). Persistence to ComposerItemGroupElement was already iteration-order based, so the local z-stack survives save/reload with no schema change. - QgsLayoutModel::childItemsInScene now defers to QgsLayoutItemGroup::items() for ordering, so reorder methods are immediately reflected in the layout items panel tree. - QgsLayout::groupItems sorts incoming selection by current global zValue (descending) before populating the new group, so the visually topmost selected item becomes the top of the group's local stack — matching the user's mental model when grouping a stack of items. Assisted-by: Claude Opus 4.7 --- src/core/layout/qgslayout.cpp | 12 ++++++- src/core/layout/qgslayoutitemgroup.cpp | 46 ++++++++++++++++++++++++++ src/core/layout/qgslayoutitemgroup.h | 27 ++++++++++++++- src/core/layout/qgslayoutmodel.cpp | 7 ++-- 4 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/core/layout/qgslayout.cpp b/src/core/layout/qgslayout.cpp index a97db3a1bb18..1bbf8b764a30 100644 --- a/src/core/layout/qgslayout.cpp +++ b/src/core/layout/qgslayout.cpp @@ -780,7 +780,17 @@ QgsLayoutItemGroup *QgsLayout::groupItems( const QList &items ) mUndoStack->beginMacro( tr( "Group Items" ) ); auto itemGroup = std::make_unique( this ); - for ( QgsLayoutItem *item : items ) + + // Sort the selection by current global z-order (descending) so the + // group's local z-stack reflects what the user saw on the canvas: + // the visually-topmost selected item is the topmost group member. + QList orderedItems = items; + std::sort( orderedItems.begin(), orderedItems.end(), + []( QgsLayoutItem * a, QgsLayoutItem * b ) + { + return a->zValue() > b->zValue(); + } ); + for ( QgsLayoutItem *item : orderedItems ) { itemGroup->addItem( item ); } diff --git a/src/core/layout/qgslayoutitemgroup.cpp b/src/core/layout/qgslayoutitemgroup.cpp index 9870ad920cbd..e1d164b2f506 100644 --- a/src/core/layout/qgslayoutitemgroup.cpp +++ b/src/core/layout/qgslayoutitemgroup.cpp @@ -120,6 +120,52 @@ QList QgsLayoutItemGroup::items() const return val; } +static int indexOfItemPtr( const QList< QPointer< QgsLayoutItem > > &list, QgsLayoutItem *item ) +{ + for ( int i = 0; i < list.size(); ++i ) + { + if ( list.at( i ).data() == item ) + return i; + } + return -1; +} + +bool QgsLayoutItemGroup::reorderItemUp( QgsLayoutItem *item ) +{ + int idx = indexOfItemPtr( mItems, item ); + if ( idx <= 0 ) + return false; // not in group, or already at top + mItems.move( idx, idx - 1 ); + return true; +} + +bool QgsLayoutItemGroup::reorderItemDown( QgsLayoutItem *item ) +{ + int idx = indexOfItemPtr( mItems, item ); + if ( idx < 0 || idx >= mItems.size() - 1 ) + return false; // not in group, or already at bottom + mItems.move( idx, idx + 1 ); + return true; +} + +bool QgsLayoutItemGroup::reorderItemToTop( QgsLayoutItem *item ) +{ + int idx = indexOfItemPtr( mItems, item ); + if ( idx <= 0 ) + return false; + mItems.move( idx, 0 ); + return true; +} + +bool QgsLayoutItemGroup::reorderItemToBottom( QgsLayoutItem *item ) +{ + int idx = indexOfItemPtr( mItems, item ); + if ( idx < 0 || idx >= mItems.size() - 1 ) + return false; + mItems.move( idx, mItems.size() - 1 ); + return true; +} + void QgsLayoutItemGroup::setVisibility( const bool visible ) { if ( !shouldBlockUndoCommands() ) diff --git a/src/core/layout/qgslayoutitemgroup.h b/src/core/layout/qgslayoutitemgroup.h index 76c30d12f347..d20c1da7cf69 100644 --- a/src/core/layout/qgslayoutitemgroup.h +++ b/src/core/layout/qgslayoutitemgroup.h @@ -60,10 +60,35 @@ class CORE_EXPORT QgsLayoutItemGroup : public QgsLayoutItem void removeItems(); /** - * Returns a list of items contained by the group. + * Returns a list of items contained by the group, in local z-order + * (index 0 = topmost within the group, last index = bottommost). */ QList items() const; + /** + * Moves an \a item one step toward the top of the group's local z-stack. + * Returns TRUE if \a item was moved. + */ + bool reorderItemUp( QgsLayoutItem *item ); + + /** + * Moves an \a item one step toward the bottom of the group's local z-stack. + * Returns TRUE if \a item was moved. + */ + bool reorderItemDown( QgsLayoutItem *item ); + + /** + * Moves an \a item to the top of the group's local z-stack. + * Returns TRUE if \a item was moved. + */ + bool reorderItemToTop( QgsLayoutItem *item ); + + /** + * Moves an \a item to the bottom of the group's local z-stack. + * Returns TRUE if \a item was moved. + */ + bool reorderItemToBottom( QgsLayoutItem *item ); + //overridden to also hide grouped items void setVisibility( bool visible ) override; diff --git a/src/core/layout/qgslayoutmodel.cpp b/src/core/layout/qgslayoutmodel.cpp index 0cace12e0eda..a8c95aa6e412 100644 --- a/src/core/layout/qgslayoutmodel.cpp +++ b/src/core/layout/qgslayoutmodel.cpp @@ -71,9 +71,12 @@ QList QgsLayoutModel::childItemsInScene( QgsLayoutItemGroup *gr QList result; if ( !group ) return result; - for ( QgsLayoutItem *item : mItemsInScene ) + // Honor the group's local z-stack (mItems order) so reorderItemUp/Down + // is reflected in the tree. Only include items currently in the scene. + const QList groupItems = group->items(); + for ( QgsLayoutItem *item : groupItems ) { - if ( item->parentGroup() == group ) + if ( item && mItemsInScene.contains( item ) ) result.append( item ); } return result; From 6263ab8381660a73c7cdbb23f421c4091544ab6d Mon Sep 17 00:00:00 2001 From: vicquick Date: Sat, 25 Apr 2026 16:47:28 +0200 Subject: [PATCH 05/14] feat: drill-into-group on Ctrl-click + double-click isolation mode QgsLayoutViewToolSelect: - The existing CTRL stack-cycle now also skips the "promote to top-level group" step, letting users click straight onto a nested member instead of always grabbing the enclosing group. Mirrors Adobe Illustrator's Group Selection / Direct Selection semantics with a single modifier. - Double-click on a group (or on any item inside one) enters isolation mode: every other layout item is dimmed to 25 % opacity so editing happens visually scoped to the group. Pages stay full opacity. - Esc, double-click on empty space, or tool deactivation restores every dimmed item to its original opacity. Original opacities are cached in mDimmedItems so a user-set opacity is preserved. - mIsolatedGroup tracked via QPointer so a deleted group is harmless. Assisted-by: Claude Opus 4.7 --- src/gui/layout/qgslayoutviewtoolselect.cpp | 128 ++++++++++++++++++++- src/gui/layout/qgslayoutviewtoolselect.h | 33 ++++++ 2 files changed, 155 insertions(+), 6 deletions(-) diff --git a/src/gui/layout/qgslayoutviewtoolselect.cpp b/src/gui/layout/qgslayoutviewtoolselect.cpp index 275f90d14d76..5c056b7648e3 100644 --- a/src/gui/layout/qgslayoutviewtoolselect.cpp +++ b/src/gui/layout/qgslayoutviewtoolselect.cpp @@ -17,9 +17,14 @@ #include +#include +#include + #include "qgslayout.h" +#include "qgslayoutitem.h" #include "qgslayoutitemgroup.h" #include "qgslayoutitempage.h" +#include "qgslayoutitemregistry.h" #include "qgslayoutmousehandles.h" #include "qgslayoutview.h" #include "qgslayoutviewmouseevent.h" @@ -114,14 +119,22 @@ void QgsLayoutViewToolSelect::layoutPressEvent( QgsLayoutViewMouseEvent *event ) selectedItem = layout()->layoutItemAt( event->layoutPoint(), true, searchToleranceInLayoutUnits() ); } - // if selected item is in a group, we actually get the top-level group it's part of - QgsLayoutItemGroup *group = selectedItem ? selectedItem->parentGroup() : nullptr; - while ( group && group->parentGroup() ) + // If the selected item is in a group, we normally promote to the top-level + // group it belongs to so the whole group is selected on a plain click. + // CTRL skips this promotion so the user can drill into nested members + // directly (mirrors Adobe Group Selection / Direct Selection semantics) + // — this composes naturally with the existing CTRL stack-cycle above. + const bool drillIntoGroup = event->modifiers() & Qt::ControlModifier; + if ( !drillIntoGroup ) { - group = group->parentGroup(); + QgsLayoutItemGroup *group = selectedItem ? selectedItem->parentGroup() : nullptr; + while ( group && group->parentGroup() ) + { + group = group->parentGroup(); + } + if ( group ) + selectedItem = group; } - if ( group ) - selectedItem = group; if ( !selectedItem ) { @@ -309,6 +322,13 @@ void QgsLayoutViewToolSelect::wheelEvent( QWheelEvent *event ) void QgsLayoutViewToolSelect::keyPressEvent( QKeyEvent *event ) { + if ( event->key() == Qt::Key_Escape && mIsolatedGroup ) + { + exitIsolation(); + event->accept(); + return; + } + if ( mMouseHandles->isDragging() || mMouseHandles->isResizing() ) { return; @@ -319,8 +339,104 @@ void QgsLayoutViewToolSelect::keyPressEvent( QKeyEvent *event ) } } +void QgsLayoutViewToolSelect::layoutDoubleClickEvent( QgsLayoutViewMouseEvent *event ) +{ + if ( event->button() != Qt::LeftButton ) + { + event->ignore(); + return; + } + + QgsLayoutItem *hit = layout()->layoutItemAt( event->layoutPoint(), true, + searchToleranceInLayoutUnits() ); + if ( !hit ) + { + // Double-click on empty area exits isolation + if ( mIsolatedGroup ) + { + exitIsolation(); + event->accept(); + return; + } + event->ignore(); + return; + } + + // The double-click target is either a group itself, or its innermost + // containing group — that's the one we isolate. + QgsLayoutItemGroup *target = qobject_cast( hit ); + if ( !target ) + target = hit->parentGroup(); + if ( !target ) + { + event->ignore(); + return; + } + + enterIsolation( target ); + event->accept(); +} + +void QgsLayoutViewToolSelect::enterIsolation( QgsLayoutItemGroup *group ) +{ + if ( !group || !layout() ) + return; + + // If already isolating something else, restore first. + if ( mIsolatedGroup && mIsolatedGroup != group ) + exitIsolation(); + + // Build the set of items considered "inside" the isolation: the group + // itself plus all transitive descendants. + QSet inside; + inside.insert( group ); + QList stack = group->items(); + while ( !stack.isEmpty() ) + { + QgsLayoutItem *it = stack.takeLast(); + if ( !it || inside.contains( it ) ) + continue; + inside.insert( it ); + if ( QgsLayoutItemGroup *nested = qobject_cast( it ) ) + stack.append( nested->items() ); + } + + // Dim everything else (excluding pages, which we always leave fully visible). + const QList sceneItems = layout()->items(); + for ( QGraphicsItem *gi : sceneItems ) + { + QgsLayoutItem *li = dynamic_cast( gi ); + if ( !li ) + continue; + if ( li->type() == QgsLayoutItemRegistry::LayoutPage ) + continue; + if ( inside.contains( li ) ) + continue; + if ( !mDimmedItems.contains( li ) ) + mDimmedItems.insert( li, li->opacity() ); + li->setOpacity( sIsolationDimOpacity ); + } + mIsolatedGroup = group; +} + +void QgsLayoutViewToolSelect::exitIsolation() +{ + if ( !mIsolatedGroup && mDimmedItems.isEmpty() ) + return; + + for ( auto it = mDimmedItems.constBegin(); it != mDimmedItems.constEnd(); ++it ) + { + if ( it.key() ) + it.key()->setOpacity( it.value() ); + } + mDimmedItems.clear(); + mIsolatedGroup = nullptr; +} + void QgsLayoutViewToolSelect::deactivate() { + if ( mIsolatedGroup ) + exitIsolation(); if ( mIsSelecting ) { mRubberBand->finish(); diff --git a/src/gui/layout/qgslayoutviewtoolselect.h b/src/gui/layout/qgslayoutviewtoolselect.h index f45b0c1ae060..e32819ac1bef 100644 --- a/src/gui/layout/qgslayoutviewtoolselect.h +++ b/src/gui/layout/qgslayoutviewtoolselect.h @@ -23,7 +23,12 @@ #include "qgslayoutviewrubberband.h" #include "qgslayoutviewtool.h" +#include +#include + class QgsLayoutMouseHandles; +class QgsLayoutItemGroup; +class QgsLayoutItem; /** * \ingroup gui @@ -43,10 +48,30 @@ class GUI_EXPORT QgsLayoutViewToolSelect : public QgsLayoutViewTool void layoutPressEvent( QgsLayoutViewMouseEvent *event ) override; void layoutMoveEvent( QgsLayoutViewMouseEvent *event ) override; void layoutReleaseEvent( QgsLayoutViewMouseEvent *event ) override; + void layoutDoubleClickEvent( QgsLayoutViewMouseEvent *event ) override; void wheelEvent( QWheelEvent *event ) override; void keyPressEvent( QKeyEvent *event ) override; void deactivate() override; + /** + * Returns the currently isolated group, or NULLPTR when no group is + * being isolated. + */ + QgsLayoutItemGroup *isolatedGroup() const { return mIsolatedGroup; } + + /** + * Enters isolation mode for \a group. While isolated, items outside + * the group are visually dimmed so the user can edit group members + * without distraction. Mirrors Adobe Illustrator's group isolation. + */ + void enterIsolation( QgsLayoutItemGroup *group ); + + /** + * Exits isolation mode and restores all items to their normal opacity. + * No-op if no group is currently isolated. + */ + void exitIsolation(); + ///@cond PRIVATE /** @@ -81,6 +106,14 @@ class GUI_EXPORT QgsLayoutViewToolSelect : public QgsLayoutViewTool //! Search tolerance in millimeters for selecting items static const double sSearchToleranceInMillimeters; + + //! Group currently isolated (NULLPTR = no isolation) + QPointer mIsolatedGroup; + + //! Items dimmed by enterIsolation, mapped to their original opacity + QHash mDimmedItems; + + static constexpr qreal sIsolationDimOpacity = 0.25; }; #endif // QGSLAYOUTVIEWTOOLSELECT_H From 4f0e92d5046ae516de6d3d5b7073042388346b06 Mon Sep 17 00:00:00 2001 From: vicquick Date: Sat, 25 Apr 2026 16:48:45 +0200 Subject: [PATCH 06/14] feat: items panel selection no longer auto-promotes to top-level group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QgsLayoutItemsListView::updateSelection used to walk parentGroup() up to the outermost group and add it to the layout selection alongside the clicked child. That was useful when the panel was the only way to grab the group at all, but now that the canvas tool handles the click → group promotion (and Ctrl-click handles drill-in), the panel becomes the precise / direct selection surface — Adobe Layers panel semantics. A click on a child row now selects exactly that child. Group rows still work the same: clicking the group row selects the group. Assisted-by: Claude Opus 4.7 --- src/gui/layout/qgslayoutitemslistview.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index 1c41a466afe5..26a9a6210fa2 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -201,14 +201,10 @@ void QgsLayoutItemsListView::updateSelection() item->setSelected( true ); } - // find top level group this item is contained within, and mark the group as selected - QgsLayoutItemGroup *group = item->parentGroup(); - while ( group && group->parentGroup() ) - { - group = group->parentGroup(); - } - if ( group && group != item ) - group->setSelected( true ); + // The items panel is the precise/direct selection surface (mirrors + // the Adobe Layers panel): clicking a child selects only that child, + // not its enclosing group. Use the canvas selection tool for + // whole-group selection on plain click. } // Reset the updating flag mUpdatingSelection = false; From 8e6fbe6019f5bf26f7d18a613e4aae95a7cfda0a Mon Sep 17 00:00:00 2001 From: vicquick Date: Sat, 25 Apr 2026 20:07:04 +0200 Subject: [PATCH 07/14] fix: move QgsLayoutViewToolSelect::isolatedGroup() out-of-line The inline accessor returned QgsLayoutItemGroup* from a QPointer member, which requires the complete QgsLayoutItemGroup type at every translation unit that includes the header. The header only forward-declares the class to keep its include surface minimal, so a translation unit that included the selection tool header without also pulling in qgslayoutitemgroup.h failed to compile (qgslayoutdesignerdialog.cpp was the first such unit hit during a clean build). Move the body to the .cpp so the QPointer-to-pointer conversion is instantiated where the full type is already available. Assisted-by: Claude Opus 4.7 --- src/gui/layout/qgslayoutviewtoolselect.cpp | 5 +++++ src/gui/layout/qgslayoutviewtoolselect.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/gui/layout/qgslayoutviewtoolselect.cpp b/src/gui/layout/qgslayoutviewtoolselect.cpp index 5c056b7648e3..721d022974ca 100644 --- a/src/gui/layout/qgslayoutviewtoolselect.cpp +++ b/src/gui/layout/qgslayoutviewtoolselect.cpp @@ -339,6 +339,11 @@ void QgsLayoutViewToolSelect::keyPressEvent( QKeyEvent *event ) } } +QgsLayoutItemGroup *QgsLayoutViewToolSelect::isolatedGroup() const +{ + return mIsolatedGroup; +} + void QgsLayoutViewToolSelect::layoutDoubleClickEvent( QgsLayoutViewMouseEvent *event ) { if ( event->button() != Qt::LeftButton ) diff --git a/src/gui/layout/qgslayoutviewtoolselect.h b/src/gui/layout/qgslayoutviewtoolselect.h index e32819ac1bef..67e775135d2f 100644 --- a/src/gui/layout/qgslayoutviewtoolselect.h +++ b/src/gui/layout/qgslayoutviewtoolselect.h @@ -57,7 +57,7 @@ class GUI_EXPORT QgsLayoutViewToolSelect : public QgsLayoutViewTool * Returns the currently isolated group, or NULLPTR when no group is * being isolated. */ - QgsLayoutItemGroup *isolatedGroup() const { return mIsolatedGroup; } + QgsLayoutItemGroup *isolatedGroup() const; /** * Enters isolation mode for \a group. While isolated, items outside From 5a15501267d17f7f6fafab4bf4c3b4183ede0e8e Mon Sep 17 00:00:00 2001 From: vicquick Date: Sun, 26 Apr 2026 09:52:48 +0200 Subject: [PATCH 08/14] fix: keep items panel hierarchy in sync after group / ungroup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-up issues from the first round of nested-grouping testing in the layout designer: 1. Visibility column overflow under nesting QTreeView puts the disclosure arrow + indent in column 0 by default, which is the visibility checkbox column. The fixed-width visibility column was getting eaten by the indent on nested rows. Switch tree position to the name column so vis / lock stay flush left at their fixed widths regardless of depth. 2. Last grouped member missing + ungroup wipes other rows QgsLayout::groupItems and ungroupItems mutate parentGroup() on members directly without telling QAbstractItemModel about the parent change. With the items model now hierarchical via parent() / index() / rowCount(), these mutations leave the tree reading stale state — a member appears under top-level when it should be under the new group, or vice versa. Easiest correct fix: emit a modelReset around the operation. Added QgsLayoutModel::emitModelReset() (a tiny public wrapper around the protected begin/endResetModel) and call it from groupItems and ungroupItems after the structural change. 3. New groups stay collapsed After modelReset the tree forgets expansion state. Wire expandAll() to modelReset so every group is visible by default — mirrors layer tree / browser tree behavior elsewhere in QGIS. Assisted-by: Claude Opus 4.7 --- src/core/layout/qgslayout.cpp | 11 +++++++++++ src/core/layout/qgslayoutmodel.cpp | 6 ++++++ src/core/layout/qgslayoutmodel.h | 8 ++++++++ src/gui/layout/qgslayoutitemslistview.cpp | 13 +++++++++++++ 4 files changed, 38 insertions(+) diff --git a/src/core/layout/qgslayout.cpp b/src/core/layout/qgslayout.cpp index 1bbf8b764a30..4d787ffb566a 100644 --- a/src/core/layout/qgslayout.cpp +++ b/src/core/layout/qgslayout.cpp @@ -803,6 +803,12 @@ QgsLayoutItemGroup *QgsLayout::groupItems( const QList &items ) mUndoStack->endMacro(); + // The members' parentGroup() just flipped from null to returnGroup; + // QAbstractItemModel never saw this so the tree's parent/rowCount + // would be stale until next refresh. Force a reset so members appear + // nested under the new group. + mItemsModel->emitModelReset(); + // cppcheck-suppress returnDanglingLifetime return returnGroup; } @@ -829,6 +835,11 @@ QList QgsLayout::ungroupItems( QgsLayoutItemGroup *group ) removeLayoutItem( group ); mUndoStack->endMacro(); + // Same staleness as groupItems(): the members' parentGroup() just + // flipped from group to null. Force a reset so they reappear at top + // level in the items panel. + mItemsModel->emitModelReset(); + return ungroupedItems; } diff --git a/src/core/layout/qgslayoutmodel.cpp b/src/core/layout/qgslayoutmodel.cpp index a8c95aa6e412..ed6a3c1acd2a 100644 --- a/src/core/layout/qgslayoutmodel.cpp +++ b/src/core/layout/qgslayoutmodel.cpp @@ -54,6 +54,12 @@ QgsLayoutItem *QgsLayoutModel::itemFromIndex( const QModelIndex &index ) const return static_cast( index.internalPointer() ); } +void QgsLayoutModel::emitModelReset() +{ + beginResetModel(); + endResetModel(); +} + QList QgsLayoutModel::topLevelItemsInScene() const { QList result; diff --git a/src/core/layout/qgslayoutmodel.h b/src/core/layout/qgslayoutmodel.h index 27ce70973998..7f672e3b4e22 100644 --- a/src/core/layout/qgslayoutmodel.h +++ b/src/core/layout/qgslayoutmodel.h @@ -289,6 +289,14 @@ class CORE_EXPORT QgsLayoutModel : public QAbstractItemModel */ void rebuildSceneItemList(); + /** + * Re-emits modelReset around an externally-driven structural change + * (e.g. grouping or ungrouping) so the tree picks up new + * parentGroup() relationships. Cheap fallback for operations that + * would otherwise need granular beginMoveRows / endMoveRows. + */ + void emitModelReset(); + /** * Returns items from mItemsInScene that have no parent group. * Order preserves the existing global z-order slice. diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index 26a9a6210fa2..d80a19e28365 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -100,6 +100,10 @@ QgsLayoutItemsListView::QgsLayoutItemsListView( QWidget *parent, QgsLayoutDesign setIndentation( 16 ); setRootIsDecorated( true ); setAnimated( true ); + // The disclosure arrow + indentation should live in the name column, + // NOT in the visibility column (column 0). Without this, nested rows + // push the visibility checkbox out of its narrow fixed-width column. + setTreePosition( QgsLayoutModel::ItemId ); // Allow multi selection from the list view setSelectionMode( QAbstractItemView::ExtendedSelection ); @@ -125,6 +129,15 @@ void QgsLayoutItemsListView::setCurrentLayout( QgsLayout *layout ) // a source model row insertion (e.g. when grouping items), which // would emit dataChanged mid-transaction and corrupt the proxy. connect( selectionModel(), &QItemSelectionModel::selectionChanged, this, &QgsLayoutItemsListView::updateSelection, Qt::QueuedConnection ); + + // After group / ungroup the source model resets; expand all groups so + // freshly-nested children are visible right away (mirrors every other + // tree-based panel in QGIS where groups are visible by default). + connect( mModel, &QAbstractItemModel::modelReset, this, [this]() + { + expandAll(); + } ); + expandAll(); } void QgsLayoutItemsListView::keyPressEvent( QKeyEvent *event ) From b01d2830e78f17e40fe3d01f6d7c9d75295dc00a Mon Sep 17 00:00:00 2001 From: vicquick Date: Sun, 26 Apr 2026 10:05:43 +0200 Subject: [PATCH 09/14] fix: dynamically widen visibility column when nesting is present Replace setTreePosition() (which moved the disclosure arrow into the name column) with a depth-aware width adjustment on the visibility column itself. The disclosure arrow now lives in the visibility column where it belongs visually (the Adobe-like nested look the user actually wants), and the column auto-grows by one indentation step per nesting level so the checkbox always has clear space. Recomputed on every modelReset (i.e. every group/ungroup) via adjustVisibilityColumnWidth(); base width when no groups exist is the original 4 'x' character widths, unchanged from upstream. Assisted-by: Claude Opus 4.7 --- src/gui/layout/qgslayoutitemslistview.cpp | 44 +++++++++++++++++++---- src/gui/layout/qgslayoutitemslistview.h | 5 +++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index d80a19e28365..e974cfd96553 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -100,10 +100,6 @@ QgsLayoutItemsListView::QgsLayoutItemsListView( QWidget *parent, QgsLayoutDesign setIndentation( 16 ); setRootIsDecorated( true ); setAnimated( true ); - // The disclosure arrow + indentation should live in the name column, - // NOT in the visibility column (column 0). Without this, nested rows - // push the visibility checkbox out of its narrow fixed-width column. - setTreePosition( QgsLayoutModel::ItemId ); // Allow multi selection from the list view setSelectionMode( QAbstractItemView::ExtendedSelection ); @@ -121,9 +117,9 @@ void QgsLayoutItemsListView::setCurrentLayout( QgsLayout *layout ) header()->setSectionResizeMode( 0, QHeaderView::Fixed ); header()->setSectionResizeMode( 1, QHeaderView::Fixed ); - setColumnWidth( 0, Qgis::UI_SCALE_FACTOR * fontMetrics().horizontalAdvance( 'x' ) * 4 ); setColumnWidth( 1, Qgis::UI_SCALE_FACTOR * fontMetrics().horizontalAdvance( 'x' ) * 4 ); header()->setSectionsMovable( false ); + adjustVisibilityColumnWidth(); // Queued so updateSelection() does not run synchronously from inside // a source model row insertion (e.g. when grouping items), which @@ -131,15 +127,49 @@ void QgsLayoutItemsListView::setCurrentLayout( QgsLayout *layout ) connect( selectionModel(), &QItemSelectionModel::selectionChanged, this, &QgsLayoutItemsListView::updateSelection, Qt::QueuedConnection ); // After group / ungroup the source model resets; expand all groups so - // freshly-nested children are visible right away (mirrors every other - // tree-based panel in QGIS where groups are visible by default). + // freshly-nested children are visible right away, and re-fit the + // visibility column so the indented checkboxes always have room. connect( mModel, &QAbstractItemModel::modelReset, this, [this]() { expandAll(); + adjustVisibilityColumnWidth(); } ); expandAll(); } +int QgsLayoutItemsListView::computeMaxNestingDepth() const +{ + if ( !mModel ) + return 0; + + int deepest = 0; + std::function walk = + [&]( const QModelIndex &parent, int depth ) + { + const int rows = mModel->rowCount( parent ); + if ( rows == 0 ) + return; + if ( depth > deepest ) + deepest = depth; + for ( int r = 0; r < rows; ++r ) + { + walk( mModel->index( r, 0, parent ), depth + 1 ); + } + }; + walk( QModelIndex(), 0 ); + return deepest; +} + +void QgsLayoutItemsListView::adjustVisibilityColumnWidth() +{ + const int base = Qgis::UI_SCALE_FACTOR * fontMetrics().horizontalAdvance( 'x' ) * 4; + const int depth = computeMaxNestingDepth(); + // QTreeView puts the disclosure arrow + per-level indentation in the + // first column. Nested rows need extra room or the visibility checkbox + // gets pushed out of view. Add one indentation step per nesting level. + setColumnWidth( 0, base + depth * indentation() ); +} + void QgsLayoutItemsListView::keyPressEvent( QKeyEvent *event ) { if ( event->key() == Qt::Key_Space ) diff --git a/src/gui/layout/qgslayoutitemslistview.h b/src/gui/layout/qgslayoutitemslistview.h index 8539f468d2e7..a5b5790bb3ea 100644 --- a/src/gui/layout/qgslayoutitemslistview.h +++ b/src/gui/layout/qgslayoutitemslistview.h @@ -103,6 +103,11 @@ class GUI_EXPORT QgsLayoutItemsListView : public QTreeView bool mUpdatingSelection = false; bool mUpdatingFromView = false; + + //! Walks the model and returns the deepest nesting level (0 = no groups). + int computeMaxNestingDepth() const; + //! Recomputes the visibility column width based on current nesting depth. + void adjustVisibilityColumnWidth(); }; #endif // QGSLAYOUTITEMSLISTVIEW_H From 66f023cce60c2036de9c8a3dabca23b320cd0d75 Mon Sep 17 00:00:00 2001 From: vicquick Date: Sun, 26 Apr 2026 10:06:00 +0200 Subject: [PATCH 10/14] fix: add missing include for std::function lambda walker Assisted-by: Claude Opus 4.7 --- src/gui/layout/qgslayoutitemslistview.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index e974cfd96553..9556bc908f77 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -25,6 +25,8 @@ #include #include +#include + #include "moc_qgslayoutitemslistview.cpp" QgsLayoutItemsListViewModel::QgsLayoutItemsListViewModel( QgsLayoutModel *model, QObject *parent ) From 1a3cdf8a40ac7360ca3db7ba9bfaa69c36d66c98 Mon Sep 17 00:00:00 2001 From: vicquick Date: Sun, 26 Apr 2026 10:29:10 +0200 Subject: [PATCH 11/14] fix: make QgsLayoutModel::emitModelReset() public MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was placed next to the other hierarchy helpers in the private section, but unlike them it is called from QgsLayout::groupItems and ungroupItems which sit outside the class — the build failure on qgslayout.cpp was the access check. The other helpers stay private because they are only invoked from QgsLayoutModel members. Assisted-by: Claude Opus 4.7 --- src/core/layout/qgslayoutmodel.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/layout/qgslayoutmodel.h b/src/core/layout/qgslayoutmodel.h index 7f672e3b4e22..b07e563841a2 100644 --- a/src/core/layout/qgslayoutmodel.h +++ b/src/core/layout/qgslayoutmodel.h @@ -249,6 +249,14 @@ class CORE_EXPORT QgsLayoutModel : public QAbstractItemModel */ QModelIndex indexForItem( QgsLayoutItem *item, int column = 0 ); + /** + * Re-emits modelReset around an externally-driven structural change + * (e.g. grouping or ungrouping) so the tree picks up new + * parentGroup() relationships. Cheap fallback for operations that + * would otherwise need granular beginMoveRows / endMoveRows. + */ + void emitModelReset(); + public slots: ///@cond PRIVATE @@ -289,14 +297,6 @@ class CORE_EXPORT QgsLayoutModel : public QAbstractItemModel */ void rebuildSceneItemList(); - /** - * Re-emits modelReset around an externally-driven structural change - * (e.g. grouping or ungrouping) so the tree picks up new - * parentGroup() relationships. Cheap fallback for operations that - * would otherwise need granular beginMoveRows / endMoveRows. - */ - void emitModelReset(); - /** * Returns items from mItemsInScene that have no parent group. * Order preserves the existing global z-order slice. From 7168f5b2ff65bea91539c1ab20f1903d65d60154 Mon Sep 17 00:00:00 2001 From: vicquick Date: Sun, 26 Apr 2026 14:30:06 +0200 Subject: [PATCH 12/14] fix: stop filtering out the topmost child of every group from the panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QgsLayoutItemsListViewModel::filterAcceptsRow used to drop sourceRow 0 unconditionally to hide the source model's top-level paper / null sentinel. With the source model now hierarchical, sourceRow 0 of a group parent is a real item — the topmost member — and was being silently dropped from the panel. Made the check parent-aware so only the root sentinel is hidden. Also widen the visibility column to base * 2 + depth * indentation when any group exists, so the disclosure arrow plus indented checkbox have actual room at deeper nesting levels. Assisted-by: Claude Opus 4.7 --- src/gui/layout/qgslayoutitemslistview.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index 9556bc908f77..8339df61e13f 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -51,10 +51,14 @@ void QgsLayoutItemsListViewModel::setSelected( const QModelIndex &index ) mModel->setSelected( mapToSource( index ) ); } -bool QgsLayoutItemsListViewModel::filterAcceptsRow( int sourceRow, const QModelIndex & ) const +bool QgsLayoutItemsListViewModel::filterAcceptsRow( int sourceRow, const QModelIndex &sourceParent ) const { - if ( sourceRow == 0 ) - return false; // hide empty null item row + // Only the root sentinel (top-level row 0, which is the paper / null item) + // should be hidden. Group children at local row 0 are real items and + // must NOT be filtered, otherwise the topmost member of every group + // disappears from the items panel after grouping. + if ( sourceRow == 0 && !sourceParent.isValid() ) + return false; return true; } @@ -167,9 +171,13 @@ void QgsLayoutItemsListView::adjustVisibilityColumnWidth() const int base = Qgis::UI_SCALE_FACTOR * fontMetrics().horizontalAdvance( 'x' ) * 4; const int depth = computeMaxNestingDepth(); // QTreeView puts the disclosure arrow + per-level indentation in the - // first column. Nested rows need extra room or the visibility checkbox - // gets pushed out of view. Add one indentation step per nesting level. - setColumnWidth( 0, base + depth * indentation() ); + // first column. When nesting is present the column needs noticeably + // more room than the bare checkbox width — double the base AND add + // one indentation step per level so deeper nesting still fits. + const int width = depth > 0 + ? base * 2 + depth * indentation() + : base; + setColumnWidth( 0, width ); } void QgsLayoutItemsListView::keyPressEvent( QKeyEvent *event ) From b956ac90e9ba006cbd84e7061b239d3ee275fb28 Mon Sep 17 00:00:00 2001 From: vicquick Date: Mon, 27 Apr 2026 13:51:20 +0200 Subject: [PATCH 13/14] chore: add LayoutPerf timing logs for grouping hot paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires QgsMessageLog::logMessage("LayoutPerf", Info) on: - QgsLayout::groupItems / ungroupItems — total wall time, plus the split between mUndoStack work and the post-op emitModelReset. - QgsLayoutModel::emitModelReset — wall time and pre-reset top-level + scene counts so we can see how big the model is. - QgsLayoutItemsListView modelReset slot — expandAll vs total panel work after a reset. - QgsLayoutItemsListView::adjustVisibilityColumnWidth — depth walk vs total column resize. User can filter the QGIS Message Log panel to "LayoutPerf" to see numbers per group/ungroup operation. No behavior change. Assisted-by: Claude Opus 4.7 --- src/core/layout/qgslayout.cpp | 21 +++++++++++++++++++++ src/core/layout/qgslayoutmodel.cpp | 9 +++++++++ src/gui/layout/qgslayoutitemslistview.cpp | 15 +++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/src/core/layout/qgslayout.cpp b/src/core/layout/qgslayout.cpp index 4d787ffb566a..0e0ce014cf0d 100644 --- a/src/core/layout/qgslayout.cpp +++ b/src/core/layout/qgslayout.cpp @@ -778,6 +778,8 @@ QgsLayoutItemGroup *QgsLayout::groupItems( const QList &items ) return nullptr; } + QElapsedTimer perfTimer; perfTimer.start(); + mUndoStack->beginMacro( tr( "Group Items" ) ); auto itemGroup = std::make_unique( this ); @@ -802,6 +804,7 @@ QgsLayoutItemGroup *QgsLayout::groupItems( const QList &items ) mProject->setDirty( true ); mUndoStack->endMacro(); + const qint64 beforeReset = perfTimer.elapsed(); // The members' parentGroup() just flipped from null to returnGroup; // QAbstractItemModel never saw this so the tree's parent/rowCount @@ -809,6 +812,13 @@ QgsLayoutItemGroup *QgsLayout::groupItems( const QList &items ) // nested under the new group. mItemsModel->emitModelReset(); + QgsMessageLog::logMessage( + QStringLiteral( "groupItems: count=%1 stack-and-reset=%2ms reset-only=%3ms total=%4ms" ) + .arg( items.size() ).arg( beforeReset ) + .arg( perfTimer.elapsed() - beforeReset ) + .arg( perfTimer.elapsed() ), + QStringLiteral( "LayoutPerf" ), Qgis::MessageLevel::Info ); + // cppcheck-suppress returnDanglingLifetime return returnGroup; } @@ -821,6 +831,9 @@ QList QgsLayout::ungroupItems( QgsLayoutItemGroup *group ) return ungroupedItems; } + QElapsedTimer perfTimer; perfTimer.start(); + const int childCount = group->items().size(); + mUndoStack->beginMacro( tr( "Ungroup Items" ) ); // Call this before removing group items so it can keep note // of contents @@ -835,11 +848,19 @@ QList QgsLayout::ungroupItems( QgsLayoutItemGroup *group ) removeLayoutItem( group ); mUndoStack->endMacro(); + const qint64 beforeReset = perfTimer.elapsed(); // Same staleness as groupItems(): the members' parentGroup() just // flipped from group to null. Force a reset so they reappear at top // level in the items panel. mItemsModel->emitModelReset(); + QgsMessageLog::logMessage( + QStringLiteral( "ungroupItems: count=%1 unstack-and-reset=%2ms reset-only=%3ms total=%4ms" ) + .arg( childCount ).arg( beforeReset ) + .arg( perfTimer.elapsed() - beforeReset ) + .arg( perfTimer.elapsed() ), + QStringLiteral( "LayoutPerf" ), Qgis::MessageLevel::Info ); + return ungroupedItems; } diff --git a/src/core/layout/qgslayoutmodel.cpp b/src/core/layout/qgslayoutmodel.cpp index ed6a3c1acd2a..7f2b9b64364b 100644 --- a/src/core/layout/qgslayoutmodel.cpp +++ b/src/core/layout/qgslayoutmodel.cpp @@ -21,10 +21,12 @@ #include "qgslayout.h" #include "qgslayoutitemgroup.h" #include "qgslogger.h" +#include "qgsmessagelog.h" #include #include #include +#include #include #include #include @@ -56,8 +58,15 @@ QgsLayoutItem *QgsLayoutModel::itemFromIndex( const QModelIndex &index ) const void QgsLayoutModel::emitModelReset() { + QElapsedTimer t; t.start(); + const int top = topLevelItemsInScene().size(); + const int scene = mItemsInScene.size(); beginResetModel(); endResetModel(); + QgsMessageLog::logMessage( + QStringLiteral( "emitModelReset: top=%1 scene=%2 elapsed=%3ms" ) + .arg( top ).arg( scene ).arg( t.elapsed() ), + QStringLiteral( "LayoutPerf" ), Qgis::MessageLevel::Info ); } QList QgsLayoutModel::topLevelItemsInScene() const diff --git a/src/gui/layout/qgslayoutitemslistview.cpp b/src/gui/layout/qgslayoutitemslistview.cpp index 8339df61e13f..04ecdcb6b624 100644 --- a/src/gui/layout/qgslayoutitemslistview.cpp +++ b/src/gui/layout/qgslayoutitemslistview.cpp @@ -20,7 +20,9 @@ #include "qgslayoutitemgroup.h" #include "qgslayoutmodel.h" #include "qgslayoutview.h" +#include "qgsmessagelog.h" +#include #include #include #include @@ -137,8 +139,15 @@ void QgsLayoutItemsListView::setCurrentLayout( QgsLayout *layout ) // visibility column so the indented checkboxes always have room. connect( mModel, &QAbstractItemModel::modelReset, this, [this]() { + QElapsedTimer t; t.start(); expandAll(); + const qint64 expandMs = t.elapsed(); adjustVisibilityColumnWidth(); + const qint64 totalMs = t.elapsed(); + QgsMessageLog::logMessage( + QStringLiteral( "panel.modelReset: expandAll=%1ms total=%2ms" ) + .arg( expandMs ).arg( totalMs ), + QStringLiteral( "LayoutPerf" ), Qgis::MessageLevel::Info ); } ); expandAll(); } @@ -168,8 +177,10 @@ int QgsLayoutItemsListView::computeMaxNestingDepth() const void QgsLayoutItemsListView::adjustVisibilityColumnWidth() { + QElapsedTimer t; t.start(); const int base = Qgis::UI_SCALE_FACTOR * fontMetrics().horizontalAdvance( 'x' ) * 4; const int depth = computeMaxNestingDepth(); + const qint64 depthMs = t.elapsed(); // QTreeView puts the disclosure arrow + per-level indentation in the // first column. When nesting is present the column needs noticeably // more room than the bare checkbox width — double the base AND add @@ -178,6 +189,10 @@ void QgsLayoutItemsListView::adjustVisibilityColumnWidth() ? base * 2 + depth * indentation() : base; setColumnWidth( 0, width ); + QgsMessageLog::logMessage( + QStringLiteral( "panel.adjustVisColumn: depth=%1 width=%2 depth-walk=%3ms total=%4ms" ) + .arg( depth ).arg( width ).arg( depthMs ).arg( t.elapsed() ), + QStringLiteral( "LayoutPerf" ), Qgis::MessageLevel::Info ); } void QgsLayoutItemsListView::keyPressEvent( QKeyEvent *event ) From 4bc45bc9d4487e87e7e6153185d7ea4762cb8688 Mon Sep 17 00:00:00 2001 From: vicquick Date: Mon, 27 Apr 2026 14:15:04 +0200 Subject: [PATCH 14/14] fix: add missing QgsMessageLog + QElapsedTimer includes to qgslayout.cpp LayoutPerf logging used both without their headers; build failed on qgslayout.cpp.o. Assisted-by: Claude Opus 4.7 --- src/core/layout/qgslayout.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/layout/qgslayout.cpp b/src/core/layout/qgslayout.cpp index 0e0ce014cf0d..618c35cd7cd5 100644 --- a/src/core/layout/qgslayout.cpp +++ b/src/core/layout/qgslayout.cpp @@ -35,7 +35,10 @@ #include "qgslayoutrendercontext.h" #include "qgslayoutreportcontext.h" #include "qgslayoutundostack.h" +#include "qgsmessagelog.h" #include "qgsproject.h" + +#include #include "qgsreadwritecontext.h" #include "qgsruntimeprofiler.h" #include "qgssettingsentryimpl.h"