Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vectorize hatches #4458

Merged
merged 8 commits into from
Sep 12, 2021
Merged

Vectorize hatches #4458

merged 8 commits into from
Sep 12, 2021

Conversation

pitdicker
Copy link
Contributor

@pitdicker pitdicker commented Sep 2, 2021

My intention was to make one big PR touching all patterns. But it seems better to split out the simple, hopefully uncontroversial part. So this is part 1: all patterns simple enough to be manually created.

Changed patterns

  • allotments.svg and orchard.svg are a simple circle or square in an 8×8 tile. The circle in allotments.svg renders slightly different from the existing raster image. Ik suppose whatever image editor was used to create the file just draws a dot a little different, or the image was created at a higher resolution and scaled. A circle with a radius of 0.95px had roughly the same overall opacity.
  • vineyard.svg: recreated as SVG to rasterize exactly like the existing image, including some lines starting at half-pixel boundaries. (Using the correct color and opacity, which was the problem in Switch vineyard to SVG #2749.)
  • grey_vertical_hatch.svg and intermittent_water.svg are simple conversations to vector.
  • danger_red_hatch.png and military_red_hatch.png were hand drawn. The vector image is a close match, but not exactly the same because a diagonal line renders with aliasing.
    The tiles are now a bit larger, I used 32x32px as a minimum size. For vector tiles (Migration to vector tiles #3201) it seems to me to be better to use fewer tiles with long lines instead of many tiles with short line sections.
    Also the lines now cross at tile edges instead of tile corners. This way every crossing breaks a line in two, instead of four to go over the four tiles in a corner.
  • plant_nursery.svg: previously the pattern contained small artifacts, dots in a triangular pattern with a distance of 6px don't fit well in a tile of 128px. Since Mapnik 3.0.22 was released 2½ yeras ago pattern tiles repeat correctly with arbitrary sizes (see PolygonPatternSymbolizer metatile alignment #937). So I changed the pattern to 30×30px, where the dots repeated horizontally every 6px and vertically every 5px. Previously the pattern didn't look like an equilateral triangle because the vertical spacing was 4px, while it should be close to ½√3=5.2px. Fixes Artifacts in plant_nursery pattern #2771.
    The consequence on an older version of Mapnik are 2px artifacts around the render tile boundary, instead of multiple 1px artifacts within a tile.
  • dog_park.svg is recreated using the shop/pet.svg symbol.
  • The graveyard patterns were already perfectly fine. I only cleaned them up a bit. grave_yard_jewish.svg is now converted to strokes, and a true hexagram instead of an approximation.

Organization

All pattern files are currently placed in symbols, with sources, documentation and 2× scaled versions in symbols/generating_patterns. I think for organization it makes sense to put move all patterns to a patterns folder. They are not really symbols anyway.

Previous attempts

The following text applies mostly to the next PR, but it was already written anyway:

I know multiple PRs that converted some patterns have failed before. As I understand it the problems stem from a combination of these issues (ignoring what has been fixed in Mapnik the past years):

  1. A bug in Mapnik (see SVG patterns in landcover-area-symbols render inconsistently in different metatiles #2750).
    Currently Mapnik ignores the viewBox of the SVG, and bases the bounding box on the extends of the elements inside the SVG. Those are then scaled and centered to the pattern tile size. If the entire pattern fits within the tile, with no symbols crossing the tile boundary, it was possible to hack around this by adding an invisible square with the size of the pattern tile.
    I think the fix in Mapnik is simple, but this PR will depend on that fix getting merged and shipped... (Fix bounding box in svg patterns mapnik/mapnik#4239).

  2. Some PRs simply used the wrong color, because they didn't take opacity into account. In Switch sand and wetland patterns to SVG #2727 beach.svg used #9a9a9a while it should be #685d45 with 40% opacity. In Switch vineyard to SVG #2749 vineyard.svg used #99c68e, while it should be #759e6c with 38% opacity.

  3. I noticed SVGs with color rasterize slightly less clean than rasterizing a black svg and coloring it with Imagemagick. When a pixel gets more transparent, its color starts to diverge from the intended color. As such a pixel is mostly transparent, it shouldn't matter much but might contribute to a slight change in overall color with fine patterns. This probably contributed to the problem in Switch sand and wetland patterns to SVG #2727.
    I didn't look into the color rounding issue closely, but expect it has to do with rounding caused by using premultiplied alpha somewhere in Cairo. This is not really fixable, just something to be aware of.

  4. There is a final issue that I can imagine contributed to Switch sand and wetland patterns to SVG #2727. The beach patterns are very complex, and admittedly look very nice when magnified 20×. But their overall look depends on the rendering of thousands of tiny grain shapes that are less than a pixel small. I expect rasterized result to depend as much on the shape of the grains, as on the implementation details of the rasterizer around sub-pixel aliasing. A pattern with less sub-pixel detail may give more consistent results.

@pitdicker
Copy link
Contributor Author

Preview of the patterns, rasterized with Mapniks svg2png:

pattern before after
allotments allotments allotments
danger_red_hatch danger_red_hatch danger_red_hatch
grey_vertical_hatch grey_vertical_hatch grey_vertical_hatch
military_red_hatch military_red_hatch military_red_hatch
intermittent_water intermittent_water intermittent_water
orchard orchard orchard
plant_nursery plant_nursery plant_nursery
vineyard vineyard vineyard
dog_park dog_park dog_park
grave_yard_generic grave_yard_generic grave_yard_generic
grave_yard_christian grave_yard_christian grave_yard_christian
grave_yard_jewish grave_yard_jewish grave_yard_jewish
grave_yard_muslim grave_yard_muslim grave_yard_muslim

I'll add better previews later, that repeat a pattern multiple times on the right background color..

The three patterns with changed rendering:

  • danger_red_hatch and military_red_hatch have their pattern shifted a couple of pixels.
  • plant_nursery has 1px more vertical spacing, and no more spacing artifacts.

@pitdicker
Copy link
Contributor Author

pitdicker commented Sep 2, 2021

Please ignore this PR today, I am not content with my own testing yet.

Edit: when comparing the rendering plant_nursery between Kosmtik and https://openstreetmap.org, the colors locally looked harder. But I was being misled because of my HiDPI monitor. On openstreetmap.org the tiles get scaled in the browser, and the scaling algorithm does some averaging with the colors of adjacent pixels. On 1x rendering the color and transparency of the SVG match that of the png closely (off by a fraction of a percent).

So, should be ready for review now.

@pnorman pnorman marked this pull request as draft September 2, 2021 21:50
@pitdicker
Copy link
Contributor Author

pitdicker commented Sep 4, 2021

I just noticed that #3469 accidentally moved the rendering of danger_area to zoom >= 15, while it was zoom >= 9 before. I added a commit to restore the rendering, but after three years this is now itself a change.

Also added a commit that moves the almost transparent background fill of from danger_red_hatch, military_red_hatch and grey_vertical_hatch to the stylesheet.

@pitdicker pitdicker marked this pull request as ready for review September 4, 2021 07:11
@pitdicker
Copy link
Contributor Author

Removed the unnecessary xml declaration from the files. And finally made a preview with background color and a few repeats:

hatches_preview

@pnorman pnorman self-requested a review September 12, 2021 18:34
@pnorman pnorman merged commit 6a4cd4b into gravitystorm:master Sep 12, 2021
@pitdicker pitdicker deleted the vectorize_hatches branch September 13, 2021 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Artifacts in plant_nursery pattern
2 participants