Skip to content
Permalink
Browse files

Fix crash when attempting to render multipolygon with missing exterio…

…r ring

This commit fixes a possible crash when the vector layer renderer
attempts to render a multipolygon containing a polygon without
an exterior ring.

The underlying cause of the creation of this invalid geometry is deeper,
but this commit hardens the renderer and makes it more robust for
handling bad geometries.

Fixes #17365
  • Loading branch information
nyalldawson committed Oct 31, 2017
1 parent 6418a83 commit eea155d6e28bbe3d66dd32e972d7c0472bbf3af4
@@ -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 );
@@ -914,6 +914,9 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont

context.setGeometry( geomCollection.geometryN( i ) );
const QgsPolygon &polygon = dynamic_cast<const QgsPolygon &>( *geomCollection.geometryN( i ) );
if ( !polygon.exteriorRing() )
break;

_getPolygon( pts, holes, context, polygon, !tileMapRendering && clipFeaturesToExtent() );
static_cast<QgsFillSymbol *>( this )->renderPolygon( pts, ( !holes.isEmpty() ? &holes : nullptr ), &feature, context, layer, selected );

@@ -22,11 +22,16 @@
#include <QDesktopServices>

//qgis includes...
#include <qgsmaplayer.h>
#include <qgsvectorlayer.h>
#include <qgsapplication.h>
#include <qgsproviderregistry.h>
#include <qgsproject.h>
#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,13 +57,15 @@ class TestQgsRenderers : public QObject
void cleanup() {} // will be called after every testfunction.

void singleSymbol();
void emptyGeometry();
// void uniqueValue();
// void graduatedSymbol();
// void continuousSymbol();
private:
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
//
Binary file not shown.

0 comments on commit eea155d

Please sign in to comment.
You can’t perform that action at this time.