Bug report #11905
QgsLayerTreeModel: presence of embedded legend breaks ModelTest
Status: | Closed | ||
---|---|---|---|
Priority: | Normal | ||
Assignee: | Martin Dobias | ||
Category: | Map Legend | ||
Affected QGIS version: | 2.6.0 | Regression?: | No |
Operating System: | Easy fix?: | No | |
Pull Request or Patch supplied: | No | Resolution: | fixed/implemented |
Crashes QGIS or corrupts data: | No | Copied to github as #: | 20115 |
Description
A single project with an embedded legend fails an assertion in ModelTester.
Ho to reproduce:
1. Build with -DENABLE_MODELTEST=ON
2. Add a vector layer
The default legend for vector layer seems to be embedded, so that QgsLayerTreeModel::rowCount() for it returns 0.
But when the legend item was added, the addition triggered a beginInsertRows was called promising a new row would have been added instead.
The ModelTester catches that discrepancy and crashes.
I don't know which way things should go (counting in embedded legends or skipping them elsewhere). Also I don't see if it could be possible to have multiple embedded legends in a layer...
History
#1 Updated by Sandro Santilli almost 10 years ago
Whaht does it mean exactly for a legend node to be embedded in parent's node ? It's not documented here:
https://github.com/qgis/QGIS/blob/master/src/core/layertree/qgslayertreemodellegendnode.h#L63-L64
#2 Updated by Martin Dobias almost 10 years ago
Legend node embedded in parent means that the legend node will be not displayed in the view as a separate item under the layer's item, rather the legend node's icon should be used for the layer item. This is used e.g. with single symbol renderer - mainly to conserve vertical space in the view. The embedding should happen if there is only one legend node generated and that flag is enabled. That means you can have cases where there is just one legend node but it is intentionally not embedded.
It seems like beginInsertRows should not be called at all in case of embedded legend node.
#3 Updated by Sandro Santilli almost 10 years ago
The following patch fixes the crash indeed, but I see beginInsertRows being also called from other places
diff --git a/src/core/layertree/qgslayertreemodel.cpp b/src/core/layertree/qgslayertreemodel.cpp index de62624..bcfddc3 100644 --- a/src/core/layertree/qgslayertreemodel.cpp +++ b/src/core/layertree/qgslayertreemodel.cpp @@ -800,7 +800,9 @@ void QgsLayerTreeModel::addLegendToLayer( QgsLayerTreeLayer* nodeL ) QList<QgsLayerTreeModelLegendNode*> filteredLstNew = filterLegendNodes( lstNew ); - beginInsertRows( node2index( nodeL ), 0, filteredLstNew.count() - 1 ); + bool isEmbedded = filteredLstNew.count() == 1 && filteredLstNew[0]->isEmbeddedInParent(); + + if ( ! isEmbedded ) beginInsertRows( node2index( nodeL ), 0, filteredLstNew.count() - 1 ); foreach ( QgsLayerTreeModelLegendNode* n, lstNew ) { @@ -811,7 +813,7 @@ void QgsLayerTreeModel::addLegendToLayer( QgsLayerTreeLayer* nodeL ) mOriginalLegendNodes[nodeL] = lstNew; mLegendNodes[nodeL] = filteredLstNew; - endInsertRows(); + if ( ! isEmbedded ) endInsertRows(); }
#4 Updated by Martin Dobias almost 10 years ago
The patch looks good from a quick look. The other beginInsertRows() call handles addition of layer tree nodes, not legend nodes.
#5 Updated by Sandro Santilli almost 10 years ago
- Target version set to Version 2.8
- Resolution set to fixed/implemented
Thanks for review, patch pushed as 32079ed3cf7cf851515c63ed01cb994169f45963 -- not sure if this is worth backporting to 2.6 branch, if anyone feels strong about it please state so here and repoen for the purpose.
#6 Updated by Martin Dobias almost 10 years ago
- Status changed from Open to Closed