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

Use masking instead of darken blending of admin boundaries and ferry routes #4471

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pitdicker
Copy link
Contributor

@pitdicker pitdicker commented Sep 16, 2021

Current

The admin boundaries are currently drawn with some complicated tricks to avoid strange renderings where boundaries overlap. It is build around comp-op: darken, applied at the attachment level. Within an attachment layer, for every boundary there is first drawn a white line to cover any previous boundaries. Then the new boundary is drawn on top. The entire attachment layer is blended on top of the map so far, where only the pixels that are darker than the map are blended. Because the white lines that cover up overlapping boundaries are lighter, they fall away when blending. This has the tricky assumption that boundary lines are always darker than what is already drawn on the map.

The admin boundaries have different styles for every admin level. The most importand ones, admin_level <= 4, are a wide line with a thin center line. The others admin levels are drawn with thin lines. This is currently drawn with three attachment layers:

  • :firstline. All lines of admin_level > 4. To keep blending with comp-op: darken working as intended, lines from admin_level <= 4 are drawn as white background lines.
  • :wideline. Wide lines of admin_level <= 4.
  • :narrowline. The center lines of admin_level <= 4.

The only reason not to combine :firstline and :wideline is so that their opacity can be seperately controlled. Until now they both have been the same, 0.5. The cost is ca. 100 lines of mss, and having to draw the background lines of admin_level <= 4 one more time than necessary.

White background lines are always drawn, even when not needed in the cases where a boundary line is solid.

New

  • Use comp-op: dst-out at the line level instead of comp-op: darken at the layer level. The result is the same, but works it independent of lightness.
  • Merge :firstline and :wideline.
  • Don't draw background lines behind solid lines.

Aliasing artifacts

comp-op: darken doesn't take the opacity of a pixel into account, only the lightness. I don't know the lightness formula for aliased pixels, but it worked out. With the current foreground and background colors the covered aliasing pixels along a border line were lighter than the background color. With the darken blending filter they became invisible.

Masking doesn't have such a luxury. Any aliased pixel with not be fully masked out. A solution is to adjust the gamma AGG uses for anti-aliasing. Setting it to 0 for the mask lines makes them fully mask out the aliased pixels. But it does lead to small artifacts at three-way intersections. In my opinion this is not a problem, and maybe even looks good. At these points there would always be small aliasing artifacts anyway, and because a three-way point rarely look perfect because the dashes would have to align just right. Now it looks like one of the dashes ends just before the intersection.

Aliasing issue:
aliasing
With gamma: 0:
aliasing_gamma0

Ferry routes

Ferry routes work around the same problem as admin boundaries: multiple dashed lines that can overlap, but then the dashes may not overlap and cause the line to look solid. The current solution is to draw a background line with the water color. This doesn't look optimal where a ferry route crosses a tunnel. Also it doesn't work with #4128. I changed them to use the same solution as admin boundaries.

Advantages

  • I am quite happy with how these changes shaved off a quarter of the mss lines related to admin borders.
  • Instead of two background lines and a border line for boundaries at admin_level <= 4, now only the border line is drawn.
  • Basically all hidden complexity around drawing admin boundaries is now gone, such as why :firstline was used to draw invisible lines for admin_level <= 4 and only starting at zoom >= 8.
  • Masking is a simpler operation than comp-op: darken. This helps with my usecase: SVG files generated with Mapnik, which get some further processing before turning them into pdf. Any files with comp-op generated by Mapnik are non-standard (='broken'), and have to be fixed by a script. But an SVG with a comp-op that does masking is much easier to fix than one that does blending, because masking is a well-supported feature by viewers while blending is not even supported well enough by browsers. This is the first time since Reduce number of overlapping admin borders #1107 that I have fully working SVG exports again 🎉.

Previews

No visible changes to admin boundaries. After:
1
2
3
4

Ferry routes crossing a tunnel before:
ferry_before
Ferry routes crossing a tunnel after:
ferry_after

@pitdicker
Copy link
Contributor Author

Note that this PR pioneers a new technique for OSM-carto: masking. I see no real problems caused by it, but had to learn two tricks:

  • setting a gamma value for the masking lines to avoid aliasing issues.
  • limit the masking operation to only the current layer, by applying a layer-level opacity.

It can be a good tool for other currently hard to solve situations, such as properly rendering the highway-area layer seemingly on top of road casings, but underneath tunnels, buildings and barriers.

@pitdicker
Copy link
Contributor Author

One more comparison: with comp-op: darken the pixels along a boundary where not aliased consistently, while they are with masking.

With comp-op: darken:
aliasing_darken
With masking:
aliasing_gamma0

@jeisenbe
Copy link
Collaborator

I haven’t reviewed the code but the preview images look good.

@kocio-pl
Copy link
Collaborator

This is the first time since Reduce number of overlapping admin borders #1107 that I have fully working SVG exports again tada.

I consider it to be very useful change in itself.

@mboeringa
Copy link

mboeringa commented Sep 26, 2021

I think this whole issue is superseded by @pnorman's work on the new flex style, that includes a new de-duplicated 'planet_osm_admin' table (created and updated at the osm2pgsql import stage), which means there will no longer be overlapping admin boundary lines.

@pitdicker
Copy link
Contributor Author

I think this whole issue is superseded by @pnorman's work on the new flex style

True, should have mentioned that. This PR would be useful in the meantime, and after that only for the ferry routes.

It is silently the reason I am looking forward to #4431 (don't understand the rest of the advantages yet). But I have tried to work on similar boundary de-duplication in an other project before, and gave up because I could imagine too many edge cases. So I am cautiously hopeful, but thought an alternative solution would be good to share.

To give a few concerns, mostly because I haven't studied the solution well, and don't know how clean the data is: Can #4431 handle it when two borders have their points in opposite order? When one border has an extra point falling on a straight line, not shared by the other? Or T-intersection of boundaries, where the border at the bar of the T doesn't have a point? Would the slicing up of a border affect text placement, or the dash pattern near intersections? Nothing against #4431, it would be the cleaner solution, it just feels more tricky to work an the data than figure things out at rasterizing time.

@imagico
Copy link
Collaborator

imagico commented Sep 26, 2021

I have looked at this a bit and have to say i don't fully understand how it works technically but the combination of using opacity: 0.99999; and mask/line-gamma: 0; makes me wary that you are making use of some fragile particularity in the way Mapnik implements comp-op.

Here a comparison at normal scale:

This PR:
this

Master:
this

@mboeringa
Copy link

To give a few concerns, mostly because I haven't studied the solution well, and don't know how clean the data is: Can #4431 handle it when two borders have their points in opposite order? When one border has an extra point falling on a straight line, not shared by the other? Or T-intersection of boundaries, where the border at the bar of the T doesn't have a point? Would the slicing up of a border affect text placement, or the dash pattern near intersections? Nothing against #4431, it would be the cleaner solution, it just feels more tricky to work an the data than figure things out at rasterizing time.

I think Paul is by far the best person to answer these questions, but I have the feeling that you assume that de-duplicating takes place in PostgreSQL after creating duplicate lines. AFAIU the code of the flex style now, that is not the case. OpenStreetMap boundary relations, by themselves as stored in the API database, are not storing duplicate geometries, only referencing the shared boundary line from multiple relations. It is the processing in osm2pgsql and other frameworks to generate e.g. Simple Feature "Polygon" objects from them, that potentially creates duplication.

By adjusting the processing at the osm2pgsql import stage through the new flex options, it is possible to avoid this duplication.

@pitdicker
Copy link
Contributor Author

I have looked at this a bit and have to say i don't fully understand how it works technically but the combination of using opacity: 0.99999; and mask/line-gamma: 0; makes me wary that you are making use of some fragile particularity in the way Mapnik implements comp-op.

About the opacity part: it is a hack, but practical. All the masking should take place within the admin-* layer, and within the ferry-routes layer. But comp-op has no way to specify the destination of its blending/masking. It applies to the entire canvas, with everything that is already drawn on it.

Mapnik documents one solution: instead of directly drawing on the main canvas, we can get a blank canvas for a layer by using comp-op or opacity at the style level. Then all comp-op operations are restricted within that layer.

How it used to work on master was that comp-op: darken was applied at the layer attachment level (style level). So all admin lines were first drawn on an empty canvas, and then blended onto the main canvas. We still want to have that seperate canvas somehow.

The wiki does say:

The default (if no comp-op is set on a Style) is to skip the creation of a temporary canvas. So, while setting the comp-op on a style to src-over will invoke the default blending method, but it will also be triggering the rendering all the entire style to a separate canvas, which can lead to different output - perhaps desirable, perhaps not - just be aware of this.

This documentation is an entirely reasonable way for comp-op to behave, but turns out not to work anymore. I consider this a bug in Mapnik, that is probably not too hard to fix. opacity: 1.0; is documented to have the same effect, but is also broken. Maybe I'll even give fixing it a try. For now we need another solution.

opacity: 0.99999 works because it rounds to 1.0 but is nog recognized as such. An alternative is comp-op: src-atop which is identical to comp-op: src-over when the main canvas is a solid background, as it is for us. I have no strong preference. Both don't really feel fragile to me, changing how opacity or comp-op work at the layer level would be a breaking change for Mapnik.


mask/line-gamma: 0; is documented in the first post under Aliasing artifacts. Suppose we have line A below, and a masking line B covering it. Pixels drawn along the line are partially transparent because of aliasing. The same is true for the masking line. A partially transparent pixel from the masking line will not fully blot out the underlying pixel.

The current hack in ferry-routes was to draw an arbitrary wider background line (where the width did not get updated when the dashed line becomes wider at a higher zoom level). How wide should the line exactly be? Instead of guessing, I remembered that the AGG feature to deal with aliasing issues is to control gamma. With it we can just draw a line with the same width, and line-gamma: 0.

I have no problem with guessing a wider masking line, but using line-gamma feels much cleaner.

You made a good preview image again to show the differences! With GIMP I can measure tiny color changes in the image from master that show the background line does not fully cover the aliased pixels, but invisible to the eye.

Master looks better in this pretty artificial test with close almost parallel lines. I suppose we don't need to set gamma to all the way to 0.0, but can approach master with something like 0.15.


OpenStreetMap boundary relations, by themselves as stored in the API database, are not storing duplicate geometries, only referencing the shared boundary line from multiple relations.

Great. Than I continue to look forward to #4431! Would this PR make sense in the meantime, or maybe just for the ferry routes?

@jeisenbe
Copy link
Collaborator

jeisenbe commented Jun 6, 2022

@pnormal #4431 might also fix this, but would this PR make sense in the meantime?

@pitdicker how do the ferry routes look with gamma 0.15?

@mboeringa
Copy link

mboeringa commented Jun 6, 2022

@pnorman #4431 might also fix this, but would this PR make sense in the meantime?

I have been using my own variant of Paul's work in #4431 for quite some time now, and think it quite solid by now, and have commented on a couple of issues with the original implementation that have been fixed (one outstanding: #4549).

I haven't tested this in the full context of the openstreetmap-carto rendering stack though, just for one-time imports using osm2pgsql (v1.6.0 or higher highly recommended). I think a couple of others have been using the new flex style options of osm2pgsql in continuously updated databases and stacks though for quite some time, so maybe someone else can comment on the solidity of that with latest osm2pgsql and such complete rendering stack.

I do think switching to flex opens up a whole lot of new possibilities, and wouldn't necessarily be disruptive for the current rendering stack with the work Paul did. It should largely be a "drop-in" replacement with #4431, although likely some other minor changes elsewhere may be needed.

@pnorman
Copy link
Collaborator

pnorman commented Jul 6, 2022

@pnorman #4431 might also fix this, but would this PR make sense in the meantime?

This PR would make sense, but I wouldn't bother with work on it in favour of doing other work, as I think flex is close enough, and enables much simpler methods.

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.

6 participants