Skip to content

Commit

Permalink
[labeling] Fix missing labels when multilinestring features are used in
Browse files Browse the repository at this point in the history
conjunction with the "merge connected lines" setting

Refs #12173
  • Loading branch information
nyalldawson committed May 30, 2019
1 parent 44d993d commit 7213030
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 15 deletions.
6 changes: 4 additions & 2 deletions python/core/auto_generated/qgspallabeling.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -560,28 +560,30 @@ called to find out whether the layer is used for labeling
%End


static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() ) /Factory/;
static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false ) /Factory/;
%Docstring
Prepares a geometry for registration with PAL. Handles reprojection, rotation, clipping, etc.

:param geometry: geometry to prepare
:param context: render context
:param ct: coordinate transform, or invalid transform if no transformation required
:param clipGeometry: geometry to clip features to, if applicable
:param mergeLines: ``True`` if touching lines from this layer will be merged and treated as single features during labeling

:return: prepared geometry

.. versionadded:: 2.9
%End

static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() );
static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false );
%Docstring
Checks whether a geometry requires preparation before registration with PAL

:param geometry: geometry to prepare
:param context: render context
:param ct: coordinate transform, or invalid transform if no transformation required
:param clipGeometry: geometry to clip features to, if applicable
:param mergeLines: ``True`` if touching lines from this layer will be merged and treated as single features during labeling

:return: ``True`` if geometry requires preparation

Expand Down
30 changes: 20 additions & 10 deletions src/core/qgspallabeling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1646,9 +1646,9 @@ void QgsPalLayerSettings::registerFeature( const QgsFeature &f, QgsRenderContext
if ( geom.type() == QgsWkbTypes::PolygonGeometry && fitInPolygonOnly )
{
permissibleZone = geom;
if ( QgsPalLabeling::geometryRequiresPreparation( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry() ) )
if ( QgsPalLabeling::geometryRequiresPreparation( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) )
{
permissibleZone = QgsPalLabeling::prepareGeometry( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry() );
permissibleZone = QgsPalLabeling::prepareGeometry( permissibleZone, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines );
}
}

Expand All @@ -1661,9 +1661,9 @@ void QgsPalLayerSettings::registerFeature( const QgsFeature &f, QgsRenderContext
}

geos::unique_ptr geos_geom_clone;
if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, doClip ? extentGeom : QgsGeometry() ) )
if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) )
{
geom = QgsPalLabeling::prepareGeometry( geom, context, ct, doClip ? extentGeom : QgsGeometry() );
geom = QgsPalLabeling::prepareGeometry( geom, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines );

if ( geom.isNull() )
return;
Expand All @@ -1672,9 +1672,9 @@ void QgsPalLayerSettings::registerFeature( const QgsFeature &f, QgsRenderContext

if ( isObstacle )
{
if ( !obstacleGeometry.isNull() && QgsPalLabeling::geometryRequiresPreparation( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry() ) )
if ( !obstacleGeometry.isNull() && QgsPalLabeling::geometryRequiresPreparation( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) )
{
obstacleGeometry = QgsGeometry( QgsPalLabeling::prepareGeometry( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry() ) );
obstacleGeometry = QgsGeometry( QgsPalLabeling::prepareGeometry( obstacleGeometry, context, ct, doClip ? extentGeom : QgsGeometry(), mergeLines ) );
}
}

Expand Down Expand Up @@ -2170,9 +2170,9 @@ void QgsPalLayerSettings::registerObstacleFeature( const QgsFeature &f, QgsRende
geos::unique_ptr geos_geom_clone;
std::unique_ptr<QgsGeometry> scopedPreparedGeom;

if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, extentGeom ) )
if ( QgsPalLabeling::geometryRequiresPreparation( geom, context, ct, extentGeom, mergeLines ) )
{
geom = QgsPalLabeling::prepareGeometry( geom, context, ct, extentGeom );
geom = QgsPalLabeling::prepareGeometry( geom, context, ct, extentGeom, mergeLines );
}
geos_geom_clone = QgsGeos::asGeos( geom );

Expand Down Expand Up @@ -2984,13 +2984,18 @@ bool QgsPalLabeling::staticWillUseLayer( QgsVectorLayer *layer )
}


bool QgsPalLabeling::geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry )
bool QgsPalLabeling::geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry, bool mergeLines )
{
if ( geometry.isNull() )
{
return false;
}

if ( geometry.type() == QgsWkbTypes::LineGeometry && geometry.isMultipart() && mergeLines )
{
return true;
}

//requires reprojection
if ( ct.isValid() && !ct.isShortCircuited() )
return true;
Expand Down Expand Up @@ -3056,7 +3061,7 @@ QStringList QgsPalLabeling::splitToGraphemes( const QString &text )
return graphemes;
}

QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry )
QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry, bool mergeLines )
{
if ( geometry.isNull() )
{
Expand All @@ -3066,6 +3071,11 @@ QgsGeometry QgsPalLabeling::prepareGeometry( const QgsGeometry &geometry, QgsRen
//don't modify the feature's geometry so that geometry based expressions keep working
QgsGeometry geom = geometry;

if ( geom.type() == QgsWkbTypes::LineGeometry && geom.isMultipart() && mergeLines )
{
geom = geom.mergeLines();
}

//reproject the geometry if necessary
if ( ct.isValid() && !ct.isShortCircuited() )
{
Expand Down
6 changes: 4 additions & 2 deletions src/core/qgspallabeling.h
Original file line number Diff line number Diff line change
Expand Up @@ -1023,21 +1023,23 @@ class CORE_EXPORT QgsPalLabeling
* \param context render context
* \param ct coordinate transform, or invalid transform if no transformation required
* \param clipGeometry geometry to clip features to, if applicable
* \param mergeLines TRUE if touching lines from this layer will be merged and treated as single features during labeling
* \returns prepared geometry
* \since QGIS 2.9
*/
static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() ) SIP_FACTORY;
static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false ) SIP_FACTORY;

/**
* Checks whether a geometry requires preparation before registration with PAL
* \param geometry geometry to prepare
* \param context render context
* \param ct coordinate transform, or invalid transform if no transformation required
* \param clipGeometry geometry to clip features to, if applicable
* \param mergeLines TRUE if touching lines from this layer will be merged and treated as single features during labeling
* \returns TRUE if geometry requires preparation
* \since QGIS 2.9
*/
static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry() );
static bool geometryRequiresPreparation( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform &ct, const QgsGeometry &clipGeometry = QgsGeometry(), bool mergeLines = false );

/**
* Splits a \a text string to a list of separate lines, using a specified wrap character (\a wrapCharacter).
Expand Down
57 changes: 56 additions & 1 deletion tests/src/core/testqgslabelingengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class TestQgsLabelingEngine : public QObject
void testRotateHidePartial();
void testParallelLabelSmallFeature();
void testAdjacentParts();
void testTouchingParts();
void testLabelBoundary();
void testLabelBlockingRegion();
void testLabelRotationWithReprojection();
Expand Down Expand Up @@ -811,7 +812,7 @@ void TestQgsLabelingEngine::testParallelLabelSmallFeature()

void TestQgsLabelingEngine::testAdjacentParts()
{
// test combination of map rotation with reprojected layer
// test polygon layer with multipart feature with adjacent parts
QgsPalLayerSettings settings;
setDefaultLabelParams( settings );

Expand Down Expand Up @@ -860,6 +861,60 @@ void TestQgsLabelingEngine::testAdjacentParts()
QVERIFY( imageCheck( QStringLiteral( "label_adjacent_parts" ), img, 20 ) );
}

void TestQgsLabelingEngine::testTouchingParts()
{
// test line layer with multipart feature with touching (but unmerged) parts
QgsPalLayerSettings settings;
setDefaultLabelParams( settings );

QgsTextFormat format = settings.format();
format.setSize( 20 );
format.setColor( QColor( 0, 0, 0 ) );
settings.setFormat( format );

settings.fieldName = QStringLiteral( "'XXXXXXXXXXXXXXXXXXXXXXXXXX'" );
settings.isExpression = true;
settings.placement = QgsPalLayerSettings::Curved;
settings.labelPerPart = false;
settings.mergeLines = true;

// if treated individually, none of these parts are long enough for the label to fit -- but the label should be rendered if the mergeLines setting is true,
// because the parts should be merged into a single linestring
std::unique_ptr< QgsVectorLayer> vl2( new QgsVectorLayer( QStringLiteral( "MultiLineString?crs=epsg:3946&field=id:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ) );
vl2->setRenderer( new QgsNullSymbolRenderer() );

QgsFeature f;
f.setAttributes( QgsAttributes() << 1 );
f.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "MultiLineString ((190000 5000010, 190050 5000000), (190050 5000000, 190100 5000000), (190200 5000000, 190150 5000000), (190150 5000000, 190100 5000000))" ) ) );
QVERIFY( vl2->dataProvider()->addFeature( f ) );

vl2->setLabeling( new QgsVectorLayerSimpleLabeling( settings ) ); // TODO: this should not be necessary!
vl2->setLabelsEnabled( true );

// make a fake render context
QSize size( 640, 480 );
QgsMapSettings mapSettings;
mapSettings.setDestinationCrs( vl2->crs() );

mapSettings.setOutputSize( size );
mapSettings.setExtent( f.geometry().boundingBox() );
mapSettings.setLayers( QList<QgsMapLayer *>() << vl2.get() );
mapSettings.setOutputDpi( 96 );

QgsLabelingEngineSettings engineSettings = mapSettings.labelingEngineSettings();
engineSettings.setFlag( QgsLabelingEngineSettings::UsePartialCandidates, false );
engineSettings.setFlag( QgsLabelingEngineSettings::DrawLabelRectOnly, true );
//engineSettings.setFlag( QgsLabelingEngineSettings::DrawCandidates, true );
mapSettings.setLabelingEngineSettings( engineSettings );

QgsMapRendererSequentialJob job( mapSettings );
job.start();
job.waitForFinished();

QImage img = job.renderedImage();
QVERIFY( imageCheck( QStringLiteral( "label_multipart_touching_lines" ), img, 20 ) );
}

void TestQgsLabelingEngine::testLabelBoundary()
{
// test that no labels are drawn outside of the specified label boundary
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 7213030

Please sign in to comment.