Skip to content

Commit

Permalink
[pal] Never treat features as obstacles for their own labels
Browse files Browse the repository at this point in the history
Fixes issues like rule based labelling registering two labels for
a single feature which was resulting in each label colliding
with the feature's geometry registered by the other label. This
resulted in poor placement for these labels.

Sponsored by City of Uster
  • Loading branch information
nyalldawson committed Nov 21, 2015
1 parent 86ad564 commit 03f9e1b
Show file tree
Hide file tree
Showing 13 changed files with 431 additions and 4 deletions.
8 changes: 8 additions & 0 deletions src/core/pal/feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ namespace pal
return mLF->id();
}

bool FeaturePart::hasSameLabelFeatureAs( FeaturePart* part ) const
{
if ( !part )
return false;

return mLF->id() == part->featureId() && mLF->layer()->name() == part->layer()->name();
}

LabelPosition::Quadrant FeaturePart::quadrantFromOffset() const
{
QPointF quadOffset = mLF->quadOffset();
Expand Down
6 changes: 6 additions & 0 deletions src/core/pal/feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ namespace pal
*/
QgsFeatureId featureId() const;

/** Tests whether this feature part belongs to the same QgsLabelFeature as another
* feature part.
* @param part part to compare to
* @returns true if both parts belong to same QgsLabelFeature
*/
bool hasSameLabelFeatureAs( FeaturePart* part ) const;

#if 0
/**
Expand Down
14 changes: 10 additions & 4 deletions src/core/pal/labelposition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,16 +441,22 @@ namespace pal

//////////

bool LabelPosition::pruneCallback( LabelPosition *lp, void *ctx )
bool LabelPosition::pruneCallback( LabelPosition *candidatePosition, void *ctx )
{
FeaturePart *feat = (( PruneCtx* ) ctx )->obstacle;
FeaturePart *obstaclePart = (( PruneCtx* ) ctx )->obstacle;

if (( feat == lp->feature ) || ( feat->getHoleOf() && feat->getHoleOf() != lp->feature ) )
// test whether we should ignore this obstacle for the candidate. We do this if:
// 1. it's not a hole, and the obstacle belongs to the same label feature as the candidate (eg
// features aren't obstacles for their own labels)
// 2. it IS a hole, and the hole belongs to a different label feature to the candidate (eg, holes
// are ONLY obstacles for the labels of the feature they belong to)
if (( !obstaclePart->getHoleOf() && candidatePosition->feature->hasSameLabelFeatureAs( obstaclePart ) )
|| ( obstaclePart->getHoleOf() && !candidatePosition->feature->hasSameLabelFeatureAs( dynamic_cast< FeaturePart* >( obstaclePart->getHoleOf() ) ) ) )
{
return true;
}

CostCalculator::addObstacleCostPenalty( lp, feat );
CostCalculator::addObstacleCostPenalty( candidatePosition, obstaclePart );

return true;
}
Expand Down
34 changes: 34 additions & 0 deletions tests/src/python/test_qgspallabeling_placement.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,40 @@ def test_point_placement_around_obstacle_large_symbol(self):
self.removeMapLayer(self.layer)
self.layer = None

def test_polygon_placement_with_hole(self):
# Horizontal label placement for polygon with hole
# Note for this test, the mask is used to check only pixels outside of the polygon.
# We don't care where in the polygon the label is, just that it
# is INSIDE the polygon
self.layer = TestQgsPalLabeling.loadFeatureLayer('polygon_with_hole')
self._TestMapSettings = self.cloneMapSettings(self._MapSettings)
self.lyr.placement = QgsPalLayerSettings.Horizontal
self.checkTest()
self.removeMapLayer(self.layer)
self.layer = None

def test_polygon_placement_with_hole_and_point(self):
# Testing that hole from a feature is not treated as an obstacle for other feature's labels
self.layer = TestQgsPalLabeling.loadFeatureLayer('point')
polyLayer = TestQgsPalLabeling.loadFeatureLayer('polygon_with_hole')
self._TestMapSettings = self.cloneMapSettings(self._MapSettings)
self.checkTest()
self.removeMapLayer(self.layer)
self.removeMapLayer(polyLayer)
self.layer = None

def test_polygon_multiple_labels(self):
# Horizontal label placement for polygon with hole
# Note for this test, the mask is used to check only pixels outside of the polygon.
# We don't care where in the polygon the label is, just that it
# is INSIDE the polygon
self.layer = TestQgsPalLabeling.loadFeatureLayer('polygon_rule_based')
self._TestMapSettings = self.cloneMapSettings(self._MapSettings)
self.lyr.placement = QgsPalLayerSettings.Horizontal
self.checkTest()
self.removeMapLayer(self.layer)
self.layer = None

if __name__ == '__main__':
# NOTE: unless PAL_SUITE env var is set all test class methods will be run
# SEE: test_qgspallabeling_tests.suiteTests() to define suite
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/testdata/labeling/pal_features_v3.sqlite
Binary file not shown.
128 changes: 128 additions & 0 deletions tests/testdata/labeling/polygon_rule_based.qml
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
<!DOCTYPE qgis PUBLIC 'http://mrcc.com/qgis.dtd' 'SYSTEM'>
<qgis version="2.13.0-Master" minimumScale="0" maximumScale="1e+08" simplifyDrawingHints="1" minLabelScale="0" maxLabelScale="1e+08" simplifyDrawingTol="1" simplifyMaxScale="1" hasScaleBasedVisibilityFlag="0" simplifyLocal="1" scaleBasedLabelVisibilityFlag="0">
<edittypes>
<edittype widgetv2type="TextEdit" name="pkuid">
<widgetv2config IsMultiline="0" fieldEditable="1" UseHtml="0" labelOnTop="0"/>
</edittype>
<edittype widgetv2type="TextEdit" name="ftype">
<widgetv2config IsMultiline="0" fieldEditable="1" UseHtml="0" labelOnTop="0"/>
</edittype>
<edittype widgetv2type="TextEdit" name="text">
<widgetv2config IsMultiline="0" fieldEditable="1" UseHtml="0" labelOnTop="0"/>
</edittype>
</edittypes>
<renderer-v2 forceraster="0" symbollevels="0" type="singleSymbol">
<symbols>
<symbol alpha="1" clip_to_extent="1" type="fill" name="0">
<layer pass="0" class="SimpleFill" locked="0">
<prop k="border_width_map_unit_scale" v="0,0,0,0,0,0"/>
<prop k="color" v="133,149,161,255"/>
<prop k="joinstyle" v="bevel"/>
<prop k="offset" v="0,0"/>
<prop k="offset_map_unit_scale" v="0,0,0,0,0,0"/>
<prop k="offset_unit" v="MM"/>
<prop k="outline_color" v="0,0,0,255"/>
<prop k="outline_style" v="no"/>
<prop k="outline_width" v="0.26"/>
<prop k="outline_width_unit" v="MM"/>
<prop k="style" v="solid"/>
</layer>
</symbol>
</symbols>
<rotation/>
<sizescale scalemethod="diameter"/>
</renderer-v2>
<labeling type="rule-based">
<rules>
<rule>
<settings>
<text-style fontItalic="1" fontFamily="Ubuntu" fontLetterSpacing="0" fontUnderline="0" fontSizeMapUnitMaxScale="0" fontWeight="63" fontStrikeout="0" textTransp="0" previewBkgrdColor="#ffffff" fontCapitals="0" textColor="0,0,0,255" fontSizeMapUnitMinScale="0" fontSizeInMapUnits="0" isExpression="0" blendMode="0" fontSize="11" fieldName="text" namedStyle="Medium Italic" fontWordSpacing="0"/>
<text-format placeDirectionSymbol="0" multilineAlign="0" rightDirectionSymbol=">" multilineHeight="1" plussign="0" addDirectionSymbol="0" leftDirectionSymbol="&lt;" formatNumbers="0" decimals="3" wrapChar="" reverseDirectionSymbol="0"/>
<text-buffer bufferSize="1" bufferSizeMapUnitMinScale="0" bufferColor="255,255,255,255" bufferDraw="0" bufferBlendMode="0" bufferTransp="0" bufferSizeInMapUnits="0" bufferSizeMapUnitMaxScale="0" bufferNoFill="0" bufferJoinStyle="64"/>
<background shapeSizeUnits="1" shapeType="0" shapeOffsetMapUnitMinScale="0" shapeSizeMapUnitMinScale="0" shapeSVGFile="" shapeOffsetX="0" shapeOffsetY="0" shapeBlendMode="0" shapeBorderWidthMapUnitMaxScale="0" shapeFillColor="255,255,255,255" shapeTransparency="0" shapeSizeType="0" shapeJoinStyle="64" shapeDraw="0" shapeSizeMapUnitMaxScale="0" shapeBorderWidthUnits="1" shapeSizeX="0" shapeSizeY="0" shapeRadiiX="0" shapeOffsetMapUnitMaxScale="0" shapeOffsetUnits="1" shapeRadiiY="0" shapeRotation="0" shapeBorderWidth="0" shapeRadiiMapUnitMinScale="0" shapeRadiiMapUnitMaxScale="0" shapeBorderColor="128,128,128,255" shapeRotationType="0" shapeRadiiUnits="1" shapeBorderWidthMapUnitMinScale="0"/>
<shadow shadowOffsetGlobal="1" shadowRadiusUnits="1" shadowRadiusMapUnitMinScale="0" shadowTransparency="30" shadowColor="0,0,0,255" shadowUnder="0" shadowScale="100" shadowOffsetDist="1" shadowOffsetMapUnitMinScale="0" shadowRadiusMapUnitMaxScale="0" shadowDraw="0" shadowOffsetAngle="135" shadowRadius="1.5" shadowBlendMode="6" shadowOffsetMapUnitMaxScale="0" shadowRadiusAlphaOnly="0" shadowOffsetUnits="1"/>
<placement repeatDistanceUnit="1" placement="1" maxCurvedCharAngleIn="20" repeatDistance="0" distMapUnitMaxScale="0" labelOffsetMapUnitMaxScale="0" distInMapUnits="0" labelOffsetInMapUnits="0" xOffset="0" preserveRotation="1" centroidWhole="0" priority="5" repeatDistanceMapUnitMaxScale="0" yOffset="-2" placementFlags="10" repeatDistanceMapUnitMinScale="0" centroidInside="0" dist="0" angleOffset="0" maxCurvedCharAngleOut="-20" fitInPolygonOnly="0" quadOffset="1" distMapUnitMinScale="0" labelOffsetMapUnitMinScale="0"/>
<rendering fontMinPixelSize="3" scaleMax="10000000" fontMaxPixelSize="10000" scaleMin="1" upsidedownLabels="0" limitNumLabels="0" obstacle="1" obstacleFactor="1" scaleVisibility="0" fontLimitPixelSize="0" mergeLines="0" obstacleType="0" labelPerPart="0" maxNumLabels="2000" displayAll="0" minFeatureSize="0"/>
<data-defined/>
</settings>
</rule>
<rule>
<settings>
<text-style fontItalic="1" fontFamily="Ubuntu" fontLetterSpacing="0" fontUnderline="0" fontSizeMapUnitMaxScale="0" fontWeight="63" fontStrikeout="0" textTransp="0" previewBkgrdColor="#ffffff" fontCapitals="0" textColor="0,0,0,255" fontSizeMapUnitMinScale="0" fontSizeInMapUnits="0" isExpression="0" blendMode="0" fontSize="11" fieldName="text" namedStyle="Medium Italic" fontWordSpacing="0"/>
<text-format placeDirectionSymbol="0" multilineAlign="0" rightDirectionSymbol=">" multilineHeight="1" plussign="0" addDirectionSymbol="0" leftDirectionSymbol="&lt;" formatNumbers="0" decimals="3" wrapChar="" reverseDirectionSymbol="0"/>
<text-buffer bufferSize="1" bufferSizeMapUnitMinScale="0" bufferColor="255,255,255,255" bufferDraw="0" bufferBlendMode="0" bufferTransp="0" bufferSizeInMapUnits="0" bufferSizeMapUnitMaxScale="0" bufferNoFill="0" bufferJoinStyle="64"/>
<background shapeSizeUnits="1" shapeType="0" shapeOffsetMapUnitMinScale="0" shapeSizeMapUnitMinScale="0" shapeSVGFile="" shapeOffsetX="0" shapeOffsetY="0" shapeBlendMode="0" shapeBorderWidthMapUnitMaxScale="0" shapeFillColor="255,255,255,255" shapeTransparency="0" shapeSizeType="0" shapeJoinStyle="64" shapeDraw="0" shapeSizeMapUnitMaxScale="0" shapeBorderWidthUnits="1" shapeSizeX="0" shapeSizeY="0" shapeRadiiX="0" shapeOffsetMapUnitMaxScale="0" shapeOffsetUnits="1" shapeRadiiY="0" shapeRotation="0" shapeBorderWidth="0" shapeRadiiMapUnitMinScale="0" shapeRadiiMapUnitMaxScale="0" shapeBorderColor="128,128,128,255" shapeRotationType="0" shapeRadiiUnits="1" shapeBorderWidthMapUnitMinScale="0"/>
<shadow shadowOffsetGlobal="1" shadowRadiusUnits="1" shadowRadiusMapUnitMinScale="0" shadowTransparency="30" shadowColor="0,0,0,255" shadowUnder="0" shadowScale="100" shadowOffsetDist="1" shadowOffsetMapUnitMinScale="0" shadowRadiusMapUnitMaxScale="0" shadowDraw="0" shadowOffsetAngle="135" shadowRadius="1.5" shadowBlendMode="6" shadowOffsetMapUnitMaxScale="0" shadowRadiusAlphaOnly="0" shadowOffsetUnits="1"/>
<placement repeatDistanceUnit="1" placement="1" maxCurvedCharAngleIn="20" repeatDistance="0" distMapUnitMaxScale="0" labelOffsetMapUnitMaxScale="0" distInMapUnits="0" labelOffsetInMapUnits="0" xOffset="0" preserveRotation="1" centroidWhole="0" priority="5" repeatDistanceMapUnitMaxScale="0" yOffset="2" placementFlags="10" repeatDistanceMapUnitMinScale="0" centroidInside="0" dist="0" angleOffset="0" maxCurvedCharAngleOut="-20" fitInPolygonOnly="0" quadOffset="7" distMapUnitMinScale="0" labelOffsetMapUnitMinScale="0"/>
<rendering fontMinPixelSize="3" scaleMax="10000000" fontMaxPixelSize="10000" scaleMin="1" upsidedownLabels="0" limitNumLabels="0" obstacle="1" obstacleFactor="1" scaleVisibility="0" fontLimitPixelSize="0" mergeLines="0" obstacleType="0" labelPerPart="0" maxNumLabels="2000" displayAll="0" minFeatureSize="0"/>
<data-defined/>
</settings>
</rule>
</rules>
</labeling>
<customproperties>
<property key="variableNames" value="_fields_"/>
<property key="variableValues" value=""/>
</customproperties>
<blendMode>0</blendMode>
<featureBlendMode>0</featureBlendMode>
<layerTransparency>0</layerTransparency>
<displayfield>pkuid</displayfield>
<label>0</label>
<labelattributes>
<label fieldname="" text="Label"/>
<family fieldname="" name="Ubuntu"/>
<size fieldname="" units="pt" value="12"/>
<bold fieldname="" on="0"/>
<italic fieldname="" on="0"/>
<underline fieldname="" on="0"/>
<strikeout fieldname="" on="0"/>
<color fieldname="" red="0" blue="0" green="0"/>
<x fieldname=""/>
<y fieldname=""/>
<offset x="0" y="0" units="pt" yfieldname="" xfieldname=""/>
<angle fieldname="" value="0" auto="0"/>
<alignment fieldname="" value="center"/>
<buffercolor fieldname="" red="255" blue="255" green="255"/>
<buffersize fieldname="" units="pt" value="1"/>
<bufferenabled fieldname="" on=""/>
<multilineenabled fieldname="" on=""/>
<selectedonly on=""/>
</labelattributes>
<SingleCategoryDiagramRenderer diagramType="Pie">
<DiagramCategory penColor="#000000" labelPlacementMethod="XHeight" penWidth="0" diagramOrientation="Up" minimumSize="0" barWidth="5" penAlpha="255" maxScaleDenominator="1e+08" backgroundColor="#ffffff" transparency="0" width="15" scaleDependency="Area" backgroundAlpha="255" angleOffset="1440" scaleBasedVisibility="0" enabled="0" height="15" sizeType="MM" minScaleDenominator="0">
<fontProperties description="Ubuntu,11,-1,5,50,0,0,0,0,0" style=""/>
</DiagramCategory>
</SingleCategoryDiagramRenderer>
<DiagramLayerSettings yPosColumn="-1" linePlacementFlags="10" placement="0" dist="0" xPosColumn="-1" priority="0" obstacle="0" showAll="1"/>
<editform></editform>
<editforminit/>
<editforminitusecode>0</editforminitusecode>
<editforminitcode><![CDATA[# -*- coding: utf-8 -*-
"""
QGIS forms can have a Python function that is called when the form is
opened.
Use this function to add extra logic to your forms.
Enter the name of the function in the "Python Init function"
field.
An example follows:
"""
from PyQt4.QtGui import QWidget

def my_form_open(dialog, layer, feature):
geom = feature.geometry()
control = dialog.findChild(QWidget, "MyLineEdit")
]]></editforminitcode>
<featformsuppress>0</featformsuppress>
<annotationform></annotationform>
<editorlayout>generatedlayout</editorlayout>
<excludeAttributesWMS/>
<excludeAttributesWFS/>
<attributeactions/>
<conditionalstyles>
<rowstyles/>
<fieldstyles/>
</conditionalstyles>
</qgis>

0 comments on commit 03f9e1b

Please sign in to comment.