From ab75951665075a07b4abe05048b4f2cbce46832f Mon Sep 17 00:00:00 2001 From: Denis Rouzaud Date: Wed, 18 Sep 2024 06:00:46 +0200 Subject: [PATCH] [mvt] halo width must not be greater than 1/4 of text size (#58649) * [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 --- .../vectortile/qgsmapboxglstyleconverter.cpp | 36 +++++++++++- tests/src/python/test_qgsmapboxglconverter.py | 55 ++++++++++++++++++- 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/core/vectortile/qgsmapboxglstyleconverter.cpp b/src/core/vectortile/qgsmapboxglstyleconverter.cpp index a9508621844f..c52b51440533 100644 --- a/src/core/vectortile/qgsmapboxglstyleconverter.cpp +++ b/src/core/vectortile/qgsmapboxglstyleconverter.cpp @@ -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: @@ -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( 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; diff --git a/tests/src/python/test_qgsmapboxglconverter.py b/tests/src/python/test_qgsmapboxglconverter.py index ca6134ce8bd1..50b6cd7e3a67 100644 --- a/tests/src/python/test_qgsmapboxglconverter.py +++ b/tests/src/python/test_qgsmapboxglconverter.py @@ -698,7 +698,6 @@ def testConvertLabels(self): self.assertTrue(labeling.labelSettings().isExpression) # text-transform - style = { "layout": { "text-field": "name_en", @@ -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 = {