Fix display of diagram legend entries (fixes #15448)

To make the implementation saner, the legend node that may be embedded within parent
layer node is kept separately from activeNodes.

(cherry picked from commit b385ebd9ba9272516eed61e5825a603fcee969e9)

python/core/layertree/qgslayertreemodel.sip

@@ -92,15 +92,21 @@ class QgsLayerTreeModel : QAbstractItemModel
     QModelIndex legendNode2index( QgsLayerTreeModelLegendNode* legendNode );
 
     //! Return filtered list of active legend nodes attached to a particular layer node
+    //! (by default it returns also legend node embedded in parent layer node (if any) unless skipNodeEmbeddedInParent is true)
     //! @note added in 2.6
+    //! @note skipNodeEmbeddedInParent added in 2.18
     //! @see layerOriginalLegendNodes()
-    QList<QgsLayerTreeModelLegendNode*> layerLegendNodes( QgsLayerTreeLayer* nodeLayer );
+    QList<QgsLayerTreeModelLegendNode*> layerLegendNodes( QgsLayerTreeLayer* nodeLayer, bool skipNodeEmbeddedInParent = false );
 
     //! Return original (unfiltered) list of legend nodes attached to a particular layer node
     //! @note added in 2.14
     //! @see layerLegendNodes()
     QList<QgsLayerTreeModelLegendNode*> layerOriginalLegendNodes( QgsLayerTreeLayer* nodeLayer );
 
+    //! Return legend node that may be embbeded in parent (i.e. its icon will be used for layer's icon).
+    //! @note added in 2.18
+    QgsLayerTreeModelLegendNode* legendNodeEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
+
     /** Searches through the layer tree to find a legend node with a matching layer ID
      * and rule key.
      * @param layerId map layer ID
@@ -233,8 +239,6 @@ class QgsLayerTreeModel : QAbstractItemModel
     QVariant legendNodeData( QgsLayerTreeModelLegendNode* node, int role ) const;
     Qt::ItemFlags legendNodeFlags( QgsLayerTreeModelLegendNode* node ) const;
     bool legendEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
-    /** Return legend node that may be embbeded in parent (i.e. its icon will be used for layer's icon). */
-    QgsLayerTreeModelLegendNode* legendNodeEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
     QIcon legendIconEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
     void legendCleanup();
     void legendInvalidateMapBasedData();

src/app/composer/qgscomposerlegendwidget.cpp

@@ -1006,9 +1006,8 @@ void QgsComposerLegendWidget::on_mItemTreeView_doubleClicked( const QModelIndex
     currentNode->setCustomProperty( QStringLiteral( "legend/title-label" ), newText );
 
     // force update of label of the legend node with embedded icon (a bit clumsy i know)
-    QList<QgsLayerTreeModelLegendNode*> nodes = model->layerLegendNodes( QgsLayerTree::toLayer( currentNode ) );
-    if ( nodes.count() == 1 && nodes[0]->isEmbeddedInParent() )
-      nodes[0]->setUserLabel( QString() );
+    if ( QgsLayerTreeModelLegendNode* embeddedNode = model->legendNodeEmbeddedInParent( QgsLayerTree::toLayer( currentNode ) ) )
+      embeddedNode->setUserLabel( QString() );
   }
   else if ( legendNode )
   {

src/app/qgsmapthemes.cpp

@@ -68,7 +68,7 @@ void QgsMapThemes::addPerLayerCheckedLegendSymbols( QgsMapThemeCollection::MapTh
     bool hasCheckableItems = false;
     bool someItemsUnchecked = false;
     QSet<QString> checkedItems;
-    Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer ) )
+    Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer, true ) )
     {
       if ( legendNode->flags() & Qt::ItemIsUserCheckable )
       {
@@ -217,7 +217,7 @@ void QgsMapThemes::applyStateToLayerTreeGroup( QgsLayerTreeGroup* parent, const
         {
           const QSet<QString>& checkedNodes = rec.perLayerCheckedLegendSymbols().value( nodeLayer->layerId() );
           // some nodes are not checked
-          Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer ) )
+          Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer, true ) )
           {
             Qt::CheckState shouldHaveState = checkedNodes.contains( legendNode->data( QgsLayerTreeModelLegendNode::RuleKeyRole ).toString() ) ? Qt::Checked : Qt::Unchecked;
             if (( legendNode->flags() & Qt::ItemIsUserCheckable ) &&
@@ -228,7 +228,7 @@ void QgsMapThemes::applyStateToLayerTreeGroup( QgsLayerTreeGroup* parent, const
         else
         {
           // all nodes should be checked
-          Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer ) )
+          Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer, true ) )
           {
             if (( legendNode->flags() & Qt::ItemIsUserCheckable ) &&
                 legendNode->data( Qt::CheckStateRole ).toInt() != Qt::Checked )

src/core/layertree/qgslayertreemodel.cpp

@@ -1196,30 +1196,45 @@ void QgsLayerTreeModel::addLegendToLayer( QgsLayerTreeLayer* nodeL )
 
   QList<QgsLayerTreeModelLegendNode*> filteredLstNew = filterLegendNodes( lstNew );
 
-  bool hasOnlyEmbedded = filteredLstNew.count() == 1 && filteredLstNew[0]->isEmbeddedInParent();
-
   Q_FOREACH ( QgsLayerTreeModelLegendNode* n, lstNew )
   {
     n->setParent( this );
     connect( n, SIGNAL( dataChanged() ), this, SLOT( legendNodeDataChanged() ) );
   }
 
-  LayerLegendData data;
-  data.originalNodes = lstNew;
-  data.activeNodes = filteredLstNew;
-  data.tree = nullptr;
+  // See if we have an embedded node - if we do, we will not use it among active nodes.
+  // Legend node embedded in parent does not have to be the first one,
+  // there can be also nodes generated for embedded widgets
+  QgsLayerTreeModelLegendNode* embeddedNode = nullptr;
+  Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, filteredLstNew )
+  {
+    if ( legendNode->isEmbeddedInParent() )
+    {
+      embeddedNode = legendNode;
+      filteredLstNew.removeOne( legendNode );
+      break;
+    }
+  }
+
+  LayerLegendTree* legendTree = nullptr;
 
   // maybe the legend nodes form a tree - try to create a tree structure from the list
   if ( testFlag( ShowLegendAsTree ) )
-    tryBuildLegendTree( data );
+    legendTree = tryBuildLegendTree( filteredLstNew );
 
-  int count = data.tree ? data.tree->children[nullptr].count() : filteredLstNew.count();
+  int count = legendTree ? legendTree->children[nullptr].count() : filteredLstNew.count();
 
-  if ( ! hasOnlyEmbedded ) beginInsertRows( node2index( nodeL ), 0, count - 1 );
+  if ( !filteredLstNew.isEmpty() ) beginInsertRows( node2index( nodeL ), 0, count - 1 );
+
+  LayerLegendData data;
+  data.originalNodes = lstNew;
+  data.activeNodes = filteredLstNew;
+  data.embeddedNodeInParent = embeddedNode;
+  data.tree = legendTree;
 
   mLegend[nodeL] = data;
 
-  if ( ! hasOnlyEmbedded ) endInsertRows();
+  if ( !filteredLstNew.isEmpty() ) endInsertRows();
 
   if ( hasStyleOverride )
     ml->styleManager()->restoreOverrideStyle();
@@ -1230,11 +1245,11 @@ void QgsLayerTreeModel::addLegendToLayer( QgsLayerTreeLayer* nodeL )
 }
 
 
-void QgsLayerTreeModel::tryBuildLegendTree( LayerLegendData& data )
+QgsLayerTreeModel::LayerLegendTree* QgsLayerTreeModel::tryBuildLegendTree( const QList<QgsLayerTreeModelLegendNode*>& nodes )
 {
   // first check whether there are any legend nodes that are not top-level
   bool hasParentKeys = false;
-  Q_FOREACH ( QgsLayerTreeModelLegendNode* n, data.activeNodes )
+  Q_FOREACH ( QgsLayerTreeModelLegendNode* n, nodes )
   {
     if ( !n->data( QgsLayerTreeModelLegendNode::ParentRuleKeyRole ).toString().isEmpty() )
     {
@@ -1243,30 +1258,31 @@ void QgsLayerTreeModel::tryBuildLegendTree( LayerLegendData& data )
     }
   }
   if ( !hasParentKeys )
-    return; // all legend nodes are top-level => stick with list representation
+    return nullptr; // all legend nodes are top-level => stick with list representation
 
   // make mapping from rules to nodes and do some sanity checks
   QHash<QString, QgsLayerTreeModelLegendNode*> rule2node;
   rule2node[QString()] = nullptr;
-  Q_FOREACH ( QgsLayerTreeModelLegendNode* n, data.activeNodes )
+  Q_FOREACH ( QgsLayerTreeModelLegendNode* n, nodes )
   {
     QString ruleKey = n->data( QgsLayerTreeModelLegendNode::RuleKeyRole ).toString();
     if ( ruleKey.isEmpty() ) // in tree all nodes must have key
-      return;
+      return nullptr;
     if ( rule2node.contains( ruleKey ) ) // and they must be unique
-      return;
+      return nullptr;
     rule2node[ruleKey] = n;
   }
 
   // create the tree structure
-  data.tree = new LayerLegendTree;
-  Q_FOREACH ( QgsLayerTreeModelLegendNode* n, data.activeNodes )
+  LayerLegendTree* tree = new LayerLegendTree;
+  Q_FOREACH ( QgsLayerTreeModelLegendNode* n, nodes )
   {
     QString parentRuleKey = n->data( QgsLayerTreeModelLegendNode::ParentRuleKeyRole ).toString();
     QgsLayerTreeModelLegendNode* parent = rule2node.value( parentRuleKey, nullptr );
-    data.tree->parents[n] = parent;
-    data.tree->children[parent] << n;
+    tree->parents[n] = parent;
+    tree->children[parent] << n;
   }
+  return tree;
 }
 
 QgsRenderContext* QgsLayerTreeModel::createTemporaryRenderContext() const
@@ -1316,6 +1332,7 @@ QModelIndex QgsLayerTreeModel::legendNode2index( QgsLayerTreeModelLegendNode* le
   int row = data.activeNodes.indexOf( legendNode );
   if ( row < 0 ) // legend node may be filtered (exists within the list of original nodes, but not in active nodes)
     return QModelIndex();
+
   return index( row, 0, parentIndex );
 }
 
@@ -1340,10 +1357,6 @@ int QgsLayerTreeModel::legendRootRowCount( QgsLayerTreeLayer* nL ) const
     return data.tree->children[nullptr].count();
 
   int count = data.activeNodes.count();
-
-  if ( legendEmbeddedInParent( nL ) )
-    count--;  // one item less -- it is embedded in parent
-
   return count;
 }
 
@@ -1408,35 +1421,34 @@ Qt::ItemFlags QgsLayerTreeModel::legendNodeFlags( QgsLayerTreeModelLegendNode* n
 
 bool QgsLayerTreeModel::legendEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const
 {
-  return legendNodeEmbeddedInParent( nodeLayer );
+  return mLegend[nodeLayer].embeddedNodeInParent != nullptr;
 }
 
 QgsLayerTreeModelLegendNode* QgsLayerTreeModel::legendNodeEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const
 {
-  // legend node embedded in parent does not have to be the first one...
-  // there could be extra legend nodes generated for embedded widgets
-  const LayerLegendData& data = mLegend[nodeLayer];
-  Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, data.activeNodes )
-  {
-    if ( legendNode->isEmbeddedInParent() )
-      return legendNode;
-  }
-  return nullptr;
+  return mLegend[nodeLayer].embeddedNodeInParent;
 }
 
 
 QIcon QgsLayerTreeModel::legendIconEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const
 {
-  QgsLayerTreeModelLegendNode* legendNode = legendNodeEmbeddedInParent( nodeLayer );
+  QgsLayerTreeModelLegendNode* legendNode = mLegend[nodeLayer].embeddedNodeInParent;
   if ( !legendNode )
     return QIcon();
   return QIcon( qvariant_cast<QPixmap>( legendNode->data( Qt::DecorationRole ) ) );
 }
 
 
-QList<QgsLayerTreeModelLegendNode*> QgsLayerTreeModel::layerLegendNodes( QgsLayerTreeLayer* nodeLayer )
+QList<QgsLayerTreeModelLegendNode*> QgsLayerTreeModel::layerLegendNodes( QgsLayerTreeLayer* nodeLayer, bool skipNodeEmbeddedInParent )
 {
-  return mLegend.value( nodeLayer ).activeNodes;
+  if ( !mLegend.contains( nodeLayer ) )
+    return QList<QgsLayerTreeModelLegendNode*>();
+
+  const LayerLegendData& data = mLegend[nodeLayer];
+  QList<QgsLayerTreeModelLegendNode*> lst( data.activeNodes );
+  if ( !skipNodeEmbeddedInParent && data.embeddedNodeInParent )
+    lst.prepend( data.embeddedNodeInParent );
+  return lst;
 }
 
 QList<QgsLayerTreeModelLegendNode*> QgsLayerTreeModel::layerOriginalLegendNodes( QgsLayerTreeLayer* nodeLayer )

src/core/layertree/qgslayertreemodel.h

@@ -118,15 +118,21 @@ class CORE_EXPORT QgsLayerTreeModel : public QAbstractItemModel
     QModelIndex legendNode2index( QgsLayerTreeModelLegendNode* legendNode );
 
     //! Return filtered list of active legend nodes attached to a particular layer node
+    //! (by default it returns also legend node embedded in parent layer node (if any) unless skipNodeEmbeddedInParent is true)
     //! @note added in 2.6
+    //! @note skipNodeEmbeddedInParent added in 2.18
     //! @see layerOriginalLegendNodes()
-    QList<QgsLayerTreeModelLegendNode*> layerLegendNodes( QgsLayerTreeLayer* nodeLayer );
+    QList<QgsLayerTreeModelLegendNode*> layerLegendNodes( QgsLayerTreeLayer* nodeLayer, bool skipNodeEmbeddedInParent = false );
 
     //! Return original (unfiltered) list of legend nodes attached to a particular layer node
     //! @note added in 2.14
     //! @see layerLegendNodes()
     QList<QgsLayerTreeModelLegendNode*> layerOriginalLegendNodes( QgsLayerTreeLayer* nodeLayer );
 
+    //! Return legend node that may be embbeded in parent (i.e. its icon will be used for layer's icon).
+    //! @note added in 2.18
+    QgsLayerTreeModelLegendNode* legendNodeEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
+
     /** Searches through the layer tree to find a legend node with a matching layer ID
      * and rule key.
      * @param layerId map layer ID
@@ -259,8 +265,6 @@ class CORE_EXPORT QgsLayerTreeModel : public QAbstractItemModel
     QVariant legendNodeData( QgsLayerTreeModelLegendNode* node, int role ) const;
     Qt::ItemFlags legendNodeFlags( QgsLayerTreeModelLegendNode* node ) const;
     bool legendEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
-    //! Return legend node that may be embbeded in parent (i.e. its icon will be used for layer's icon).
-    QgsLayerTreeModelLegendNode* legendNodeEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
     QIcon legendIconEmbeddedInParent( QgsLayerTreeLayer* nodeLayer ) const;
     void legendCleanup();
     void legendInvalidateMapBasedData();
@@ -293,9 +297,19 @@ class CORE_EXPORT QgsLayerTreeModel : public QAbstractItemModel
     //! @note not available in Python bindings
     struct LayerLegendData
     {
+      LayerLegendData()
+          : embeddedNodeInParent( nullptr )
+          , tree( nullptr )
+      {
+      }
+
       //! Active legend nodes. May have been filtered.
       //! Owner of legend nodes is still originalNodes !
       QList<QgsLayerTreeModelLegendNode*> activeNodes;
+      //! A legend node that is not displayed separately, its icon is instead
+      //! shown within the layer node's item.
+      //! May be null. if non-null, node is owned by originalNodes !
+      QgsLayerTreeModelLegendNode* embeddedNodeInParent;
       //! Data structure for storage of legend nodes.
       //! These are nodes as received from QgsMapLayerLegend
       QList<QgsLayerTreeModelLegendNode*> originalNodes;
@@ -304,7 +318,7 @@ class CORE_EXPORT QgsLayerTreeModel : public QAbstractItemModel
     };
 
     //! @note not available in Python bindings
-    void tryBuildLegendTree( LayerLegendData& data );
+    LayerLegendTree* tryBuildLegendTree( const QList<QgsLayerTreeModelLegendNode*>& nodes );
 
     //! Overrides of map layers' styles: key = layer ID, value = style XML.
     //! This allows to show legend that is different from the current style of layers

src/core/qgslegendrenderer.cpp

@@ -593,8 +593,7 @@ QgsComposerLegendStyle::Style QgsLegendRenderer::nodeLegendStyle( QgsLayerTreeNo
     return QgsComposerLegendStyle::Group;
   else if ( QgsLayerTree::isLayer( node ) )
   {
-    QList<QgsLayerTreeModelLegendNode*> legendNodes = model->layerLegendNodes( QgsLayerTree::toLayer( node ) );
-    if ( legendNodes.count() == 1 && legendNodes[0]->isEmbeddedInParent() )
+    if ( model->legendNodeEmbeddedInParent( QgsLayerTree::toLayer( node ) ) )
       return QgsComposerLegendStyle::Hidden;
     return QgsComposerLegendStyle::Subgroup;
   }

src/gui/layertree/qgslayertreeview.cpp

@@ -138,7 +138,7 @@ void QgsLayerTreeView::modelRowsInserted( const QModelIndex& index, int start, i
     if ( QgsMapLayer* layer = nodeLayer->layer() )
     {
       int widgetsCount = layer->customProperty( QStringLiteral( "embeddedWidgets/count" ), 0 ).toInt();
-      QList<QgsLayerTreeModelLegendNode*> legendNodes = layerTreeModel()->layerLegendNodes( nodeLayer );
+      QList<QgsLayerTreeModelLegendNode*> legendNodes = layerTreeModel()->layerLegendNodes( nodeLayer, true );
       for ( int i = 0; i < widgetsCount; ++i )
       {
         QString providerId = layer->customProperty( QStringLiteral( "embeddedWidgets/%1/id" ).arg( i ) ).toString();
@@ -159,7 +159,7 @@ void QgsLayerTreeView::modelRowsInserted( const QModelIndex& index, int start, i
     if ( expandedNodeKeys.isEmpty() )
       return;
 
-    Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, layerTreeModel()->layerLegendNodes( QgsLayerTree::toLayer( parentNode ) ) )
+    Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, layerTreeModel()->layerLegendNodes( QgsLayerTree::toLayer( parentNode ), true ) )
     {
       QString ruleKey = legendNode->data( QgsLayerTreeModelLegendNode::RuleKeyRole ).toString();
       if ( expandedNodeKeys.contains( ruleKey ) )
@@ -345,7 +345,7 @@ static void _expandAllLegendNodes( QgsLayerTreeLayer* nodeLayer, bool expanded,
   QStringList lst;
   if ( expanded )
   {
-    Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer ) )
+    Q_FOREACH ( QgsLayerTreeModelLegendNode* legendNode, model->layerLegendNodes( nodeLayer, true ) )
     {
       QString parentKey = legendNode->data( QgsLayerTreeModelLegendNode::ParentRuleKeyRole ).toString();
       if ( !parentKey.isEmpty() && !lst.contains( parentKey ) )