Navigation Menu

Skip to content

Commit

Permalink
Fix composer legends are not restored from XML
Browse files Browse the repository at this point in the history 
Also add unit test to protect this and switch to unique_ptrs
instead of raw pointers
  • Loading branch information
@nyalldawson
nyalldawson committed Apr 12, 2017
1 parent 3bb3f59 commit da5c0ed
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 19 deletions.
30 changes: 15 additions & 15 deletions src/core/composer/qgscomposerlegend.cpp
View file
Expand Up @@ -35,6 +35,7 @@

QgsComposerLegend::QgsComposerLegend( QgsComposition *composition )
: QgsComposerItem( composition )
, mLegendModel( new QgsLegendModel( mComposition->project()->layerTreeRoot() ) )
, mCustomLayerTree( nullptr )
, mComposerMap( nullptr )
, mLegendFilterByMap( false )
Expand All @@ -46,8 +47,6 @@ QgsComposerLegend::QgsComposerLegend( QgsComposition *composition )
, mForceResize( false )
, mSizeToContents( true )
{
mLegendModel = new QgsLegendModel( mComposition->project()->layerTreeRoot() );

connect( &composition->atlasComposition(), &QgsAtlasComposition::renderEnded, this, &QgsComposerLegend::onAtlasEnded );
connect( &composition->atlasComposition(), &QgsAtlasComposition::featureChanged, this, &QgsComposerLegend::onAtlasFeature );

Expand All @@ -73,12 +72,6 @@ QgsComposerLegend::QgsComposerLegend()

}

QgsComposerLegend::~QgsComposerLegend()
{
delete mLegendModel;
delete mCustomLayerTree;
}

void QgsComposerLegend::paint( QPainter *painter, const QStyleOptionGraphicsItem *itemStyle, QWidget *pWidget )
{
Q_UNUSED( itemStyle );
Expand Down Expand Up @@ -119,7 +112,7 @@ void QgsComposerLegend::paint( QPainter *painter, const QStyleOptionGraphicsItem
}
mInitialMapScaleCalculated = true;

QgsLegendRenderer legendRenderer( mLegendModel, mSettings );
QgsLegendRenderer legendRenderer( mLegendModel.get(), mSettings );
legendRenderer.setLegendSize( mForceResize && mSizeToContents ? QSize() : rect().size() );

//adjust box if width or height is too small
Expand Down Expand Up @@ -180,7 +173,7 @@ QSizeF QgsComposerLegend::paintAndDetermineSize( QPainter *painter )
doUpdateFilterByMap();
}

QgsLegendRenderer legendRenderer( mLegendModel, mSettings );
QgsLegendRenderer legendRenderer( mLegendModel.get(), mSettings );
QSizeF size = legendRenderer.minimumSize();
if ( painter )
legendRenderer.drawLegend( painter );
Expand All @@ -202,7 +195,7 @@ void QgsComposerLegend::adjustBoxSize()
return;
}

QgsLegendRenderer legendRenderer( mLegendModel, mSettings );
QgsLegendRenderer legendRenderer( mLegendModel.get(), mSettings );
QSizeF size = legendRenderer.minimumSize();
QgsDebugMsg( QString( "width = %1 height = %2" ).arg( size.width() ).arg( size.height() ) );
if ( size.isValid() )
Expand All @@ -227,8 +220,7 @@ void QgsComposerLegend::setCustomLayerTree( QgsLayerTree *rootGroup )
{
mLegendModel->setRootGroup( rootGroup ? rootGroup : mComposition->project()->layerTreeRoot() );

delete mCustomLayerTree;
mCustomLayerTree = rootGroup;
mCustomLayerTree.reset( rootGroup );
}


Expand Down Expand Up @@ -493,7 +485,15 @@ bool QgsComposerLegend::readXml( const QDomElement &itemElem, const QDomDocument
if ( layerTreeElem.isNull() )
layerTreeElem = itemElem.firstChildElement( QStringLiteral( "layer-tree-group" ) );

setCustomLayerTree( QgsLayerTree::readXml( layerTreeElem ) );
if ( !layerTreeElem.isNull() )
{
std::unique_ptr< QgsLayerTree > tree( QgsLayerTree::readXml( layerTreeElem ) );
if ( mComposition )
tree->resolveReferences( mComposition->project() );
setCustomLayerTree( tree.release() );
}
else
setCustomLayerTree( nullptr );

//restore general composer item properties
QDomNodeList composerItemList = itemElem.elementsByTagName( QStringLiteral( "ComposerItem" ) );
Expand Down Expand Up @@ -686,7 +686,7 @@ void QgsComposerLegend::doUpdateFilterByMap()
mLegendModel->setLayerStyleOverrides( QMap<QString, QString>() );


bool filterByExpression = QgsLayerTreeUtils::hasLegendFilterExpression( *( mCustomLayerTree ? mCustomLayerTree : mComposition->project()->layerTreeRoot() ) );
bool filterByExpression = QgsLayerTreeUtils::hasLegendFilterExpression( *( mCustomLayerTree ? mCustomLayerTree.get() : mComposition->project()->layerTreeRoot() ) );

if ( mComposerMap && ( mLegendFilterByMap || filterByExpression || mInAtlas ) )
{
Expand Down
8 changes: 4 additions & 4 deletions src/core/composer/qgscomposerlegend.h
View file
Expand Up @@ -22,6 +22,7 @@
#include "qgscomposeritem.h"
#include "qgslayertreemodel.h"
#include "qgslegendsettings.h"
#include "qgslayertreegroup.h"

class QgsLayerTreeModel;
class QgsSymbol;
Expand Down Expand Up @@ -58,7 +59,6 @@ class CORE_EXPORT QgsComposerLegend : public QgsComposerItem

public:
QgsComposerLegend( QgsComposition *composition );
~QgsComposerLegend();

//! Return correct graphics item type.
virtual int type() const override { return ComposerLegend; }
Expand Down Expand Up @@ -90,7 +90,7 @@ class CORE_EXPORT QgsComposerLegend : public QgsComposerItem
/**
* Returns the legend model
*/
QgsLegendModel *model() { return mLegendModel; }
QgsLegendModel *model() { return mLegendModel.get(); }

//! \since QGIS 2.6
void setAutoUpdateModel( bool autoUpdate );
Expand Down Expand Up @@ -308,8 +308,8 @@ class CORE_EXPORT QgsComposerLegend : public QgsComposerItem
//! use new custom layer tree and update model. if new root is null pointer, will use project's tree
void setCustomLayerTree( QgsLayerTree *rootGroup );

QgsLegendModel *mLegendModel = nullptr;
QgsLayerTreeGroup *mCustomLayerTree = nullptr;
std::unique_ptr< QgsLegendModel > mLegendModel;
std::unique_ptr< QgsLayerTreeGroup > mCustomLayerTree;

QgsLegendSettings mSettings;

Expand Down
144 changes: 144 additions & 0 deletions tests/src/core/testqgscomposition.cpp
View file
Expand Up @@ -27,6 +27,11 @@
#include "qgsmultirenderchecker.h"
#include "qgsfillsymbollayer.h"
#include "qgsproject.h"
#include "qgscomposerlegend.h"
#include "qgsvectorlayer.h"
#include "qgslayertreegroup.h"
#include "qgslayertreelayer.h"
#include "qgslayertree.h"

#include <QObject>
#include "qgstest.h"
Expand Down Expand Up @@ -58,6 +63,8 @@ class TestQgsComposition : public QObject
void variablesEdited();
void itemVariablesFunction();
void referenceMap();
void legendRestoredFromTemplate();
void legendRestoredFromTemplateAutoUpdate();

private:
QgsComposition *mComposition = nullptr;
Expand Down Expand Up @@ -659,5 +666,142 @@ void TestQgsComposition::referenceMap()
delete composition;
}

void TestQgsComposition::legendRestoredFromTemplate()
{
// load a layer

QFileInfo vectorFileInfo( QString( TEST_DATA_DIR ) + "/points.shp" );
QgsVectorLayer *layer = new QgsVectorLayer( vectorFileInfo.filePath(),
vectorFileInfo.completeBaseName(),
"ogr" );
QgsProject p;
p.addMapLayer( layer );

// create composition
QgsComposition c( &p );
// add a legend
QgsComposerLegend *legend = new QgsComposerLegend( &c );
c.addComposerLegend( legend );
legend->setAutoUpdateModel( false );

QgsLegendModel *model = legend->model();
QgsLayerTreeNode *node = model->rootGroup()->children().at( 0 );
// make sure we've got right node
QgsLayerTreeLayer *layerNode = dynamic_cast< QgsLayerTreeLayer * >( node );
QVERIFY( layerNode );
QCOMPARE( layerNode->layer(), layer );

// got it!
layerNode->setCustomProperty( "legend/title-label", QString( "new title!" ) );
// make sure new title stuck
QCOMPARE( model->data( model->node2index( layerNode ), Qt::DisplayRole ).toString(), QString( "new title!" ) );

// save composition to template
QDomDocument doc;
QDomElement composerElem = doc.createElement( "Composer" );
doc.appendChild( composerElem );
c.writeXml( composerElem, doc );
c.atlasComposition().writeXml( composerElem, doc );


// make a new composition from template
QgsComposition c2( &p );
QVERIFY( c2.loadFromTemplate( doc ) );
// get legend from new composition
QList< QgsComposerLegend * > legends2;
c2.composerItems( legends2 );
QgsComposerLegend *legend2 = legends2.at( 0 );
QVERIFY( legend2 );

QgsLegendModel *model2 = legend2->model();
QgsLayerTreeNode *node2 = model2->rootGroup()->children().at( 0 );
QgsLayerTreeLayer *layerNode2 = dynamic_cast< QgsLayerTreeLayer * >( node2 );
QVERIFY( layerNode2 );
QCOMPARE( layerNode2->layer(), layer );
QCOMPARE( model2->data( model->node2index( layerNode2 ), Qt::DisplayRole ).toString(), QString( "new title!" ) );

#if 0 //expected failure (#2738)
QString oldId = layer->id();
// new test
// remove existing layer
p.removeMapLayer( layer );

// reload it, with a new id
QgsVectorLayer *layer2 = new QgsVectorLayer( vectorFileInfo.filePath(),
vectorFileInfo.completeBaseName(),
"ogr" );
p.addMapLayer( layer2 );
QVERIFY( oldId != layer2->id() );

// load composition from template
QgsComposition c3( &p );
QVERIFY( c3.loadFromTemplate( doc ) );
// get legend from new composition
QList< QgsComposerLegend * > legends3;
c3.composerItems( legends3 );
QgsComposerLegend *legend3 = legends3.at( 0 );
QVERIFY( legend3 );

//make sure customisation remains intact
QgsLegendModel *model3 = legend3->model();
QgsLayerTreeNode *node3 = model3->rootGroup()->children().at( 0 );
QgsLayerTreeLayer *layerNode3 = dynamic_cast< QgsLayerTreeLayer * >( node3 );
QVERIFY( layerNode3 );
QCOMPARE( layerNode3->layer(), layer2 );
QCOMPARE( model3->data( model->node2index( layerNode3 ), Qt::DisplayRole ).toString(), QString( "new title!" ) );
#endif
}

void TestQgsComposition::legendRestoredFromTemplateAutoUpdate()
{
// load a layer

QFileInfo vectorFileInfo( QString( TEST_DATA_DIR ) + "/points.shp" );
QgsVectorLayer *layer = new QgsVectorLayer( vectorFileInfo.filePath(),
vectorFileInfo.completeBaseName(),
"ogr" );
QgsProject p;
p.addMapLayer( layer );

// create composition
QgsComposition c( &p );
// add a legend
QgsComposerLegend *legend = new QgsComposerLegend( &c );
c.addComposerLegend( legend );
legend->setAutoUpdateModel( true );

QgsLegendModel *model = legend->model();
QgsLayerTreeNode *node = model->rootGroup()->children().at( 0 );
// make sure we've got right node
QgsLayerTreeLayer *layerNode = dynamic_cast< QgsLayerTreeLayer * >( node );
QVERIFY( layerNode );
QCOMPARE( layerNode->layer(), layer );
QCOMPARE( model->data( model->node2index( layerNode ), Qt::DisplayRole ).toString(), QString( "points" ) );

// save composition to template
QDomDocument doc;
QDomElement composerElem = doc.createElement( "Composer" );
doc.appendChild( composerElem );
c.writeXml( composerElem, doc );
c.atlasComposition().writeXml( composerElem, doc );


// make a new composition from template
QgsComposition c2( &p );
QVERIFY( c2.loadFromTemplate( doc ) );
// get legend from new composition
QList< QgsComposerLegend * > legends2;
c2.composerItems( legends2 );
QgsComposerLegend *legend2 = legends2.at( 0 );
QVERIFY( legend2 );

QgsLegendModel *model2 = legend2->model();
QgsLayerTreeNode *node2 = model2->rootGroup()->children().at( 0 );
QgsLayerTreeLayer *layerNode2 = dynamic_cast< QgsLayerTreeLayer * >( node2 );
QVERIFY( layerNode2 );
QCOMPARE( layerNode2->layer(), layer );
QCOMPARE( model2->data( model->node2index( layerNode2 ), Qt::DisplayRole ).toString(), QString( "points" ) );
}

QGSTEST_MAIN( TestQgsComposition )
#include "testqgscomposition.moc"

0 comments on commit da5c0ed

Please sign in to comment.