diff --git a/src/core/qgslegendrenderer.cpp b/src/core/qgslegendrenderer.cpp index 0d8f9534d8a..48875f229a0 100644 --- a/src/core/qgslegendrenderer.cpp +++ b/src/core/qgslegendrenderer.cpp @@ -265,17 +265,12 @@ void QgsLegendRenderer::setColumns( QList& atomList ) // Divide atoms to columns double totalHeight = 0; - // bool first = true; qreal maxAtomHeight = 0; Q_FOREACH ( const Atom& atom, atomList ) { - //if ( !first ) - //{ totalHeight += spaceAboveAtom( atom ); - //} totalHeight += atom.size.height(); maxAtomHeight = qMax( atom.size.height(), maxAtomHeight ); - // first = false; } // We know height of each atom and we have to split them into columns @@ -283,31 +278,36 @@ void QgsLegendRenderer::setColumns( QList& atomList ) // We are using simple heuristic, brute fore appeared to be to slow, // the number of combinations is N = n!/(k!*(n-k)!) where n = atomsCount-1 // and k = columnsCount-1 - - double avgColumnHeight = totalHeight / mSettings.columnCount(); + double maxColumnHeight = 0; int currentColumn = 0; int currentColumnAtomCount = 0; // number of atoms in current column double currentColumnHeight = 0; - double maxColumnHeight = 0; double closedColumnsHeight = 0; - // first = true; // first in column + for ( int i = 0; i < atomList.size(); i++ ) { - Atom atom = atomList[i]; + // Recalc average height for remaining columns including current + double avgColumnHeight = ( totalHeight - closedColumnsHeight ) / ( mSettings.columnCount() - currentColumn ); + + Atom atom = atomList.at( i ); double currentHeight = currentColumnHeight; - //if ( !first ) - //{ - currentHeight += spaceAboveAtom( atom ); - //} + if ( currentColumnAtomCount > 0 ) + currentHeight += spaceAboveAtom( atom ); currentHeight += atom.size.height(); - // Recalc average height for remaining columns including current - avgColumnHeight = ( totalHeight - closedColumnsHeight ) / ( mSettings.columnCount() - currentColumn ); - if (( currentHeight - avgColumnHeight ) > atom.size.height() / 2 // center of current atom is over average height - && currentColumnAtomCount > 0 // do not leave empty column - && currentHeight > maxAtomHeight // no sense to make smaller columns than max atom height - && currentHeight > maxColumnHeight // no sense to make smaller columns than max column already created - && currentColumn < mSettings.columnCount() - 1 ) // must not exceed max number of columns + bool canCreateNewColumn = ( currentColumnAtomCount > 0 ) // do not leave empty column + && ( currentColumn < mSettings.columnCount() - 1 ); // must not exceed max number of columns + + bool shouldCreateNewColumn = ( currentHeight - avgColumnHeight ) > atom.size.height() / 2 // center of current atom is over average height + && currentColumnAtomCount > 0 // do not leave empty column + && currentHeight > maxAtomHeight // no sense to make smaller columns than max atom height + && currentHeight > maxColumnHeight; // no sense to make smaller columns than max column already created + + // also should create a new column if the number of items left < number of columns left + // in this case we should spread the remaining items out over the remaining columns + shouldCreateNewColumn |= ( atomList.size() - i < mSettings.columnCount() - currentColumn ); + + if ( canCreateNewColumn && shouldCreateNewColumn ) { // New column currentColumn++; @@ -322,11 +322,9 @@ void QgsLegendRenderer::setColumns( QList& atomList ) atomList[i].column = currentColumn; currentColumnAtomCount++; maxColumnHeight = qMax( currentColumnHeight, maxColumnHeight ); - - // first = false; } - // Alling labels of symbols for each layr/column to the same labelXOffset + // Align labels of symbols for each layr/column to the same labelXOffset QMap maxSymbolWidth; for ( int i = 0; i < atomList.size(); i++ ) { diff --git a/tests/src/core/testqgslegendrenderer.cpp b/tests/src/core/testqgslegendrenderer.cpp index d968fdef17f..78457e67a15 100644 --- a/tests/src/core/testqgslegendrenderer.cpp +++ b/tests/src/core/testqgslegendrenderer.cpp @@ -118,6 +118,8 @@ class TestQgsLegendRenderer : public QObject void testThreeColumns(); void testFilterByMap(); void testFilterByMapSameSymbol(); + void testColumns_data(); + void testColumns(); void testRasterBorder(); void testFilterByPolygon(); void testFilterByExpression(); @@ -131,6 +133,7 @@ class TestQgsLegendRenderer : public QObject QgsVectorLayer* mVL3; // point QgsRasterLayer* mRL; QString mReport; + bool _testLegendColumns( int itemCount, int columnCount, const QString& testName ); }; @@ -459,6 +462,65 @@ void TestQgsLegendRenderer::testFilterByMapSameSymbol() QgsMapLayerRegistry::instance()->removeMapLayer( vl4 ); } +bool TestQgsLegendRenderer::_testLegendColumns( int itemCount, int columnCount, const QString& testName ) +{ + QgsFillSymbol* sym = new QgsFillSymbol(); + sym->setColor( Qt::cyan ); + + QgsLayerTreeGroup* root = new QgsLayerTreeGroup(); + + QList< QgsVectorLayer* > layers; + for ( int i = 1; i <= itemCount; ++i ) + { + QgsVectorLayer* vl = new QgsVectorLayer( "Polygon", QString( "Layer %1" ).arg( i ), "memory" ); + QgsMapLayerRegistry::instance()->addMapLayer( vl ); + vl->setRenderer( new QgsSingleSymbolRenderer( sym->clone() ) ); + root->addLayer( vl ); + layers << vl; + } + delete sym; + + QgsLayerTreeModel legendModel( root ); + QgsLegendSettings settings; + settings.setColumnCount( columnCount ); + _setStandardTestFont( settings, "Bold" ); + _renderLegend( testName, &legendModel, settings ); + bool result = _verifyImage( testName, mReport ); + + Q_FOREACH ( QgsVectorLayer* l, layers ) + { + QgsMapLayerRegistry::instance()->removeMapLayer( l ); + } + return result; +} + +void TestQgsLegendRenderer::testColumns_data() +{ + QTest::addColumn( "testName" ); + QTest::addColumn( "items" ); + QTest::addColumn( "columns" ); + + QTest::newRow( "2 items, 2 columns" ) << "legend_2_by_2" << 2 << 2; + QTest::newRow( "3 items, 2 columns" ) << "legend_3_by_2" << 3 << 2; + QTest::newRow( "4 items, 2 columns" ) << "legend_4_by_2" << 4 << 2; + QTest::newRow( "5 items, 2 columns" ) << "legend_5_by_2" << 5 << 2; + QTest::newRow( "3 items, 3 columns" ) << "legend_3_by_3" << 3 << 3; + QTest::newRow( "4 items, 3 columns" ) << "legend_4_by_3" << 4 << 3; + QTest::newRow( "5 items, 3 columns" ) << "legend_5_by_3" << 5 << 3; + QTest::newRow( "6 items, 3 columns" ) << "legend_6_by_3" << 6 << 3; + QTest::newRow( "7 items, 3 columns" ) << "legend_7_by_3" << 7 << 3; +} + +void TestQgsLegendRenderer::testColumns() +{ + //test rendering legend with different combinations of columns and items + + QFETCH( QString, testName ); + QFETCH( int, items ); + QFETCH( int, columns ); + QVERIFY( _testLegendColumns( items, columns, testName ) ); +} + void TestQgsLegendRenderer::testRasterBorder() { QString testName = "legend_raster_border"; diff --git a/tests/testdata/control_images/legend/expected_legend_2_by_2/expected_legend_2_by_2.png b/tests/testdata/control_images/legend/expected_legend_2_by_2/expected_legend_2_by_2.png new file mode 100644 index 00000000000..33fce227f59 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_2_by_2/expected_legend_2_by_2.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_2_by_2/expected_legend_2_by_2_mask.png b/tests/testdata/control_images/legend/expected_legend_2_by_2/expected_legend_2_by_2_mask.png new file mode 100644 index 00000000000..b2f5f23fc80 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_2_by_2/expected_legend_2_by_2_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_3_by_2/expected_legend_3_by_2.png b/tests/testdata/control_images/legend/expected_legend_3_by_2/expected_legend_3_by_2.png new file mode 100644 index 00000000000..925aa738580 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_3_by_2/expected_legend_3_by_2.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_3_by_2/expected_legend_3_by_2_mask.png b/tests/testdata/control_images/legend/expected_legend_3_by_2/expected_legend_3_by_2_mask.png new file mode 100644 index 00000000000..26a6e24be36 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_3_by_2/expected_legend_3_by_2_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_3_by_3/expected_legend_3_by_3.png b/tests/testdata/control_images/legend/expected_legend_3_by_3/expected_legend_3_by_3.png new file mode 100644 index 00000000000..027354915fd Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_3_by_3/expected_legend_3_by_3.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_3_by_3/expected_legend_3_by_3_mask.png b/tests/testdata/control_images/legend/expected_legend_3_by_3/expected_legend_3_by_3_mask.png new file mode 100644 index 00000000000..619c13d4824 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_3_by_3/expected_legend_3_by_3_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_4_by_2/expected_legend_4_by_2.png b/tests/testdata/control_images/legend/expected_legend_4_by_2/expected_legend_4_by_2.png new file mode 100644 index 00000000000..a3e7fb3617c Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_4_by_2/expected_legend_4_by_2.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_4_by_2/expected_legend_4_by_2_mask.png b/tests/testdata/control_images/legend/expected_legend_4_by_2/expected_legend_4_by_2_mask.png new file mode 100644 index 00000000000..5f831690654 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_4_by_2/expected_legend_4_by_2_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_4_by_3/expected_legend_4_by_3.png b/tests/testdata/control_images/legend/expected_legend_4_by_3/expected_legend_4_by_3.png new file mode 100644 index 00000000000..c1e72c655da Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_4_by_3/expected_legend_4_by_3.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_4_by_3/expected_legend_4_by_3_mask.png b/tests/testdata/control_images/legend/expected_legend_4_by_3/expected_legend_4_by_3_mask.png new file mode 100644 index 00000000000..eb17139ea92 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_4_by_3/expected_legend_4_by_3_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_5_by_2/expected_legend_5_by_2.png b/tests/testdata/control_images/legend/expected_legend_5_by_2/expected_legend_5_by_2.png new file mode 100644 index 00000000000..e61cb2f4701 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_5_by_2/expected_legend_5_by_2.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_5_by_2/expected_legend_5_by_2_mask.png b/tests/testdata/control_images/legend/expected_legend_5_by_2/expected_legend_5_by_2_mask.png new file mode 100644 index 00000000000..b7c9aa4ac68 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_5_by_2/expected_legend_5_by_2_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_5_by_3/expected_legend_5_by_3.png b/tests/testdata/control_images/legend/expected_legend_5_by_3/expected_legend_5_by_3.png new file mode 100644 index 00000000000..56f1db3139a Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_5_by_3/expected_legend_5_by_3.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_5_by_3/expected_legend_5_by_3_mask.png b/tests/testdata/control_images/legend/expected_legend_5_by_3/expected_legend_5_by_3_mask.png new file mode 100644 index 00000000000..8fabd8daaed Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_5_by_3/expected_legend_5_by_3_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_6_by_3/expected_legend_6_by_3.png b/tests/testdata/control_images/legend/expected_legend_6_by_3/expected_legend_6_by_3.png new file mode 100644 index 00000000000..26155b3c8ee Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_6_by_3/expected_legend_6_by_3.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_6_by_3/expected_legend_6_by_3_mask.png b/tests/testdata/control_images/legend/expected_legend_6_by_3/expected_legend_6_by_3_mask.png new file mode 100644 index 00000000000..0228bfca536 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_6_by_3/expected_legend_6_by_3_mask.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_7_by_3/expected_legend_7_by_3.png b/tests/testdata/control_images/legend/expected_legend_7_by_3/expected_legend_7_by_3.png new file mode 100644 index 00000000000..bae0e2f2cb4 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_7_by_3/expected_legend_7_by_3.png differ diff --git a/tests/testdata/control_images/legend/expected_legend_7_by_3/expected_legend_7_by_3_mask.png b/tests/testdata/control_images/legend/expected_legend_7_by_3/expected_legend_7_by_3_mask.png new file mode 100644 index 00000000000..63ef7e962e0 Binary files /dev/null and b/tests/testdata/control_images/legend/expected_legend_7_by_3/expected_legend_7_by_3_mask.png differ