Skip to content

Commit

Permalink
[mvt] fix line dash arrays using literals (#58857)
Browse files Browse the repository at this point in the history 
* [mvt] fix line dash arrays using literals

* Update src/core/vectortile/qgsmapboxglstyleconverter.h

Co-authored-by: Nyall Dawson <nyall.dawson@gmail.com>

---------

Co-authored-by: Nyall Dawson <nyall.dawson@gmail.com>
  • Loading branch information
@3nids @nyalldawson
3nids and nyalldawson authored Sep 25, 2024
1 parent b5d1779 commit 6d4e343
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 34 deletions.
2 changes: 2 additions & 0 deletions python/PyQt6/core/auto_additions/qgsmapboxglstyleconverter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
QgsMapBoxGlStyleConverter.PropertyType.Numeric.__doc__ = "Numeric property (e.g. line width, text size)"
QgsMapBoxGlStyleConverter.PropertyType.Opacity.__doc__ = "Opacity property"
QgsMapBoxGlStyleConverter.PropertyType.Point.__doc__ = "Point/offset property"
QgsMapBoxGlStyleConverter.PropertyType.NumericArray.__doc__ = "Numeric array for dash arrays ot such"
QgsMapBoxGlStyleConverter.PropertyType.__doc__ = """Property types, for interpolated value conversion
.. warning::
Expand All @@ -16,6 +17,7 @@
* ``Numeric``: Numeric property (e.g. line width, text size)
* ``Opacity``: Opacity property
* ``Point``: Point/offset property
* ``NumericArray``: Numeric array for dash arrays ot such
"""
# --
Expand Down
1 change: 1 addition & 0 deletions python/PyQt6/core/auto_generated/vectortile/qgsmapboxglstyleconverter.sip.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ Constructor for QgsMapBoxGlStyleConverter.
Numeric,
Opacity,
Point,
NumericArray,
};

Result convert( const QVariantMap &style, QgsMapBoxGlStyleConversionContext *context = 0 );
Expand Down
2 changes: 2 additions & 0 deletions python/core/auto_additions/qgsmapboxglstyleconverter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
QgsMapBoxGlStyleConverter.PropertyType.Numeric.__doc__ = "Numeric property (e.g. line width, text size)"
QgsMapBoxGlStyleConverter.PropertyType.Opacity.__doc__ = "Opacity property"
QgsMapBoxGlStyleConverter.PropertyType.Point.__doc__ = "Point/offset property"
QgsMapBoxGlStyleConverter.PropertyType.NumericArray.__doc__ = "Numeric array for dash arrays ot such"
QgsMapBoxGlStyleConverter.PropertyType.__doc__ = """Property types, for interpolated value conversion
.. warning::
Expand All @@ -14,6 +15,7 @@
* ``Numeric``: Numeric property (e.g. line width, text size)
* ``Opacity``: Opacity property
* ``Point``: Point/offset property
* ``NumericArray``: Numeric array for dash arrays ot such
"""
# --
Expand Down
1 change: 1 addition & 0 deletions python/core/auto_generated/vectortile/qgsmapboxglstyleconverter.sip.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ Constructor for QgsMapBoxGlStyleConverter.
Numeric,
Opacity,
Point,
NumericArray,
};

Result convert( const QVariantMap &style, QgsMapBoxGlStyleConversionContext *context = 0 );
Expand Down
130 changes: 96 additions & 34 deletions src/core/vectortile/qgsmapboxglstyleconverter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,48 +699,64 @@ bool QgsMapBoxGlStyleConverter::parseLineLayer( const QVariantMap &jsonLayer, Qg
{
const QVariantList dashSource = jsonLineDashArray.toList();

QVector< double > rawDashVectorSizes;
rawDashVectorSizes.reserve( dashSource.size() );
for ( const QVariant &v : dashSource )
{
rawDashVectorSizes << v.toDouble();
}

// handle non-compliant dash vector patterns
if ( rawDashVectorSizes.size() == 1 )
if ( dashSource.at( 0 ).userType() == QMetaType::Type::QString )
{
// match behavior of MapBox style rendering -- if a user makes a line dash array with one element, it's ignored
rawDashVectorSizes.clear();
QgsProperty property = parseValueList( dashSource, PropertyType::NumericArray, context, 1, 255, nullptr, nullptr );
if ( !lineWidthProperty.asExpression().isEmpty() )
{
property = QgsProperty::fromExpression( QStringLiteral( "array_to_string(array_foreach(%1,@element * (%2)), ';')" ) // skip-keyword-check
.arg( property.asExpression(), lineWidthProperty.asExpression() ) );
}
else
{
property = QgsProperty::fromExpression( QStringLiteral( "array_to_string(%1, ';')" ).arg( property.asExpression() ) );
}
ddProperties.setProperty( QgsSymbolLayer::Property::CustomDash, property );
}
else if ( rawDashVectorSizes.size() % 2 == 1 )
else
{
// odd number of dash pattern sizes -- this isn't permitted by Qt/QGIS, but isn't explicitly blocked by the MapBox specs
// MapBox seems to add the extra dash element to the first dash size
rawDashVectorSizes[0] = rawDashVectorSizes[0] + rawDashVectorSizes[rawDashVectorSizes.size() - 1];
rawDashVectorSizes.resize( rawDashVectorSizes.size() - 1 );
}
QVector< double > rawDashVectorSizes;
rawDashVectorSizes.reserve( dashSource.size() );
for ( const QVariant &v : dashSource )
{
rawDashVectorSizes << v.toDouble();
}

if ( !rawDashVectorSizes.isEmpty() && ( !lineWidthProperty.asExpression().isEmpty() ) )
{
QStringList dashArrayStringParts;
dashArrayStringParts.reserve( rawDashVectorSizes.size() );
for ( double v : std::as_const( rawDashVectorSizes ) )
// handle non-compliant dash vector patterns
if ( rawDashVectorSizes.size() == 1 )
{
dashArrayStringParts << qgsDoubleToString( v );
// match behavior of MapBox style rendering -- if a user makes a line dash array with one element, it's ignored
rawDashVectorSizes.clear();
}
else if ( rawDashVectorSizes.size() % 2 == 1 )
{
// odd number of dash pattern sizes -- this isn't permitted by Qt/QGIS, but isn't explicitly blocked by the MapBox specs
// MapBox seems to add the extra dash element to the first dash size
rawDashVectorSizes[0] = rawDashVectorSizes[0] + rawDashVectorSizes[rawDashVectorSizes.size() - 1];
rawDashVectorSizes.resize( rawDashVectorSizes.size() - 1 );
}

QString arrayExpression = QStringLiteral( "array_to_string(array_foreach(array(%1),@element * (%2)), ';')" ) // skip-keyword-check
.arg( dashArrayStringParts.join( ',' ),
lineWidthProperty.asExpression() );
ddProperties.setProperty( QgsSymbolLayer::Property::CustomDash, QgsProperty::fromExpression( arrayExpression ) );
}
if ( !rawDashVectorSizes.isEmpty() && ( !lineWidthProperty.asExpression().isEmpty() ) )
{
QStringList dashArrayStringParts;
dashArrayStringParts.reserve( rawDashVectorSizes.size() );
for ( double v : std::as_const( rawDashVectorSizes ) )
{
dashArrayStringParts << qgsDoubleToString( v );
}

// dash vector sizes for QGIS symbols must be multiplied by the target line width
for ( double v : std::as_const( rawDashVectorSizes ) )
{
dashVector << v *lineWidth;
}
QString arrayExpression = QStringLiteral( "array_to_string(array_foreach(array(%1),@element * (%2)), ';')" ) // skip-keyword-check
.arg( dashArrayStringParts.join( ',' ),
lineWidthProperty.asExpression() );
ddProperties.setProperty( QgsSymbolLayer::Property::CustomDash, QgsProperty::fromExpression( arrayExpression ) );
}

// dash vector sizes for QGIS symbols must be multiplied by the target line width
for ( double v : std::as_const( rawDashVectorSizes ) )
{
dashVector << v *lineWidth;
}
}
break;
}

Expand Down Expand Up @@ -2853,6 +2869,19 @@ QgsProperty QgsMapBoxGlStyleConverter::parseMatchList( const QVariantList &json,
value.toList().value( 0 ).toDouble() * multiplier );
break;
}

case PropertyType::NumericArray:
{
if ( value.toList().count() == 2 && value.toList().first().toString() == QLatin1String( "literal" ) )
{
valueString = QStringLiteral( "array(%1)" ).arg( value.toList().at( 1 ).toStringList().join( ',' ) );
}
else
{
valueString = QStringLiteral( "array(%1)" ).arg( value.toStringList().join( ',' ) );
}
break;
}
}

if ( matchString.count() == 1 )
Expand Down Expand Up @@ -2915,6 +2944,19 @@ QgsProperty QgsMapBoxGlStyleConverter::parseMatchList( const QVariantList &json,
break;
}

case PropertyType::NumericArray:
{
if ( json.constLast().toList().count() == 2 && json.constLast().toList().first().toString() == QLatin1String( "literal" ) )
{
elseValue = QStringLiteral( "array(%1)" ).arg( json.constLast().toList().at( 1 ).toStringList().join( ',' ) );
}
else
{
elseValue = QStringLiteral( "array(%1)" ).arg( json.constLast().toStringList().join( ',' ) );
}
break;
}

}
break;
}
Expand All @@ -2941,7 +2983,9 @@ QgsProperty QgsMapBoxGlStyleConverter::parseStepList( const QVariantList &json,
const QVariant stepValue = json.value( i + 1 );

QString valueString;
if ( stepValue.canConvert<QVariantList>() && ( stepValue.toList().count() != 2 || type != PropertyType::Point ) )
if ( stepValue.canConvert<QVariantList>()
&& ( stepValue.toList().count() != 2 || type != PropertyType::Point )
&& type != PropertyType::NumericArray )
{
valueString = parseValueList( stepValue.toList(), type, context, multiplier, maxOpacity, defaultColor, defaultNumber ).expressionString();
}
Expand Down Expand Up @@ -2978,6 +3022,19 @@ QgsProperty QgsMapBoxGlStyleConverter::parseStepList( const QVariantList &json,
);
break;
}

case PropertyType::NumericArray:
{
if ( stepValue.toList().count() == 2 && stepValue.toList().first().toString() == QLatin1String( "literal" ) )
{
valueString = QStringLiteral( "array(%1)" ).arg( stepValue.toList().at( 1 ).toStringList().join( ',' ) );
}
else
{
valueString = QStringLiteral( "array(%1)" ).arg( stepValue.toStringList().join( ',' ) );
}
break;
}
}
}

Expand Down Expand Up @@ -3048,6 +3105,11 @@ QgsProperty QgsMapBoxGlStyleConverter::parseInterpolateListByZoom( const QVarian

case PropertyType::Point:
return parseInterpolatePointByZoom( props, context, multiplier );

case PropertyType::NumericArray:
context.pushWarning( QObject::tr( "%1: Skipping unsupported numeric array in interpolate" ).arg( context.layerId() ) );
return QgsProperty();

}
return QgsProperty();
}
Expand Down
1 change: 1 addition & 0 deletions src/core/vectortile/qgsmapboxglstyleconverter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ class CORE_EXPORT QgsMapBoxGlStyleConverter
Numeric, //!< Numeric property (e.g. line width, text size)
Opacity, //!< Opacity property
Point, //!< Point/offset property
NumericArray, //!< Numeric array for dash arrays or such
};
Q_ENUM( PropertyType )

Expand Down
45 changes: 45 additions & 0 deletions tests/src/python/test_qgsmapboxglconverter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,51 @@ def testParseLineDashArraySingleNumber(self):
'CASE WHEN @vector_tile_zoom >= 10 AND @vector_tile_zoom <= 11 THEN (1.5) + ((1.2^(@vector_tile_zoom - 10) - 1) / (1.2^(11 - 10) - 1)) * ((2) - (1.5)) WHEN @vector_tile_zoom > 11 AND @vector_tile_zoom <= 12 THEN (2) + ((1.2^(@vector_tile_zoom - 11) - 1) / (1.2^(12 - 11) - 1)) * ((3) - (2)) WHEN @vector_tile_zoom > 12 AND @vector_tile_zoom <= 13 THEN (3) + ((1.2^(@vector_tile_zoom - 12) - 1) / (1.2^(13 - 12) - 1)) * ((5) - (3)) WHEN @vector_tile_zoom > 13 AND @vector_tile_zoom <= 14 THEN (5) + ((1.2^(@vector_tile_zoom - 13) - 1) / (1.2^(14 - 13) - 1)) * ((6) - (5)) WHEN @vector_tile_zoom > 14 AND @vector_tile_zoom <= 16 THEN (6) + ((1.2^(@vector_tile_zoom - 14) - 1) / (1.2^(16 - 14) - 1)) * ((10) - (6)) WHEN @vector_tile_zoom > 16 AND @vector_tile_zoom <= 17 THEN (10) + ((1.2^(@vector_tile_zoom - 16) - 1) / (1.2^(17 - 16) - 1)) * ((12) - (10)) WHEN @vector_tile_zoom > 17 THEN 12 END')
self.assertFalse(dd_properties.property(QgsSymbolLayer.Property.PropertyCustomDash).isActive())

def testParseLineDashArrayLiteral(self):
conversion_context = QgsMapBoxGlStyleConversionContext()
style = {
"id": "tunnel_public_transport",
"type": "line",
"source": "base_v1.0.0",
"source-layer": "transportation",
"minzoom": 8.0,
"layout": {"line-cap": "butt", "line-join": "miter", "visibility": "visible"},
"paint": {
"line-blur": 0.4,
"line-color": "hsl(0,80%,60%)",
"line-width": [
"interpolate", ["linear"], ["zoom"], 8, 0.5, 10, 1.2, 12, ["match", ["get", "class"], ["rail"], ["match", ["get", "subclass"], ["rail", "narrow_gauge", "rack_rail"], ["match", ["get", "service"], ["yard", "siding"], 0.25, 1], 1], 1], 14, ["match", ["get", "class"], ["rail", "rail_construction"], ["match", ["get", "subclass"], ["rail", "narrow_gauge", "rack_rail"], ["match", ["get", "service"], ["yard", "siding"], 0.25, 1.5], 1.5], 1.5], 18,
["match", ["get", "class"], ["rail", "rail_construction"], ["match", ["get", "subclass"], ["rail", "narrow_gauge", "rack_rail"], ["match", ["get", "service"], ["yard", "siding"], 1, 2], 1.5], 1.5]
],
"line-opacity": [
"interpolate", ["linear"], ["zoom"], 8, 0, 8.5, ["match", ["get", "class"], ["rail"], 1, 0], 13, ["match", ["get", "subclass"], ["rail", "subway", "funicular", "narrow_gauge", "rack_rail"], ["match", ["get", "is_route"], 99, 1, 0], 0], 14,
["match", ["get", "class"], ["rail_construction", "transit_construction"], 0.8, ["match", ["get", "subclass"], ["rail", "narrow_gauge", "funicular", "subway", "rack_rail"], ["match", ["get", "service"], ["yard", "siding"], 0, 1], 0]], 14.5, ["match", ["get", "class"], ["rail_construction", "transit_construction"], 0.8, 1]
],
"line-dasharray": ["step", ["zoom"], ["literal", [3, 1.875]], 14, ["literal", [4, 2.5]], 15, ["literal", [5, 3.125]], 16, ["literal", [6, 3.75]]]
},
"filter": ["all", ["==", ["get", "brunnel"], "tunnel"], ["in", ["get", "class"], ["literal", ["cable_car", "gondola", "rail", "rail_construction", "transit", "transit_construction"]]], ["==", ["geometry-type"], "LineString"]]
}
has_renderer, rendererStyle = QgsMapBoxGlStyleConverter.parseLineLayer(style, conversion_context)
self.assertTrue(has_renderer)
self.assertEqual(rendererStyle.geometryType(), QgsWkbTypes.GeometryType.LineGeometry)
self.assertFalse(rendererStyle.symbol()[0].useCustomDashPattern())
dd_properties = rendererStyle.symbol().symbolLayers()[0].dataDefinedProperties()
self.assertEqual(dd_properties.property(QgsSymbolLayer.Property.CustomDash).asExpression(),
'''array_to_string(array_foreach(CASE WHEN @vector_tile_zoom >= 16 THEN (array(6,3.75)) WHEN @vector_tile_zoom >= 15 THEN (array(5,3.125)) WHEN @vector_tile_zoom >= 14 THEN (array(4,2.5)) ELSE (array(3,1.875)) END,@element * (CASE WHEN @vector_tile_zoom >= 8 AND @vector_tile_zoom <= 10 THEN scale_linear(@vector_tile_zoom,8,10,0.5,1.2) WHEN @vector_tile_zoom > 10 AND @vector_tile_zoom <= 12 THEN scale_linear(@vector_tile_zoom,10,12,1.2,CASE WHEN "class" = 'rail' THEN CASE WHEN "subclass" IN ('rail', 'narrow_gauge', 'rack_rail') THEN CASE WHEN "service" IN ('yard', 'siding') THEN 0.25 ELSE 1 END ELSE 1 END ELSE 1 END) WHEN @vector_tile_zoom > 12 AND @vector_tile_zoom <= 14 THEN scale_linear(@vector_tile_zoom,12,14,CASE WHEN "class" = 'rail' THEN CASE WHEN "subclass" IN ('rail', 'narrow_gauge', 'rack_rail') THEN CASE WHEN "service" IN ('yard', 'siding') THEN 0.25 ELSE 1 END ELSE 1 END ELSE 1 END,CASE WHEN "class" IN ('rail', 'rail_construction') THEN CASE WHEN "subclass" IN ('rail', 'narrow_gauge', 'rack_rail') THEN CASE WHEN "service" IN ('yard', 'siding') THEN 0.25 ELSE 1.5 END ELSE 1.5 END ELSE 1.5 END) WHEN @vector_tile_zoom > 14 AND @vector_tile_zoom <= 18 THEN scale_linear(@vector_tile_zoom,14,18,CASE WHEN "class" IN ('rail', 'rail_construction') THEN CASE WHEN "subclass" IN ('rail', 'narrow_gauge', 'rack_rail') THEN CASE WHEN "service" IN ('yard', 'siding') THEN 0.25 ELSE 1.5 END ELSE 1.5 END ELSE 1.5 END,CASE WHEN "class" IN ('rail', 'rail_construction') THEN CASE WHEN "subclass" IN ('rail', 'narrow_gauge', 'rack_rail') THEN CASE WHEN "service" IN ('yard', 'siding') THEN 1 ELSE 2 END ELSE 1.5 END ELSE 1.5 END) WHEN @vector_tile_zoom > 18 THEN ( ( CASE WHEN "class" IN ('rail', 'rail_construction') THEN CASE WHEN "subclass" IN ('rail', 'narrow_gauge', 'rack_rail') THEN CASE WHEN "service" IN ('yard', 'siding') THEN 1 ELSE 2 END ELSE 1.5 END ELSE 1.5 END ) * 1 ) END)), ';')''')
self.assertTrue(dd_properties.property(QgsSymbolLayer.Property.PropertyCustomDash).isActive())

conversion_context = QgsMapBoxGlStyleConversionContext()
style["paint"].pop("line-width")
has_renderer, rendererStyle = QgsMapBoxGlStyleConverter.parseLineLayer(style, conversion_context)
self.assertTrue(has_renderer)
self.assertEqual(rendererStyle.geometryType(), QgsWkbTypes.GeometryType.LineGeometry)
self.assertFalse(rendererStyle.symbol()[0].useCustomDashPattern())
dd_properties = rendererStyle.symbol().symbolLayers()[0].dataDefinedProperties()
self.assertEqual(dd_properties.property(QgsSymbolLayer.Property.PropertyStrokeWidth).asExpression(), '')
self.assertEqual(dd_properties.property(QgsSymbolLayer.Property.CustomDash).asExpression(),
'''array_to_string(CASE WHEN @vector_tile_zoom >= 16 THEN (array(6,3.75)) WHEN @vector_tile_zoom >= 15 THEN (array(5,3.125)) WHEN @vector_tile_zoom >= 14 THEN (array(4,2.5)) ELSE (array(3,1.875)) END, ';')''')
self.assertTrue(dd_properties.property(QgsSymbolLayer.Property.PropertyCustomDash).isActive())

def testParseLineNoWidth(self):
conversion_context = QgsMapBoxGlStyleConversionContext()
style = {
Expand Down

0 comments on commit 6d4e343

Please sign in to comment.