Skip to content

Commit

Permalink
Fix display of diagram legend entries (fixes #15448)
Browse files Browse the repository at this point in the history 
To make the implementation saner, the legend node that may be embedded within parent
layer node is kept separately from activeNodes.
  • Loading branch information
@wonder-sk
wonder-sk committed Oct 20, 2016
1 parent 5ae0e78 commit b385ebd
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 55 deletions.
10 changes: 7 additions & 3 deletions python/core/layertree/qgslayertreemodel.sip
View file
Expand Up @@ -94,15 +94,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
Expand Down Expand Up @@ -252,8 +258,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();
Expand Down
5 changes: 2 additions & 3 deletions src/app/composer/qgscomposerlegendwidget.cpp
View file
Expand Up @@ -1006,9 +1006,8 @@ void QgsComposerLegendWidget::on_mItemTreeView_doubleClicked( const QModelIndex
currentNode->setCustomProperty( "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 )
{
Expand Down
6 changes: 3 additions & 3 deletions src/app/qgsvisibilitypresets.cpp
View file
Expand Up @@ -68,7 +68,7 @@ void QgsVisibilityPresets::addPerLayerCheckedLegendSymbols( QgsVisibilityPresetC
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 )
{
Expand Down Expand Up @@ -210,7 +210,7 @@ void QgsVisibilityPresets::applyStateToLayerTreeGroup( QgsLayerTreeGroup* parent
{
const QSet<QString>& checkedNodes = rec.mPerLayerCheckedLegendSymbols[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 ) &&
Expand All @@ -221,7 +221,7 @@ void QgsVisibilityPresets::applyStateToLayerTreeGroup( QgsLayerTreeGroup* parent
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 )
Expand Down
86 changes: 49 additions & 37 deletions src/core/layertree/qgslayertreemodel.cpp
View file
Expand Up @@ -1204,30 +1204,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();
Expand All @@ -1238,11 +1253,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() )
{
Expand All @@ -1251,30 +1266,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;
}


Expand Down Expand Up @@ -1308,6 +1324,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 );
}

Expand All @@ -1332,10 +1349,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;
}

Expand Down Expand Up @@ -1400,35 +1413,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 )
Expand Down
22 changes: 18 additions & 4 deletions src/core/layertree/qgslayertreemodel.h
View file
Expand Up @@ -119,15 +119,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
Expand Down Expand Up @@ -277,8 +283,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();
Expand Down Expand Up @@ -311,9 +315,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;
Expand All @@ -322,7 +336,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
Expand Down
3 changes: 1 addition & 2 deletions src/core/qgslegendrenderer.cpp
View file
Expand Up @@ -594,8 +594,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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/gui/layertree/qgslayertreeview.cpp
View file
Expand Up @@ -138,7 +138,7 @@ void QgsLayerTreeView::modelRowsInserted( const QModelIndex& index, int start, i
if ( QgsMapLayer* layer = nodeLayer->layer() )
{
int widgetsCount = layer->customProperty( "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( QString( "embeddedWidgets/%1/id" ).arg( i ) ).toString();
Expand All @@ -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 ) )
Expand Down Expand Up @@ -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 ) )
Expand Down

4 comments on commit b385ebd

@wonder-sk
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have realized this broke the tests - fixing the control images now, will commit a fix soonish

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wonder-sk it's not just the control images that are broken - this has changed the behaviour of legends.

Before:

expected_legend_diagram_attributes

After:
legend_diagram_attributes

The "point layer" group header is no longer rendered

@wonder-sk
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson yeah and I believe the new behavior is correct. Before the legend node embedded in parent was not correctly detected when using with diagrams - now it is detected and thus the layer group is hidden by default.

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - no strong opinions here either way. Users can get the old appearance back by manually adding a group if they want it.

Please sign in to comment.