Add WellKnownName's to support extra styles from QGIS #828
Conversation
|
I think there may still be an issue with zoom styling. It seems to apply the first style, but ignores subsequent ones. |
Tried using async to manage SVG loading for pattern fills, but it broke the style functions. Reverted back to using RegularShapes for pattern fills to avoid timing issues with loading. Added a number of extra RegularShapes.
|
I believe the earlier issues with zoom and style functions is now rectified. Instead of using SVG's for polygon pattern fills, I've reverted to OpenLayers RegularShapes, so not all shapes are supported, but many are. |
There was a problem hiding this comment.
Thanks for the PR @terry-longmacch. This is quite a lot to review. At first, just a few questions.
I also just updated the geostyler-style version in #829. So if you rebase this PR, we can see if the checks pass.
|
@jansule Yes, plenty to review here, but hopefully it makes sense? Also, hope I didn't add too many extra unnecessary changes. It was a bit of a process trying to get it all working, so there were many iterations of the code! |
|
No worries @terry-longmacch, from a first glance this looks great! We are happy to have contributors like you that dedicate quite an amount of time and work into improving GeoStyler. I will try to review this PR properly as soon as possible. |
There was a problem hiding this comment.
Thanks for the great work @terry-longmacch! There is just one remark left to address. Otherwise I'm +1 for getting this in.
| const diameter = graphicFill.radius as number * 2; | ||
| iconSize = [diameter, diameter]; | ||
| // Hack to try to join lines for hatch patterns, but space out icon patterns. | ||
| // Diagonal lines still do not render nicely in the corners, due to tiling. |
There was a problem hiding this comment.
Can we move this into a separate PR, so we can have a streamlined discussion about this specific feature?
There was a problem hiding this comment.
Sorry Jan, are you just referring to the calculations to join lines that aren't horizontal or vertical? Currently I have the iconSpacing factor at 2 for all non-line markers (line 1366), do you want this kept or would you like the spacing reverted to 1 for everything?
There was a problem hiding this comment.
Could you revert both parts and put them into a separate PR, so this PR stays as simple as possible?
We can then see in the other PR how we might need to adjust the changes regarding the join lines.
Description
Support for a large number of WellKnownName's has been added. This is to align with the point markers provided in the styling used with QGIS. Most of the new styles could not be replicated using OpenLayer's RegularShape specification, which is quite limited. As a result styling for points has been changed to use OpenLayer's Icon styles with SVG used for the image source. In most cases the SVG code for shapes was translated directly from the QGIS Python code.
The code has been tested as much as possible, but I have mainly been concentrating on converting SLD (from QGIS Server) to OpenLayers, and this seems to be working pretty well. I've had limited exposure to the other parts of Geostyler, so please let me know if there are other areas that need attention. I completely understand that there may be a lot of work needed, but hopefully it can be incorporated at some point. An attempt was made to use it with Geostyler Demo, but I think there might be other issues with this repository, as I couldn't get my fork working without the code changes.
Good luck!!
Polygon fill pattern issue
An attempt was made to use SVG point markers with
graphicFillfills for polygons. The SVG is loaded as a utf8 encoded data URL (see example below), which should be almost instantaneous to load, however the process of saving it as an HTMLImageElement for the canvas pattern seems to introduce a timing issue where the image isn't loaded before being passed as the fill for the polygon. This can be replicated by changing thepreferSvgparameter on this line to true, which will revert to using SVG's.Example SVG
Some evidence that it was a timing issue. Firstly, if a single SVG is hardcoded for all
graphicFillstyles, it will not load for the first layer loaded, but it will load for all subsequent layers. Secondly, if a zoom-based style is applied, the style will not show on the initial load, but once any zoom level change is made the style becomes available.A further attempt to get this working was made where the
writeStyleand all of its nested functions were made asynchronous (see this commit). This successfully loaded SVG's as the marker in pattern fill, however it broke all function based styling - zoom based and styling based on feature attributes. I believe this is because OpenLayers loads styles synchronously, so when presented with a style function that should be asynchronous, it doesn't respect theasyncnature.Further notes
Polygon fill patterns currently don't have any mechanism to alter the spacing of point markers in the fill. For the moment, if a line pattern is specified (
horline,vertline,line) the spacing between markers has been reduced to 0. This is an attempt to support hash pattern fills where lines should be joining across canvas tiles. There are tile border issues when lines aren't either vertical or horizontal, that become more evident the thicker the stroke width - I don't know that there is any simple solution to this.Thick diagonal hash pattern line issue
For the remaining marker styles a default fixed spacing of 2x the marker width has been set to allow for some gaps between the markers, until marker spacing is supported.
Related issues or pull requests
The following pull requests are related due to the new definition of the accepted WellKnownTypes.
Geostyler Style - geostyler/geostyler-style#652
Geostyler SLD Parser - geostyler/geostyler-sld-parser#978
Pull request type
Please check the type of change your PR introduces:
Do you introduce a breaking change?
Checklist
npm testlocally)