https://issues.qgis.org/https://issues.qgis.org/favicon.ico2014-12-22T06:39:14ZQGIS Issue TrackingQGIS Application - Bug report #11905: QgsLayerTreeModel: presence of embedded legend breaks ModelTesthttps://issues.qgis.org/issues/11905?journal_id=583232014-12-22T06:39:14ZSandro Santillistrk@kbt.io
<ul></ul><p>Whaht does it mean exactly for a legend node to be embedded in parent's node ? It's not documented here:<br /><a class="external" href="https://github.com/qgis/QGIS/blob/master/src/core/layertree/qgslayertreemodellegendnode.h#L63-L64">https://github.com/qgis/QGIS/blob/master/src/core/layertree/qgslayertreemodellegendnode.h#L63-L64</a></p> QGIS Application - Bug report #11905: QgsLayerTreeModel: presence of embedded legend breaks ModelTesthttps://issues.qgis.org/issues/11905?journal_id=583242014-12-22T06:51:12ZMartin Dobiaswonder.sk@gmail.com
<ul></ul><p>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.</p>
<p>It seems like beginInsertRows should not be called at all in case of embedded legend node.</p> QGIS Application - Bug report #11905: QgsLayerTreeModel: presence of embedded legend breaks ModelTesthttps://issues.qgis.org/issues/11905?journal_id=583262014-12-22T07:00:33ZSandro Santillistrk@kbt.io
<ul></ul><p>The following patch fixes the crash indeed, but I see beginInsertRows being also called from other places</p>
<pre>
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();
}
</pre> QGIS Application - Bug report #11905: QgsLayerTreeModel: presence of embedded legend breaks ModelTesthttps://issues.qgis.org/issues/11905?journal_id=583272014-12-22T07:12:41ZMartin Dobiaswonder.sk@gmail.com
<ul></ul><p>The patch looks good from a quick look. The other beginInsertRows() call handles addition of layer tree nodes, not legend nodes.</p> QGIS Application - Bug report #11905: QgsLayerTreeModel: presence of embedded legend breaks ModelTesthttps://issues.qgis.org/issues/11905?journal_id=583382014-12-23T00:42:01ZSandro Santillistrk@kbt.io
<ul><li><strong>Target version</strong> set to <i>Version 2.8</i></li><li><strong>Resolution</strong> set to <i>fixed/implemented</i></li></ul><p>Thanks for review, patch pushed as <a class="changeset" href="https://issues.qgis.org/projects/qgis/repository/revisions/32079ed3cf7cf851515c63ed01cb994169f45963" title="Fix LayerTreeModel use of beginInsertRows with embedded legends See http://hub.qgis.org/issues/1...">32079ed3cf7cf851515c63ed01cb994169f45963</a> -- 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.</p> QGIS Application - Bug report #11905: QgsLayerTreeModel: presence of embedded legend breaks ModelTesthttps://issues.qgis.org/issues/11905?journal_id=584382015-01-03T16:36:28ZMartin Dobiaswonder.sk@gmail.com
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li></ul>