[Qgis-developer] SLD parsing: handling ogc:Literal within CssParameter PR #3996

[Luigi Pirelli]

great :) and many thanks Jorge :)
Luigi Pirelli

**************************************************************************************************
* Boundless QGIS Support/Development: lpirelli AT boundlessgeo DOT com
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
**************************************************************************************************


On 15 January 2017 at 21:08, Jorge Gustavo Rocha <jgr at di.uminho.pt> wrote:
> Hi devs,
>
> This message introduces the PR #3996 [0] to the developers mailing list.
>
> QGIS fails to load SLD files with valid SLD syntax that uses
> <ogc:Literal> to specify the value of a <CssParameter>.
>
> This works:
> <CssParameter name="stroke-width">2</CssParameter>
>
> This fails:
> <CssParameter name="stroke-width">
>         <ogc:Literal>2</ogc:Literal>
> </CssParameter>
>
> Both are valid SLD syntax.
>
> I noticed this behaviour when importing styles with the Geoserver
> Explorer plugin. I opened an issue on qgis-geoserver-plugin [1]. From
> the discussion with Luigi Pirelli, it seems wise to start by solving the
> problem on the QGIS side.
>
> So, my plan is to fix QGIS and then also fix the qgis-geoserver-plugin.
>
> I've made my humble first PR to QGIS regarding this issue. I hope I did
> it right.
>
> More details
>
> To confirm this behaviour, we can use the default data and styles used
> by Geoserver. Several default styles from Geoserver uses this
> <ogc:Literal> tag, and that's why so many styles are not parsed correctly.
>
> Let's use the shapefile geoserver/data/data/sf/streams.shp and the style
> geoserver/data/styles/simple_streams.sld as an example. Those files can
> be downloaded from [5].
>
> 1) The style uses <ogc:Literal> tags around <CssParameter> values.
>
> 2) The style is valid according to the SLD definition.
>
> xmlstarlet val -s
> http://schemas.opengis.net/sld/1.0.0/StyledLayerDescriptor.xsd
> simple_streams.sld
> simple_streams.sld - valid
>
> 3) When the style is loaded in QGIS, these values are not parsed and the
> default values are assumed instead.
>
> 4) The rendering on Geoserver is [2]. The render on QGIS (right now) is [3].
>
> The same syntax is used in other default Geoserver styles.
>
> Note: The <ogc:Literal> is properly handled when it appears in the
> context of a Filter. For example, the following SLD is already properly
> imported:
>
>           <ogc:Filter>
>             <ogc:PropertyIsBetween>
>               <ogc:PropertyName>PERSONS</ogc:PropertyName>
>               <ogc:LowerBoundary>
>                 <ogc:Literal>2000000</ogc:Literal>
>               </ogc:LowerBoundary>
>               <ogc:UpperBoundary>
>                 <ogc:Literal>4000000</ogc:Literal>
>               </ogc:UpperBoundary>
>             </ogc:PropertyIsBetween>
>           </ogc:Filter>The problem
>
> The limitation is only when a value of a <CssParameter> parameter is
> surrounded by <ogc:Literal>.
>
> The solution
>
> To solve this issue, I've changed
> QgsSymbolLayerUtils::getSvgParameterList at
> QGIS/src/core/symbology-ng/qgssymbollayerutils.cpp
>
> Instead of just get the value from the firstChild(), I add a test to
> check if there is an additional <ogc:Literal> ElementNode. So, the value
> can be or not be within a <ogc:Literal> tag.
>
> The rendering on QGIS, after applying the changes proposed, is [4].
>
> [0] https://github.com/qgis/QGIS/pull/3996
> [1] https://github.com/boundlessgeo/qgis-geoserver-plugin/issues/58
> [2]
> http://webgis.di.uminho.pt/~jgr/geoserver%20rendering%20of%20simple_streams.png
> [3]
> http://webgis.di.uminho.pt/~jgr/qgis%20rendering%20of%20simple_streams.png
> [4]
> http://webgis.di.uminho.pt/~jgr/qgis%20rendering%20of%20simple_streams%20fixed.png
> [5] http://webgis.di.uminho.pt/~jgr/simple_streams.zip
>
> I would love to provide unit tests to confirm the error and to show that
> the proposed solution works, but I need to study more deeply how to
> write tests. I've just tried to find out a test that already reads SLD
> files, but I didn't find any. So, I think I have to write the test from
> scratch.
>
> I would like to thank Luigi Pirelli for the initial comments and
> pointing me the this approach.
>
> Please be gentle: it is my first PR.
>
> Regards,
>
> J. Gustavo
> --
> Jorge Gustavo Rocha
> Departamento de Informática
> Universidade do Minho
> 4710-057 Braga
> Tel: +351 253604480
> Fax: +351 253604471
> Móvel: +351 910333888
> skype: nabocudnosor