Unpaved rendering

[sommerluk]

Fixes #110

Closes #4137 (alternative version)

That’s essentially the same code as #3357 with the following differences:

• No special unpaved rendering for highway=platform and railway=platform. (There was a minor bug in Render unpaved roads different #3357 rendering in wrong colour for railway=platform on bridges with layers ≠ 0 if there is no corresponding layer also in the “highway” part. Thought this case is rare, the fix in the SQL code could be a little complicate, so for the moment I’ve dropped it for “platform”.)
• Some small performance tuning in the SQL query for bridges. The bridges query (at difference to the roads-low-zoom or roads-fill query) can potentially be rendered various times: once for each layer=*. Together with the fact that it has yet a SELECT DISTINCT clause, it was my preferred starting point, as here with little change we might get better performance.

I’ve tested this locally, trying to eliminate influences of other programs running in the background. I got know the same rendering time as without unpaved rendering (while the old unpaved rendering took 10% more time). @rrzefox Could you take a look at the performance here also?

[imagico]

Do you have indication that this actually improves performance in a meaningful way? I am asking because this makes adapting the roads code to different designs - like with tertiary roads in a distinct color - significantly more complex.

[sommerluk]

Do you have indication that this actually improves performance in a meaningful way?

Well, the starting point was the comment of talaj who explained that likely dst-out is no performance problem, but dst-over is. Furthermore, he said that dst-over isn’t executed when there is no actual geometry that matches. Based on this, with this code I get the same performance on my local environment as without unpaved rendering. With the old code I get 10% slower performance. This is likely not representative. The last time I made I measured local performance, the results of @rrzefox where different from mine, and I suppose his results are more close to performance on production servers.

In short: There are some weak indications, but nothing of which I could say it’s representative.

this makes adapting the roads code to different designs - like with tertiary roads in a distinct colour - significantly more complex.

Indeed. I share your concerns. If we get to the conclusion that we can live with the performance without this SQL change, than I prefer not doing the SQL change for exactly the reason you mentioned.

[imagico]

Have you tried limiting the number of dst-over operations also in the roads-fill layer and not only in the bridges layer? If this has a significant influence on performance it would be way more significant to do this on the road fills than just on the bridges.

Also checking if a performance difference is due to the bridges layer or the roads-fill layer is relatively easy since both layers exist in both versions of the code so you can simply disable the bridges layer in both and check if there still is a performance difference without it.

[sommerluk]

In the underlying Python script I call the Mapnik rendering now various times instead of a single time, so with more overall time, perhaps the measurement becomes better.

It shows that with full SQL tuning (also for roads-fill, as you proposed) it is indeed faster.

[sommerluk]

Added the full SQL tuning to this branch.

[HolgerJeromin]

Mixing "color" white and "class" tertiary_link feels odd.

[sommerluk]

I agree, it’s indeed ugly.

[kocio-pl]

As v4.15.0 is near, I guess this code is not yet ready for merging?

We have to wait some more until mapnik/mapnik#3980 (comment) will be usable for us, because we depend on multiple other code releases and packages, but it looks like proper solution is on the way.

[rrzefox]

@rrzefox Could you take a look at the performance here also?

I've deployed this on Friday, so excluding the weekend, I now have 3 days of stats:

    |         OLD  |          OLD |         OLD  |         NEW  |         NEW  |         NEW
  z |  2018-08-28  |  2018-08-29  |  2018-08-30  |  2018-09-24  |  2018-09-25  |  2018-09-26
 19 |        1.66  |        1.48  |        1.50  |        1.69  |        1.51  |        1.64
 18 |        1.44  |        1.56  |        1.60  |        1.85  |        1.69  |        1.78
 17 |        1.58  |        1.79  |        1.82  |        2.24  |        1.85  |        2.01
 16 |        2.34  |        2.37  |        2.53  |        3.06  |        2.74  |        2.63
 15 |        3.51  |        3.58  |        3.94  |        4.65  |        4.10  |        4.32

This does look fine for me, the difference is within the margin of error (remember, this is just average over all tiles rendered on a day, so fluctuations are to be expected because it's not the same tiles every day).

[sommerluk]

@rrzefox

Thanks a lot for testing! This information helps me a lot.

@imagico @HolgerJeromin

Indeed the road-type-color query is ugly. I guess we could use simple the road type. This would be a much clearer SQL query (but would have less performance gain)…

[kocio-pl]

Just a quick status check: mapnik/mapnik#4004 has been merged, so the next thing we wait for is a new Mapnik v3.0.x release (probably 3.0.22), then update of mapnik-reference package (mapnik/mapnik-reference#153) and Kosmtik update, but also making Debian and Ubuntu Mapnik package and deployment of new Mapnik package on OSMF servers. New Kosmtik release might be also needed.

[kocio-pl]

Oh, and CartoCSS probably will need a new release together with new npm package - mapnik/mapnik#4004 (comment).

[rrzefox]

Just a heads up: this is now no longer deployed on my server because it wouldn't apply on top of 4.17.0.

[kocio-pl]

There are new Mapnik release (3.0.22), mapnik-reference releases (3.0.22 and npm 8.10.0) and also corresponding CartoCSS releases (1.2.0 and npm 1.2.0 - mapbox/carto#498), what else is missing in our toolchain? Kosmtik update, release and npm release?

[Adamant36]

Just because I find it interesting, what in particular from a technical standpoint is it about roads that make rendering surfaces on them slower then rendering them on anything else? also, what's the reason something like dashed casing, similar to how @StyXman says he does it in his style, isn't a possible alternative?

[sommerluk]

@Adamant36 See #110, #2640, #3280, #3357

[Adamant36]

@sommerluk, thanks. I'll look through them.

[StyXman]

@Adamant36 I don't quite like how my style looks (the 'sausages'). I still can't find the time to implement mapnik/mapnik#3982.

[sommerluk]

Okay. For some time, I was quite busy and therefor did not work much on this issue. While still being quite busy, I want to give it another try now.

In the meantime some things have changed: The guys from Mapnik have made a native Mapnik implementation of polygon-like pattern rendering on line geometries which has been backported to v3.0.x and released with Mapnik 3.0.22. Also mapnik-reference has been updated to 3.0.22. Furthermore a corresponding new npm package has been published with version number 8.10.0. Also CartoCSS had a new release 1.2.0 which updates its dependencies to Mapnik 3.0.22 respectively mapnik-reference 8.10.0. This CartoCSS release is also available as npm package carto 1.2.0. I've opened an issue requesting a new kosmtik release that uses all this. For openSUSE users, as the Geo repository still has Mapnik 3.0.20, for the moment I provide RPM packages of Mapnik 3.0.22 for openSUSE in my home repository. For Ubuntu, there seem to be plans to integrate Mapnik 3.0.22 into the next release 19.04 (but this is not an LTS release).

The native support of Mapnik for polygon-like patterns on line geometries is really a great news. That fits perfectly our use case (except turning circles). Using it, the code can be much simpler and straightforward and without pitfalls. Given that it's only a matter of time until the new Mapnik version is available in Linux distributions, I'm inclined to close this PR and make a new one based on Mapnik 3.0.22 features. It's a pity because a lot of work had gone into the comp-op solution previously, but the new code would be much cleaner. What do you think about? How could this look like in practice? Waiting until Ubuntu 20.04 will be available on openstreetmap.org production servers?

[kocio-pl]

Nice to hear you still focused on this task!

The good news is that there is a special PPA for packages not included in the standard Ubuntu repositories:

https://launchpad.net/~osmadmins/+archive/ubuntu/ppa

It's enough to backport Mapnik packages to Bionic.

[sommerluk]

Cross-reference for updating Tilemill to support Carto 1.2.0/Mapnik 3.0.22: tilemill-project/tilemill#2695

[pnorman]

Note: If we bump the Mapnik version to 3.0.22, we should do that in a major release, not a minor.

[sommerluk]

Okay.

Just for the record: I have updated the documentation for the comp-op approach recently, considering the earlier comment from @imagico and commenting also the performance issues we had with this approach and the possible solutions. And of course mentioning the now available native Mapnik solution.

I've switched to Kubuntu 19.04 in order to have up-to-date Mapnik version, using Kosmtik current master (seeing some memory problems in Kosmtik at lower zoom levels, but that's not specific to unpaved rendering).

So: Time to write some actual code for the new Mapnik native support: https://github.com/sommerluk/openstreetmap-carto/tree/unpaved40 has the same pattern as here, but the changes to roads.mss and project.mml have been written from scratch.

No special code for turning circles because the Mapnik native support is only available on line geometries, and not on point geometries. (We could of course, if really desired, possibly fall back for the comp-op approach for turning circles, or maybe to some SVG marker files.).

[sommerluk]

@tomhughes If we would like to switch to Mapnik 3.0.22 and CartoCSS 1.2.0 in the future, what would be a practicable roadmap for the openstreetmap.org servers?

[tomhughes]

Well so long as there are mapnik packages we can use you pretty much just have to say when you want it to happen.

[jeisenbe]

Is there any update on the status of this? In Indonesia some of the other local mappers have decided to start mapping highway=primary on unpaved roads, which makes a lot of sense based on the road system here (they are the only roads connecting large towns to other towns and cities), but it would be very helpful to distinguish between paved and unpaved roads.

@sommerluk, does the new code work in https://github.com/sommerluk/openstreetmap-carto/tree/unpaved40 with the upgrade to Carto 1.2.0/Mapnik 3.0.22? Any new test renderings available?

Should we be asking openstreetmap.org to upgrade to Carto 1.2.0/Mapnik 3.0.22 in the near future?

[sommerluk]

@jeisenbe Sorry for the long delay. Currently I'm quite busy with other stuff. I hope that will get better next year… We have the same situation as in Indonesia also here in West Africa. I understand your point.

does the new code work in https://github.com/sommerluk/openstreetmap-carto/tree/unpaved40 with the upgrade to Carto 1.2.0/Mapnik 3.0.22?

Yes.

The new code relies on a new feature in Mapnik 3.0.22 which the Mapnik maintainers kindly developed to help us with the rendering of unpaved roads. Not only Mapnik, but also software that depends on Mapnik (Carto, Kosmtik) has to be upgrades. Carto 1.2.0 is okay for the new feature. For Kosmtik, you have to clone the github repository and build it locally (as the last published npm package is too old).

A new version rebased to the current master of openstreetmap-carto is available at https://github.com/sommerluk/openstreetmap-carto/tree/unpaved46 now. Testing is welcome.

Any new test renderings available?

No. But the rendering should be very much the same: It's the same pattern. The only difference is that it does not include support for special rendering for unpaved turning circles, and it adds support for unpaved platforms also for railways (not only for buses).

Should we be asking openstreetmap.org to upgrade to Carto 1.2.0/Mapnik 3.0.22 in the near future?

Yes. @tomhughes said that basically there have to be Mapnik packages available for the distribution that is currently running on the openstreetmap servers. For now, this is Ubuntu 18.04. Apparently there are no Mapnik 3.0.22 packages available for Ubuntu 18.04. I've requested a backport of Mapnik 3.0.22 to Ubuntu 18.04 now, but I don't know if this will become reality, and I don't know much about Ubuntu packaging process… The alternative is to wait until openstreetmap.org servers upgrade their Ubuntu version (probably 20.04 which means to wait about one year).

Maybe I should clode this PR and open a new one with the new code relying on the Mapnik 3.0.22?

[tomhughes]

Your chances of getting an official backport are probably minimal but unless there is some significant technical hurdle there's nothing to stop us doing our own.

[kocio-pl]

See also Kosmtik problems: #3717.

[imagico]

Note i have in the meantime tuned the access restriction dashing to work both with paved and unpaved roads reasonably well i think in the ac-style:

http://blog.imagico.de/rethinking-road-access-restrictions-rendering/

I am fine with the alternative to remove the access restrictions illustration instead - but combining the two while still being readable quite clearly seems to be possible.

[sommerluk]

Okay. Thanks for the hint!

I think, if we would change the access rendering, this would have to go into a separate pull request anyway.

So for now, maybe we could continue with this PR preserving access rendering for the moment – and access rendering might be changed later…

The questions have been ansered.

[imagico]

Got around testing this finally.

The fundamental things first:

Design wise I think this is a good solution for rendering unpaved surface roads in a subtle yet intuitive way that fits into the existing rendering system. In particular because it works for all road types rendered with fill and casing (including pedestrian/service road polygons) and all aeroway types with a common styling paradigm. And because it works well both in areas where paved roads are dominant as well as ones where unpaved roads are dominant - making it a good solution for a global project like this. No alternative has been suggested since #110 was opened 9 years ago that would even closely match these advantages.

The need to require a recent kosmtik version installed from source seems fine with me. I have been taking that approach for kosmtik for some time anyway and it is not a problem from my perspective (beyond the awkwardness any node-js software comes with obviously). We cannot in the long term continue this project if we essentially feature freeze our toolchain based on the last kosmtik release.

Now to testing - it looks good, i have not done extensive real world testing since design wise this is very similar to what i have been using in the ac-style for some time - you can find samples for that on https://imagico.de/map/ac_samples_en.php.

Here is an abstract test at z18 in comparison:

Master:

ac-style:

This PR:

Other zoom levels: z12, z13, z14, z15, z16, z17, z19

I see the following issues:

• visibility of the access dashing is not good but it can be improved by toning the dashing color. I am fine with doing that as a followup change.
• it could be considered to drop both the access dashing and the unpaved pattern rendering from roads with very narrow fill (like highway=road and highway=service at z15, minor service roads at z17, residential/unclassified at z13). This also is a tuning idea that can be done later as well.
• pedestrian roads have the unpaved pattern on tunnels while the others don't. For consistency i think this should be changed.

[sommerluk]

Okay.

For the first and the second point, I agree to do this as possible follow-up changes.

For the third point (inconsistent tunnel rendering for pedestrian), I've fixed this now.

[imagico]

Looks good now.

[davidbannon]

Great that this has been done. But how do we see it ? I imagined we were talking about what is shown on osm.org but no sign of any way to tell the difference between sealed and unsealed roads there ??
Davo

[imagico]

We have not yet made a new release since this was merged (and yes, we should probably do so).

[davidbannon]

Ah, that makes perfect sense, thanks ! "Release" is hard work, I am about to do a new release of my app, tomboy-ng on a much smaller scale. Doing the dev is lots more fun! Good luck.

Davo

[imagico]

Since this issue has now been prominently linked to by WeeklyOSM i wanted to add that the majority of the development discussion can be found not here but in previous pull requests: #3357 and in particular #2640. A lot of the early discussion happened also in #110. This has been a lengthy process but reading this can provide some valuable insights into how cooperative map design can work and what obstacles this is practically faced with.

Overall a lot of people contributed over the years to discussing and trying out different ideas to make this work and quite a few were in some form involved in making the solution we have now chosen (which was first discussed back in 2015) work despite all the obstacles that lead to many delays.

[pardthemonster]

I'm a little late to the discussion and I am sure this is discussed somewhere, but I just haven't found it yet. My question is if all the other unpaved variations per the wiki (https://wiki.openstreetmap.org/wiki/Key:surface#Unpaved) will be rendered, i.e. surface=compacted, surface=fine_gravel, surface=gravel, etc.

[imagico]

You have not actually formulated a question. The way we interpret the surface tag can be found in

openstreetmap-carto/project.mml

Lines 702 to 706 in 6591d7b

	CASE WHEN surface IN ('unpaved', 'compacted', 'dirt', 'earth', 'fine_gravel', 'grass', 'grass_paver', 'gravel', 'ground',
	'mud', 'pebblestone', 'salt', 'sand', 'woodchips', 'clay', 'ice', 'snow') THEN 'unpaved'
	WHEN surface IN ('paved', 'asphalt', 'cobblestone', 'cobblestone:flattened', 'sett', 'concrete', 'concrete:lanes',
	'concrete:plates', 'paving_stones', 'metal', 'wood', 'unhewn_cobblestone') THEN 'paved'
	END AS int_surface,

This is the same logic we already used for differentiating path/footway so far.

[pardthemonster]

Thank you on clarifying the other variations.