Skip to content

Commit

Permalink
[mvt] halo width must not be greater than 1/4 of text size (#58649)
Browse files Browse the repository at this point in the history
* [mvt] halo width must not be greater than 1/4 of text size

* fix halo size + add test

* fix layout

* Update src/core/vectortile/qgsmapboxglstyleconverter.cpp

---------

Co-authored-by: Nyall Dawson <nyall.dawson@gmail.com>
  • Loading branch information
3nids and nyalldawson authored Sep 18, 2024
1 parent 6163a81 commit ab75951
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 3 deletions.
36 changes: 34 additions & 2 deletions src/core/vectortile/qgsmapboxglstyleconverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1447,6 +1447,7 @@ void QgsMapBoxGlStyleConverter::parseSymbolLayer( const QVariantMap &jsonLayer,
if ( jsonPaint.contains( QStringLiteral( "text-halo-width" ) ) )
{
const QVariant jsonHaloWidth = jsonPaint.value( QStringLiteral( "text-halo-width" ) );
QString bufferSizeDataDefined;
switch ( jsonHaloWidth.userType() )
{
case QMetaType::Type::Int:
Expand All @@ -1457,19 +1458,50 @@ void QgsMapBoxGlStyleConverter::parseSymbolLayer( const QVariantMap &jsonLayer,

case QMetaType::Type::QVariantMap:
bufferSize = 1;
ddLabelProperties.setProperty( QgsPalLayerSettings::Property::BufferSize, parseInterpolateByZoom( jsonHaloWidth.toMap(), context, context.pixelSizeConversionFactor() * BUFFER_SIZE_SCALE, &bufferSize ) );
bufferSizeDataDefined = parseInterpolateByZoom( jsonHaloWidth.toMap(), context, context.pixelSizeConversionFactor() * BUFFER_SIZE_SCALE, &bufferSize ).asExpression();
break;

case QMetaType::Type::QVariantList:
case QMetaType::Type::QStringList:
bufferSize = 1;
ddLabelProperties.setProperty( QgsPalLayerSettings::Property::BufferSize, parseValueList( jsonHaloWidth.toList(), PropertyType::Numeric, context, context.pixelSizeConversionFactor() * BUFFER_SIZE_SCALE, 255, nullptr, &bufferSize ) );
bufferSizeDataDefined = parseValueList( jsonHaloWidth.toList(), PropertyType::Numeric, context, context.pixelSizeConversionFactor() * BUFFER_SIZE_SCALE, 255, nullptr, &bufferSize ).asExpression();
break;

default:
context.pushWarning( QObject::tr( "%1: Skipping unsupported text-halo-width type (%2)" ).arg( context.layerId(), QMetaType::typeName( static_cast<QMetaType::Type>( jsonHaloWidth.userType() ) ) ) );
break;
}

// from the specs halo should not be larger than 1/4 of the text-size
// https://docs.mapbox.com/style-spec/reference/layers/#paint-symbol-text-halo-width
if ( bufferSize > 0 )
{
if ( textSize > 0 && bufferSizeDataDefined.isEmpty() )
{
bufferSize = std::min( bufferSize, textSize * BUFFER_SIZE_SCALE / 4 );
}
else if ( textSize > 0 && !bufferSizeDataDefined.isEmpty() )
{
bufferSizeDataDefined = QStringLiteral( "min(%1/4, %2)" ).arg( textSize * BUFFER_SIZE_SCALE ).arg( bufferSizeDataDefined );
ddLabelProperties.setProperty( QgsPalLayerSettings::Property::BufferSize, QgsProperty::fromExpression( bufferSizeDataDefined ) );
}
else if ( !bufferSizeDataDefined.isEmpty() )
{
bufferSizeDataDefined = QStringLiteral( "min(%1*%2/4, %3)" )
.arg( textSizeProperty.asExpression() )
.arg( BUFFER_SIZE_SCALE )
.arg( bufferSizeDataDefined );
ddLabelProperties.setProperty( QgsPalLayerSettings::Property::BufferSize, QgsProperty::fromExpression( bufferSizeDataDefined ) );
}
else if ( bufferSizeDataDefined.isEmpty() )
{
bufferSizeDataDefined = QStringLiteral( "min(%1*%2/4, %3)" )
.arg( textSizeProperty.asExpression() )
.arg( BUFFER_SIZE_SCALE )
.arg( bufferSize );
ddLabelProperties.setProperty( QgsPalLayerSettings::Property::BufferSize, QgsProperty::fromExpression( bufferSizeDataDefined ) );
}
}
}

double haloBlurSize = 0;
Expand Down
55 changes: 54 additions & 1 deletion tests/src/python/test_qgsmapboxglconverter.py
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,6 @@ def testConvertLabels(self):
self.assertTrue(labeling.labelSettings().isExpression)

# text-transform

style = {
"layout": {
"text-field": "name_en",
Expand Down Expand Up @@ -761,6 +760,60 @@ def testConvertLabels(self):
'''lower(concat(concat("name_en",' - ',"name_fr"),"bar"))''')
self.assertTrue(labeling.labelSettings().isExpression)

def testHaloMaxSize(self):
# text-halo-width is max 1/4 of font-size
# https://docs.mapbox.com/style-spec/reference/layers/#paint-symbol-text-halo-width
context = QgsMapBoxGlStyleConversionContext()

# the pixel based text buffers appear larger when rendered on the web - so automatically scale
# them up when converting to a QGIS style
BUFFER_SIZE_SCALE = 2.0

# (text_size, halo_size, expected_size, expected_data_defined)
data = (
(16, 3, 3, None),
(16, 5, 4, None),
(12, ["get", "some_field_1"], None, 'min(24/4, "some_field_1")'),
(["get", "some_field_2"], 4, None, f'min("some_field_2"*{BUFFER_SIZE_SCALE:.0f}/4, {BUFFER_SIZE_SCALE * 4:.0f})'),
(["get", "some_field_3"], ["get", "some_field_4"], None, f'min("some_field_3"*{BUFFER_SIZE_SCALE:.0f}/4, "some_field_4")'),
)

for (text_size, halo_size, expected_size, expected_data_defined) in data:
style = {
"layout": {
"text-field": "name_en",
"text-font": [
"Open Sans Semibold",
"Arial Unicode MS Bold"
],
"text-transform": "uppercase",
"text-max-width": 8,
"text-anchor": "top",
"text-size": text_size,
"icon-size": 1
},
"type": "symbol",
"id": "poi_label",
"paint": {
"text-color": "#666",
"text-halo-width": halo_size,
"text-halo-color": "rgba(255,255,255,0.95)",
"text-halo-blur": 1
},
"source-layer": "poi_label"
}
renderer, has_renderer, labeling, has_labeling = QgsMapBoxGlStyleConverter.parseSymbolLayer(style, context)
self.assertFalse(has_renderer)
self.assertTrue(has_labeling)
if expected_size:
f = labeling.labelSettings().format()
buffer = f.buffer()
self.assertEqual(buffer.size(), expected_size * BUFFER_SIZE_SCALE)
if expected_data_defined:
ls = labeling.labelSettings()
dd = ls.dataDefinedProperties()
self.assertEqual(dd.property(QgsPalLayerSettings.Property.BufferSize).asExpression(), expected_data_defined)

def testFontFamilyReplacement(self):
context = QgsMapBoxGlStyleConversionContext()
style = {
Expand Down

0 comments on commit ab75951

Please sign in to comment.