From e38daf6b2fb61974709286513595dc5efe0d4f31 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 12 Nov 2015 11:15:40 +0100 Subject: [PATCH] Fix endpoint swap on negative marker line offset Fixes #13811 Includes testcase --- .../symbology-ng/qgssymbollayerv2utils.cpp | 6 +- tests/src/core/CMakeLists.txt | 1 + tests/src/core/testqgsmarkerlinesymbol.cpp | 154 ++++++++++ .../expected_line_offset.png | Bin 0 -> 40258 bytes tests/testdata/marker_line_offset.qml | 284 ++++++++++++++++++ 5 files changed, 444 insertions(+), 1 deletion(-) create mode 100644 tests/src/core/testqgsmarkerlinesymbol.cpp create mode 100644 tests/testdata/control_images/markerlinesymbol/expected_line_offset/expected_line_offset.png create mode 100644 tests/testdata/marker_line_offset.qml diff --git a/src/core/symbology-ng/qgssymbollayerv2utils.cpp b/src/core/symbology-ng/qgssymbollayerv2utils.cpp index 93d985c0de5c..c64cd81008d1 100644 --- a/src/core/symbology-ng/qgssymbollayerv2utils.cpp +++ b/src/core/symbology-ng/qgssymbollayerv2utils.cpp @@ -750,7 +750,11 @@ QList offsetLine( QPolygonF polyline, double dist, QGis::GeometryType if ( QGis::flatType( tempGeometry->wkbType() ) == QGis::WKBLineString ) { - resultLine.append( makeOffsetGeometry( tempGeometry->asPolyline() ) ); + QgsPolyline line = tempGeometry->asPolyline(); + // Reverse the line if offset was negative, see + // http://hub.qgis.org/issues/13811 + if ( dist < 0 ) std::reverse(line.begin(), line.end() ); + resultLine.append( makeOffsetGeometry( line ) ); delete tempGeometry; return resultLine; } diff --git a/tests/src/core/CMakeLists.txt b/tests/src/core/CMakeLists.txt index ac7f4b364ecd..bbffec3e0a76 100644 --- a/tests/src/core/CMakeLists.txt +++ b/tests/src/core/CMakeLists.txt @@ -142,6 +142,7 @@ ADD_QGIS_TEST(colorscheme testqgscolorscheme.cpp) ADD_QGIS_TEST(maptopixeltest testqgsmaptopixel.cpp) ADD_QGIS_TEST(maprotationtest testqgsmaprotation.cpp) ADD_QGIS_TEST(mapsettingstest testqgsmapsettings.cpp) +ADD_QGIS_TEST(markerlinessymboltest testqgsmarkerlinesymbol.cpp) ADD_QGIS_TEST(networkcontentfetcher testqgsnetworkcontentfetcher.cpp ) ADD_QGIS_TEST(legendrenderertest testqgslegendrenderer.cpp ) ADD_QGIS_TEST(vectorlayerjoinbuffer testqgsvectorlayerjoinbuffer.cpp ) diff --git a/tests/src/core/testqgsmarkerlinesymbol.cpp b/tests/src/core/testqgsmarkerlinesymbol.cpp new file mode 100644 index 000000000000..fdec55ee1adf --- /dev/null +++ b/tests/src/core/testqgsmarkerlinesymbol.cpp @@ -0,0 +1,154 @@ +/*************************************************************************** + testqgsmarkerlinesymbol.cpp + -------------------------------------- + Date : Nov 12 2015 + Copyright : (C) 2015 by Sandro Santilli + Email : strk@keybit.net + *************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ +#include +#include +#include +#include +#include +#include +#include + +//qgis includes... +#include "qgsrasterlayer.h" +#include "qgsvectorlayer.h" +#include "qgsmultibandcolorrenderer.h" +#include "qgsmaplayerregistry.h" +#include "qgsapplication.h" +#include "qgsmaprenderer.h" +#include "qgspallabeling.h" +#include "qgsfontutils.h" + +//qgis unit test includes +#include + +/** \ingroup UnitTests + * This is a unit test for the Marker Line symbol + */ +class TestQgsMarkerLineSymbol : public QObject +{ + Q_OBJECT + public: + TestQgsMarkerLineSymbol() + : mLinesLayer( 0 ) + , mMapSettings( 0 ) + { + mTestDataDir = QString( TEST_DATA_DIR ) + '/'; + } + + ~TestQgsMarkerLineSymbol(); + + private slots: + void initTestCase();// will be called before the first testfunction is executed. + void cleanupTestCase();// will be called after the last testfunction was executed. + void init() {} // will be called before each testfunction is executed. + void cleanup() {} // will be called after every testfunction. + + void lineOffset(); + + private: + bool render( const QString& theFileName ); + + QString mTestDataDir; + QgsVectorLayer* mLinesLayer; + QgsMapSettings *mMapSettings; + QString mReport; +}; + +//runs before all tests +void TestQgsMarkerLineSymbol::initTestCase() +{ + // init QGIS's paths - true means that all path will be inited from prefix + QgsApplication::init(); + QgsApplication::initQgis(); + + mMapSettings = new QgsMapSettings(); + + QList mapLayers; + + //create a line layer that will be used in all tests... + QString myLinesFileName = mTestDataDir + "lines_cardinals.shp"; + QFileInfo myLinesFileInfo( myLinesFileName ); + mLinesLayer = new QgsVectorLayer( myLinesFileInfo.filePath(), + myLinesFileInfo.completeBaseName(), "ogr" ); + mapLayers << mLinesLayer; + + // Register all layers with the registry + QgsMapLayerRegistry::instance()->addMapLayers( mapLayers ); + + // This is needed to correctly set rotation center, + // the actual size doesn't matter as QgsRenderChecker will + // re-set it to the size of the expected image + mMapSettings->setOutputSize( QSize( 256, 256 ) ); + + mReport += "

Line Marker Symbol Tests

\n"; + + QgsFontUtils::loadStandardTestFonts( QStringList() << "Bold" ); +} + +TestQgsMarkerLineSymbol::~TestQgsMarkerLineSymbol() +{ + +} + +//runs after all tests +void TestQgsMarkerLineSymbol::cleanupTestCase() +{ + delete mMapSettings; + QgsApplication::exitQgis(); + + QString myReportFile = QDir::tempPath() + "/qgistest.html"; + QFile myFile( myReportFile ); + if ( myFile.open( QIODevice::WriteOnly | QIODevice::Append ) ) + { + QTextStream myQTextStream( &myFile ); + myQTextStream << mReport; + myFile.close(); + } +} + +void TestQgsMarkerLineSymbol::lineOffset() +{ + mMapSettings->setLayers( QStringList() << mLinesLayer->id() ); + + // Negative offset on marker line + // See http://hub.qgis.org/issues/13811 + + QString qml = mTestDataDir + "marker_line_offset.qml"; + bool success = false; + mLinesLayer->loadNamedStyle( qml, success ); + + QVERIFY( success ); + mMapSettings->setExtent( QgsRectangle(-140,-140,140,140) ); + QVERIFY( render( "line_offset" ) ); + + // TODO: -0.0 offset, see + // http://hub.qgis.org/issues/13811#note-1 +} + +bool TestQgsMarkerLineSymbol::render( const QString& theTestType ) +{ + mReport += "

" + theTestType + "

\n"; + mMapSettings->setOutputDpi( 96 ); + QgsRenderChecker checker; + checker.setControlPathPrefix( "markerlinesymbol" ); + checker.setControlName( "expected_" + theTestType ); + checker.setMapSettings( *mMapSettings ); + bool result = checker.runTest( theTestType ); + mReport += "\n\n\n" + checker.report(); + return result; +} + +QTEST_MAIN( TestQgsMarkerLineSymbol ) +#include "testqgsmarkerlinesymbol.moc" diff --git a/tests/testdata/control_images/markerlinesymbol/expected_line_offset/expected_line_offset.png b/tests/testdata/control_images/markerlinesymbol/expected_line_offset/expected_line_offset.png new file mode 100644 index 0000000000000000000000000000000000000000..756dad1a90966d8a89374ca5fa283e563dee0681 GIT binary patch literal 40258 zcmeI5U2GIp6vxjlOW9Ui+ETzu8!?C}5{=P>R3##9!R||g(qfE?5HKo1tY88n3DrnK zm6l4a)l$}muQnmAF{u`92olsLCTe_9wDkpivMCiCzU*$-Ior-=cAeRoJDqmU&YY8M z=g$57-~Vs!%@yv|dGXNknP+s<=wB3v^f132S`eIY1wDIjI2~_%}*yG>4 zU)oQvE`NFlK-M?-@-!{YS^%&R0%a>JcX?oYP2FD4NBuV=Qbi6T4tR;^U>Oq*Fc4yg z0s~hgyMK02&1!hm(1XQTWU~J8pto_dtpma zM%?SdcV}a!>%ovKQi77wKq8R{jD^E-uLz8PSK51|A?eDHVaQu5P*NI*Y-Zv%3SX#{n@-A)%pNP#8!82<5IFP<%=O zPDsz3g57Z2q9V)s)#@lEfHd~JJS5)>pfwS-{4Fh?!jgwe z)QVCfk56C3FfkAYLd*eTa(IO>5MmAxlfx^7fe>?mm>gaq41|~i#N_Y_VIagDASQ=b z2m>MJ05LhdLKp}!2Z+hx6~aJ>IY3MfuMh@8%mHF@c!gLDq<&L)iJ5x=)MPLmHCJrR zV<0w~uQoITQFFz{JO*N;`D#Nm5H(k9%wr%nny)sr@GYjZ`_4(vF46dSVDX&*(&J@R<3e~8ie}6tA#W@#4SxD|Ku8Ob zLrF-pqboBIVt5${F(ASh_1_O2m!>d&QO`Z|IplSnh)roMZ59k1{3NzN)rmjOeF@#c zhPYu$U)T=b<*VZIrgCR-^?LA(hGIhuR;`EO$I7Go9usdagn^7Kdn7v8{NpWgFrt~# z6HOz_mdC_Rbz<MDqb8c&)!Wz9eK zF%-Q2j`2}kRUuF4k| zqEHM0VHmQ#=C)BN?MDnX)&;^a4Ed;Asj&Vnm}dY=QXmXN*Jq&$MxbZ{@eL0_ep{2! zfI5|*y=V6mR^1DrGgN96=LZ|XcV$>ptR|dL&K&ryxgEk;*_w^HK+J(qb}saP*bbrD zIhqYMfnba4cQ0iqn%0RG$S|%gmhK)(5=ntzJBE_PJ%dF?VPJQ+G8s=7iT1p$ZJPW!-ArFr)@$EM8j%3V4xJo0a@|P ziy9Ui&$bJOBZ1t%dJU}GQ)k@$gqn83a3m0%RLS#OO+m3625L{~_rdsn*C#z=rws4J zGiC?{|9$AGeXlEvHPaMepg7@&*fnQFwBSrzqb?LGFwjEBUDOkgi_#6RK-HTE60>9b zlwhDJ#{BenPrGP>a2I@@di#kp~BoGQlPjl_Ge*!nE z)A1K|H6yP6Jp{L8&rvI~J + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +