[sld] Don't try to write rules/categorizes without symbolizers

Only create rules/categorized categories/graduated ranges if the associated symbol could be converted to SLD, and is not an "empty" symbol. Otherwise we do not generate a rule, as SLD spec requires a Symbolizer element to be present. (cherry picked from commit 308b69a)
qgis · Nov 19, 2024 · 9458e83 · 9458e83
1 parent 8cff1af
commit 9458e83
Show file tree

Hide file tree

Showing 11 changed files with 336 additions and 3 deletions.
diff --git a/python/PyQt6/core/auto_generated/symbology/qgssymbollayerutils.sip.in b/python/PyQt6/core/auto_generated/symbology/qgssymbollayerutils.sip.in
@@ -57,6 +57,13 @@ class QgsSymbolLayerUtils
     static QString encodeSldBrushStyle( Qt::BrushStyle style );
     static Qt::BrushStyle decodeSldBrushStyle( const QString &str );
 
+    static bool hasSldSymbolizer( const QDomElement &element );
+%Docstring
+Returns ``True`` if a DOM ``element`` contains an SLD Symbolizer element.
+
+.. versionadded:: 3.42
+%End
+
     static Qgis::SymbolCoordinateReference decodeCoordinateReference( const QString &string, bool *ok /Out/ = 0 );
 %Docstring
 Decodes a ``string`` representing a symbol coordinate reference mode.

diff --git a/python/core/auto_generated/symbology/qgssymbollayerutils.sip.in b/python/core/auto_generated/symbology/qgssymbollayerutils.sip.in
@@ -57,6 +57,13 @@ class QgsSymbolLayerUtils
     static QString encodeSldBrushStyle( Qt::BrushStyle style );
     static Qt::BrushStyle decodeSldBrushStyle( const QString &str );
 
+    static bool hasSldSymbolizer( const QDomElement &element );
+%Docstring
+Returns ``True`` if a DOM ``element`` contains an SLD Symbolizer element.
+
+.. versionadded:: 3.42
+%End
+
     static Qgis::SymbolCoordinateReference decodeCoordinateReference( const QString &string, bool *ok /Out/ = 0 );
 %Docstring
 Decodes a ``string`` representing a symbol coordinate reference mode.

diff --git a/src/core/symbology/qgscategorizedsymbolrenderer.cpp b/src/core/symbology/qgscategorizedsymbolrenderer.cpp
@@ -151,7 +151,6 @@ void QgsRendererCategory::toSld( QDomDocument &doc, QDomElement &element, QVaria
   }
 
   QDomElement ruleElem = doc.createElement( QStringLiteral( "se:Rule" ) );
-  element.appendChild( ruleElem );
 
   QDomElement nameElem = doc.createElement( QStringLiteral( "se:Name" ) );
   nameElem.appendChild( doc.createTextNode( mLabel ) );
@@ -200,6 +199,14 @@ void QgsRendererCategory::toSld( QDomDocument &doc, QDomElement &element, QVaria
   QgsSymbolLayerUtils::applyScaleDependency( doc, ruleElem, props );
 
   mSymbol->toSld( doc, ruleElem, props );
+  if ( !QgsSymbolLayerUtils::hasSldSymbolizer( ruleElem ) )
+  {
+    // symbol could not be converted to SLD, or is an "empty" symbol. In this case we do not generate a rule, as
+    // SLD spec requires a Symbolizer element to be present
+    return;
+  }
+
+  element.appendChild( ruleElem );
 }
 
 ///////////////////

diff --git a/src/core/symbology/qgsrendererrange.cpp b/src/core/symbology/qgsrendererrange.cpp
@@ -138,7 +138,6 @@ void QgsRendererRange::toSld( QDomDocument &doc, QDomElement &element, QVariantM
   QString attrName = props[ QStringLiteral( "attribute" )].toString();
 
   QDomElement ruleElem = doc.createElement( QStringLiteral( "se:Rule" ) );
-  element.appendChild( ruleElem );
 
   QDomElement nameElem = doc.createElement( QStringLiteral( "se:Name" ) );
   nameElem.appendChild( doc.createTextNode( mLabel ) );
@@ -160,6 +159,14 @@ void QgsRendererRange::toSld( QDomDocument &doc, QDomElement &element, QVariantM
   QgsSymbolLayerUtils::createFunctionElement( doc, ruleElem, filterFunc );
 
   mSymbol->toSld( doc, ruleElem, props );
+  if ( !QgsSymbolLayerUtils::hasSldSymbolizer( ruleElem ) )
+  {
+    // symbol could not be converted to SLD, or is an "empty" symbol. In this case we do not generate a rule, as
+    // SLD spec requires a Symbolizer element to be present
+    return;
+  }
+
+  element.appendChild( ruleElem );
 }
 
 //////////

diff --git a/src/core/symbology/qgsrulebasedrenderer.cpp b/src/core/symbology/qgsrulebasedrenderer.cpp
@@ -369,7 +369,6 @@ void QgsRuleBasedRenderer::Rule::toSld( QDomDocument &doc, QDomElement &element,
   if ( mSymbol )
   {
     QDomElement ruleElem = doc.createElement( QStringLiteral( "se:Rule" ) );
-    element.appendChild( ruleElem );
 
     //XXX: <se:Name> is the rule identifier, but our the Rule objects
     // have no properties could be used as identifier. Use the label.
@@ -403,6 +402,13 @@ void QgsRuleBasedRenderer::Rule::toSld( QDomDocument &doc, QDomElement &element,
     QgsSymbolLayerUtils::applyScaleDependency( doc, ruleElem, props );
 
     mSymbol->toSld( doc, ruleElem, props );
+
+    // Only create rules if symbol could be converted to SLD, and is not an "empty" symbol. Otherwise we do not generate a rule, as
+    // SLD spec requires a Symbolizer element to be present
+    if ( QgsSymbolLayerUtils::hasSldSymbolizer( ruleElem ) )
+    {
+      element.appendChild( ruleElem );
+    }
   }
 
   // loop into children rule list

diff --git a/src/core/symbology/qgssymbollayerutils.cpp b/src/core/symbology/qgssymbollayerutils.cpp
@@ -389,6 +389,20 @@ Qt::BrushStyle QgsSymbolLayerUtils::decodeSldBrushStyle( const QString &str )
   return Qt::NoBrush;
 }
 
+bool QgsSymbolLayerUtils::hasSldSymbolizer( const QDomElement &element )
+{
+  const QDomNodeList children = element.childNodes();
+  for ( int i = 0; i < children.size(); ++i )
+  {
+    const QDomElement childElement = children.at( i ).toElement();
+    if ( childElement.tagName() == QLatin1String( "se:LineSymbolizer" )
+         || childElement.tagName() == QLatin1String( "se:PointSymbolizer" )
+         || childElement.tagName() == QLatin1String( "se:PolygonSymbolizer" ) )
+      return true;
+  }
+  return false;
+}
+
 Qgis::SymbolCoordinateReference QgsSymbolLayerUtils::decodeCoordinateReference( const QString &string, bool *ok )
 {
   const QString compareString = string.trimmed();

diff --git a/src/core/symbology/qgssymbollayerutils.h b/src/core/symbology/qgssymbollayerutils.h
@@ -93,6 +93,13 @@ class CORE_EXPORT QgsSymbolLayerUtils
     static QString encodeSldBrushStyle( Qt::BrushStyle style );
     static Qt::BrushStyle decodeSldBrushStyle( const QString &str );
 
+    /**
+     * Returns TRUE if a DOM \a element contains an SLD Symbolizer element.
+     *
+     * \since QGIS 3.42
+     */
+    static bool hasSldSymbolizer( const QDomElement &element );
+
     /**
      * Decodes a \a string representing a symbol coordinate reference mode.
      *

diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt
@@ -345,6 +345,7 @@ ADD_PYTHON_TEST(PyQgsRenderContext test_qgsrendercontext.py)
 ADD_PYTHON_TEST(PyQgsRenderedItemResults test_qgsrendereditemresults.py)
 ADD_PYTHON_TEST(PyQgsRenderer test_qgsrenderer.py)
 ADD_PYTHON_TEST(PyQgsReport test_qgsreport.py)
+ADD_PYTHON_TEST(PyQgsRuleBasedRenderer test_qgsrulebasedrenderer.py)
 ADD_PYTHON_TEST(PyQgsRubberBand test_qgsrubberband.py)
 ADD_PYTHON_TEST(PyQgsScaleBarRendererRegistry test_qgsscalebarrendererregistry.py)
 ADD_PYTHON_TEST(PyQgsScaleCalculator test_qgsscalecalculator.py)

diff --git a/tests/src/python/test_qgscategorizedsymbolrenderer.py b/tests/src/python/test_qgscategorizedsymbolrenderer.py
@@ -936,6 +936,13 @@ def test_to_sld(self):
         symbol_f = createMarkerSymbol()
         renderer.addCategory(QgsRendererCategory(None, symbol_f, 'f', True, '4'))
 
+        # this category should NOT be included in the SLD, as it would otherwise result
+        # in an invalid se:rule with no symbolizer element
+        symbol_which_is_empty_in_sld = createFillSymbol()
+        symbol_which_is_empty_in_sld[0].setBrushStyle(Qt.NoBrush)
+        symbol_which_is_empty_in_sld[0].setStrokeStyle(Qt.NoPen)
+        renderer.addCategory(QgsRendererCategory(None, symbol_which_is_empty_in_sld, 'empty', True, '4'))
+
         dom = QDomDocument()
         root = dom.createElement("FakeRoot")
         dom.appendChild(root)

diff --git a/tests/src/python/test_qgsgraduatedsymbolrenderer.py b/tests/src/python/test_qgsgraduatedsymbolrenderer.py
@@ -25,6 +25,7 @@
     QgsRendererRange,
     QgsRendererRangeLabelFormat,
     QgsVectorLayer,
+    QgsFillSymbol,
 )
 import unittest
 from qgis.testing import start_app, QgisTestCase
@@ -45,6 +46,13 @@ def createMarkerSymbol():
     return symbol
 
 
+def createFillSymbol():
+    symbol = QgsFillSymbol.createSimple({
+        "color": "100,150,50"
+    })
+    return symbol
+
+
 def createMemoryLayer(values):
     ml = QgsVectorLayer("Point?crs=epsg:4236&field=id:integer&field=value:double",
                         "test_data", "memory")
@@ -564,6 +572,135 @@ def test_legend_key_to_expression(self):
         self.assertTrue(ok)
         self.assertEqual(exp, """(log("field_name") >= 15.5) AND (log("field_name") <= 16.5)""")
 
+    def test_to_sld(self):
+        renderer = QgsGraduatedSymbolRenderer()
+        renderer.setClassAttribute('field_name')
+
+        symbol_a = createMarkerSymbol()
+        renderer.addClassRange(QgsRendererRange(1, 2, symbol_a, 'a', True, '0'))
+        symbol_b = createMarkerSymbol()
+        renderer.addClassRange(QgsRendererRange(5, 6, symbol_b, 'b', True, '1'))
+        symbol_c = createMarkerSymbol()
+        renderer.addClassRange(QgsRendererRange(15.5, 16.5, symbol_c, 'c', False, '2'))
+
+        # this category should NOT be included in the SLD, as it would otherwise result
+        # in an invalid se:rule with no symbolizer element
+        symbol_which_is_empty_in_sld = createFillSymbol()
+        symbol_which_is_empty_in_sld[0].setBrushStyle(Qt.NoBrush)
+        symbol_which_is_empty_in_sld[0].setStrokeStyle(Qt.NoPen)
+        renderer.addClassRange(
+            QgsRendererRange(25.5, 26.5, symbol_which_is_empty_in_sld, 'd', False, '2'))
+
+        dom = QDomDocument()
+        root = dom.createElement("FakeRoot")
+        dom.appendChild(root)
+        renderer.toSld(dom, root, {})
+
+        expected = """<FakeRoot>
+ <se:Rule>
+  <se:Name>a</se:Name>
+  <se:Description>
+   <se:Title>a</se:Title>
+  </se:Description>
+  <ogc:Filter xmlns:ogc="http://www.opengis.net/ogc">
+   <ogc:And>
+    <ogc:PropertyIsGreaterThanOrEqualTo>
+     <ogc:PropertyName>field_name</ogc:PropertyName>
+     <ogc:Literal>1</ogc:Literal>
+    </ogc:PropertyIsGreaterThanOrEqualTo>
+    <ogc:PropertyIsLessThanOrEqualTo>
+     <ogc:PropertyName>field_name</ogc:PropertyName>
+     <ogc:Literal>2</ogc:Literal>
+    </ogc:PropertyIsLessThanOrEqualTo>
+   </ogc:And>
+  </ogc:Filter>
+  <se:PointSymbolizer>
+   <se:Graphic>
+    <se:Mark>
+     <se:WellKnownName>square</se:WellKnownName>
+     <se:Fill>
+      <se:SvgParameter name="fill">#649632</se:SvgParameter>
+     </se:Fill>
+     <se:Stroke>
+      <se:SvgParameter name="stroke">#232323</se:SvgParameter>
+      <se:SvgParameter name="stroke-width">0.5</se:SvgParameter>
+     </se:Stroke>
+    </se:Mark>
+    <se:Size>11</se:Size>
+   </se:Graphic>
+  </se:PointSymbolizer>
+ </se:Rule>
+ <se:Rule>
+  <se:Name>b</se:Name>
+  <se:Description>
+   <se:Title>b</se:Title>
+  </se:Description>
+  <ogc:Filter xmlns:ogc="http://www.opengis.net/ogc">
+   <ogc:And>
+    <ogc:PropertyIsGreaterThan>
+     <ogc:PropertyName>field_name</ogc:PropertyName>
+     <ogc:Literal>5</ogc:Literal>
+    </ogc:PropertyIsGreaterThan>
+    <ogc:PropertyIsLessThanOrEqualTo>
+     <ogc:PropertyName>field_name</ogc:PropertyName>
+     <ogc:Literal>6</ogc:Literal>
+    </ogc:PropertyIsLessThanOrEqualTo>
+   </ogc:And>
+  </ogc:Filter>
+  <se:PointSymbolizer>
+   <se:Graphic>
+    <se:Mark>
+     <se:WellKnownName>square</se:WellKnownName>
+     <se:Fill>
+      <se:SvgParameter name="fill">#649632</se:SvgParameter>
+     </se:Fill>
+     <se:Stroke>
+      <se:SvgParameter name="stroke">#232323</se:SvgParameter>
+      <se:SvgParameter name="stroke-width">0.5</se:SvgParameter>
+     </se:Stroke>
+    </se:Mark>
+    <se:Size>11</se:Size>
+   </se:Graphic>
+  </se:PointSymbolizer>
+ </se:Rule>
+ <se:Rule>
+  <se:Name>c</se:Name>
+  <se:Description>
+   <se:Title>c</se:Title>
+  </se:Description>
+  <ogc:Filter xmlns:ogc="http://www.opengis.net/ogc">
+   <ogc:And>
+    <ogc:PropertyIsGreaterThan>
+     <ogc:PropertyName>field_name</ogc:PropertyName>
+     <ogc:Literal>15.5</ogc:Literal>
+    </ogc:PropertyIsGreaterThan>
+    <ogc:PropertyIsLessThanOrEqualTo>
+     <ogc:PropertyName>field_name</ogc:PropertyName>
+     <ogc:Literal>16.5</ogc:Literal>
+    </ogc:PropertyIsLessThanOrEqualTo>
+   </ogc:And>
+  </ogc:Filter>
+  <se:PointSymbolizer>
+   <se:Graphic>
+    <se:Mark>
+     <se:WellKnownName>square</se:WellKnownName>
+     <se:Fill>
+      <se:SvgParameter name="fill">#649632</se:SvgParameter>
+     </se:Fill>
+     <se:Stroke>
+      <se:SvgParameter name="stroke">#232323</se:SvgParameter>
+      <se:SvgParameter name="stroke-width">0.5</se:SvgParameter>
+     </se:Stroke>
+    </se:Mark>
+    <se:Size>11</se:Size>
+   </se:Graphic>
+  </se:PointSymbolizer>
+ </se:Rule>
+</FakeRoot>
+"""
+
+        self.assertEqual(dom.toString(), expected)
+
 
 if __name__ == "__main__":
     unittest.main()