diff --git a/src/core/qgsvectorlayerrenderer.cpp b/src/core/qgsvectorlayerrenderer.cpp index 14fa04b3bf5..b5ccc9e3529 100644 --- a/src/core/qgsvectorlayerrenderer.cpp +++ b/src/core/qgsvectorlayerrenderer.cpp @@ -271,7 +271,7 @@ void QgsVectorLayerRenderer::drawRenderer( QgsFeatureIterator &fit ) break; } - if ( !fet.hasGeometry() ) + if ( !fet.hasGeometry() || fet.geometry().isEmpty() ) continue; // skip features without geometry mContext.expressionContext().setFeature( fet ); diff --git a/src/core/symbology/qgssymbol.cpp b/src/core/symbology/qgssymbol.cpp index 6e812d1f285..6fe07bd005c 100644 --- a/src/core/symbology/qgssymbol.cpp +++ b/src/core/symbology/qgssymbol.cpp @@ -914,6 +914,9 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont context.setGeometry( geomCollection.geometryN( i ) ); const QgsPolygon &polygon = dynamic_cast( *geomCollection.geometryN( i ) ); + if ( !polygon.exteriorRing() ) + break; + _getPolygon( pts, holes, context, polygon, !tileMapRendering && clipFeaturesToExtent() ); static_cast( this )->renderPolygon( pts, ( !holes.isEmpty() ? &holes : nullptr ), &feature, context, layer, selected ); diff --git a/tests/src/core/testqgsrenderers.cpp b/tests/src/core/testqgsrenderers.cpp index e852fd5a247..a37f446c187 100644 --- a/tests/src/core/testqgsrenderers.cpp +++ b/tests/src/core/testqgsrenderers.cpp @@ -22,11 +22,16 @@ #include //qgis includes... -#include -#include -#include -#include -#include +#include "qgsmaplayer.h" +#include "qgsvectorlayer.h" +#include "qgsapplication.h" +#include "qgsproviderregistry.h" +#include "qgsproject.h" +#include "qgsmultipolygon.h" +#include "qgspolygon.h" +#include "qgslinestring.h" +#include "qgsmultilinestring.h" +#include "qgsmultipoint.h" //qgis test includes #include "qgsmultirenderchecker.h" @@ -52,6 +57,7 @@ class TestQgsRenderers : public QObject void cleanup() {} // will be called after every testfunction. void singleSymbol(); + void emptyGeometry(); // void uniqueValue(); // void graduatedSymbol(); // void continuousSymbol(); @@ -59,6 +65,7 @@ class TestQgsRenderers : public QObject bool mTestHasError = false ; bool setQml( const QString &type ); //uniquevalue / continuous / single / bool imageCheck( const QString &type ); //as above + bool checkEmptyRender( const QString &name, QgsVectorLayer *layer ); QgsMapSettings *mMapSettings = nullptr; QgsMapLayer *mpPointsLayer = nullptr; QgsMapLayer *mpLinesLayer = nullptr; @@ -147,6 +154,80 @@ void TestQgsRenderers::singleSymbol() QVERIFY( imageCheck( "single" ) ); } +void TestQgsRenderers::emptyGeometry() +{ + // test rendering an empty geometry + // note - this test is of limited use, because the features with empty geometries should not be rendered + // by the feature iterator given that renderer uses a filter rect on the request. It's placed here + // for manual testing by commenting out the part of the renderer which places the filterrect on the + // layer's feature request. The purpose of this test is to ensure that we do not crash for empty geometries, + // as it's possible that malformed providers OR bugs in underlying libraries will still return empty geometries + // even when a filter rect request was made, and we shouldn't crash for these. + QgsVectorLayer *vl = new QgsVectorLayer( QStringLiteral( "MultiPolygon?crs=epsg:4326&field=pk:int&field=col1:string" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); + QVERIFY( vl->isValid() ); + QgsProject::instance()->addMapLayer( vl ); + + QgsFeature f; + std::unique_ptr< QgsMultiPolygon > mp = qgis::make_unique< QgsMultiPolygon >(); + mp->addGeometry( new QgsPolygon() ); + f.setGeometry( QgsGeometry( std::move( mp ) ) ); + QVERIFY( vl->dataProvider()->addFeature( f ) ); + QVERIFY( checkEmptyRender( "Multipolygon", vl ) ); + + // polygon + vl = new QgsVectorLayer( QStringLiteral( "Polygon?crs=epsg:4326&field=pk:int&field=col1:string" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); + QVERIFY( vl->isValid() ); + QgsProject::instance()->addMapLayer( vl ); + f.setGeometry( QgsGeometry( new QgsPolygon() ) ); + QVERIFY( vl->dataProvider()->addFeature( f ) ); + QVERIFY( checkEmptyRender( "Polygon", vl ) ); + + // linestring + vl = new QgsVectorLayer( QStringLiteral( "LineString?crs=epsg:4326&field=pk:int&field=col1:string" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); + QVERIFY( vl->isValid() ); + QgsProject::instance()->addMapLayer( vl ); + f.setGeometry( QgsGeometry( new QgsLineString() ) ); + QVERIFY( vl->dataProvider()->addFeature( f ) ); + QVERIFY( checkEmptyRender( "LineString", vl ) ); + + // multilinestring + vl = new QgsVectorLayer( QStringLiteral( "MultiLineString?crs=epsg:4326&field=pk:int&field=col1:string" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); + QVERIFY( vl->isValid() ); + QgsProject::instance()->addMapLayer( vl ); + std::unique_ptr< QgsMultiLineString > mls = qgis::make_unique< QgsMultiLineString >(); + mls->addGeometry( new QgsLineString() ); + f.setGeometry( QgsGeometry( std::move( mls ) ) ); + QVERIFY( vl->dataProvider()->addFeature( f ) ); + QVERIFY( checkEmptyRender( "MultiLineString", vl ) ); + + // multipoint + vl = new QgsVectorLayer( QStringLiteral( "MultiPoint?crs=epsg:4326&field=pk:int&field=col1:string" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ); + QVERIFY( vl->isValid() ); + QgsProject::instance()->addMapLayer( vl ); + std::unique_ptr< QgsMultiPoint > mlp = qgis::make_unique< QgsMultiPoint >(); + f.setGeometry( QgsGeometry( std::move( mlp ) ) ); + QVERIFY( vl->dataProvider()->addFeature( f ) ); + QVERIFY( checkEmptyRender( "MultiPoint", vl ) ); +} + +bool TestQgsRenderers::checkEmptyRender( const QString &testName, QgsVectorLayer *layer ) +{ + QgsMapSettings ms; + QgsRectangle extent( -180, -90, 180, 90 ); + ms.setExtent( extent ); + ms.setFlag( QgsMapSettings::ForceVectorOutput ); + ms.setOutputDpi( 96 ); + ms.setLayers( QList< QgsMapLayer * >() << layer ); + QgsMultiRenderChecker myChecker; + myChecker.setControlName( "expected_emptygeometry" ); + myChecker.setMapSettings( ms ); + myChecker.setControlPathPrefix( "map_renderer" ); + myChecker.setColorTolerance( 15 ); + bool myResultFlag = myChecker.runTest( testName, 200 ); + mReport += myChecker.report(); + return myResultFlag; +} + // // Private helper functions not called directly by CTest // diff --git a/tests/testdata/control_images/map_renderer/expected_emptygeometry/expected_emptygeometry.png b/tests/testdata/control_images/map_renderer/expected_emptygeometry/expected_emptygeometry.png new file mode 100644 index 00000000000..dff6c86be40 Binary files /dev/null and b/tests/testdata/control_images/map_renderer/expected_emptygeometry/expected_emptygeometry.png differ