Fix layout items list selection update race causing crash and duplica… - #355
Closed
agiudiceandrea wants to merge 99 commits into
Closed
Fix layout items list selection update race causing crash and duplica…#355agiudiceandrea wants to merge 99 commits into
agiudiceandrea wants to merge 99 commits into
Conversation
Returns an expression which is the simplest direct equivalent of this expression Eg. an expression like ``1 + 2`` can be effectively replaced by the expression ``3``, or ``(1 + 2) * "a"`` by ``3 * a``.
* install built app * add to debian packaging * superceeds qgis#67064
…ompatible if draging from output to input
No longer used
This was previously done via private classes in app, but those are inaccessible to gui widgets or python plugins.
...instead of relying on iface methods. Removes another roadblock from porting this code to c++
Don't write default rotation properties to xml
As an alternative for app-only iface methods
🪟 Windows Qt6 buildsDownload Windows Qt6 builds of this PR for testing. |
vicquick
force-pushed
the
fix/61702-layout-items-list-queued-connection
branch
from
August 26, 2026 10:41
51a789b to
5082956
Compare
github-actions
Bot
added
GUI/UX
Processing
Expressions
3D
Annotations
Build/Install
Form
Mesh
Profile tool
Rasters
Server
labels
…te 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 qgis#61702 Assisted-by: Claude (Anthropic)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…te 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.
Assisted-by: Claude (Anthropic)
Description
[Replace this with some text explaining the rationale and details about this pull request]
AI tool usage