When converting mapbox fills, don't set a stroke on the QGIS

fill symbol if the mapbox fill color is non-opaque If the outline color is semi-transparent, then drawing the default 1px stroke will result in a double rendering of strokes for adjacent polygons, resulting in visible seams between tiles. Accordingly, we only set the stroke color if it's a completely different color to the fill or if the stroke color is opaque and the double-rendering artifacts aren't an issue
qgis · Apr 22, 2023 · ae5808f · ae5808f
1 parent 8cd44fc
commit ae5808f
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 2 deletions.
diff --git a/src/core/vectortile/qgsmapboxglstyleconverter.cpp b/src/core/vectortile/qgsmapboxglstyleconverter.cpp
@@ -434,7 +434,15 @@ bool QgsMapBoxGlStyleConverter::parseFillLayer( const QVariantMap &jsonLayer, Qg
     symbol->setOpacity( fillOpacity );
   }
 
-  if ( fillOutlineColor.isValid() )
+  // some complex logic here!
+  // by default a MapBox fill style will share the same stroke color as the fill color.
+  // This is generally desirable and the 1px stroke can help to hide boundaries between features which
+  // would otherwise be visible due to antialiasing effects.
+  // BUT if the outline color is semi-transparent, then drawing the stroke will result in a double rendering
+  // of strokes for adjacent polygons, resulting in visible seams between tiles. Accordingly, we only
+  // set the stroke color if it's a completely different color to the fill (ie the style designer explicitly
+  // wants a visible stroke) OR the stroke color is opaque and the double-rendering artifacts aren't an issue
+  if ( fillOutlineColor.isValid() && ( fillOutlineColor.alpha() == 255 || fillOutlineColor != fillColor ) )
   {
     // mapbox fill strokes are always 1 px wide
     fillSymbol->setStrokeWidth( 0 );

diff --git a/tests/src/python/test_qgsmapboxglconverter.py b/tests/src/python/test_qgsmapboxglconverter.py
@@ -10,7 +10,11 @@
 __copyright__ = 'Copyright 2020, The QGIS Project'
 
 import qgis  # NOQA
-from qgis.PyQt.QtCore import QCoreApplication, QSize
+from qgis.PyQt.QtCore import (
+    Qt,
+    QCoreApplication,
+    QSize
+)
 from qgis.PyQt.QtGui import QColor, QImage
 from qgis.core import (
     Qgis,
@@ -1145,6 +1149,51 @@ def testFillStroke(self):
         # mapbox fill strokes are always 1 px wide
         self.assertEqual(renderer.symbol()[0].strokeWidth(), 0)
 
+        self.assertEqual(renderer.symbol()[0].strokeStyle(), Qt.SolidLine)
+        # if "fill-outline-color" is not specified, then MapBox specs state the
+        # stroke color matches the value of fill-color if unspecified.
+        self.assertEqual(renderer.symbol()[0].strokeColor().name(), '#47b312')
+        self.assertEqual(renderer.symbol()[0].strokeColor().alpha(), 255)
+
+        # explicit outline color
+        style = {
+            "id": "Land/Not ice",
+            "type": "fill",
+            "source": "esri",
+            "source-layer": "Land",
+            "layout": {},
+            "paint": {
+                "fill-color": "rgb(71,179,18)",
+                "fill-outline-color": "rgb(255,0,0)",
+            }
+        }
+        has_renderer, renderer = QgsMapBoxGlStyleConverter.parseFillLayer(style, context)
+        self.assertTrue(has_renderer)
+
+        self.assertEqual(renderer.symbol()[0].strokeStyle(), Qt.SolidLine)
+        self.assertEqual(renderer.symbol()[0].strokeColor().name(), '#ff0000')
+        self.assertEqual(renderer.symbol()[0].strokeColor().alpha(), 255)
+
+        # semi-transparent fill color
+        style = {
+            "id": "Land/Not ice",
+            "type": "fill",
+            "source": "esri",
+            "source-layer": "Land",
+            "layout": {},
+            "paint": {
+                "fill-color": "rgb(71,179,18,0.25)",
+            }
+        }
+        has_renderer, renderer = QgsMapBoxGlStyleConverter.parseFillLayer(style, context)
+        self.assertTrue(has_renderer)
+        # if the outline color is semi-transparent, then drawing the default 1px stroke
+        # will result in a double rendering of strokes for adjacent polygons,
+        # resulting in visible seams between tiles. Accordingly, we only
+        # set the stroke color if it's a completely different color to the
+        # fill if the stroke color is opaque and the double-rendering artifacts aren't an issue
+        self.assertEqual(renderer.symbol()[0].strokeStyle(), Qt.NoPen)
+
     def testFillOpacityWithStops(self):
         context = QgsMapBoxGlStyleConversionContext()
         style = {