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 dedicated style for landcover polygons over water #5067

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

Conversation

tpetillon
Copy link
Contributor

Fixes #3854
Fixes #3707
Fixes #3840

This PR allows rendering on-land and over-water landcover polygons separately, in order to have different styles applied to each case. Compositing operations are used to achieve this: first the landcover polygons are drawn in their on-land style, then water areas are cut out, landcover polygons are drawn in their over-water style (when they have one) from below, and finally water areas are drawn, from below as well.

Before-afters:

tidal-4

tidal-5

tidal-6

Overview of affected styles:

tidal-2

Notable points:

  • This is mostly for the intertidal zone, but all water areas are concerned here.
  • Transparency is not used, colours for both styles are wholly separate. This allows for a greater control over the style. The proposed colours have been mostly obtained through transparency though.
  • These colours can most probably be improved. To minimise changes the colours of polygons that were already rendered over water (e.g. tidal flats) are unchanged.
  • Polygon patterns are not affected. Rendering them together with fill colours would be nice, as it would allow for different patterns over land or water if desired (could be useful for rocky areas), and it would allow drawing them below water lines (streams). However in the current style these patterns are sometimes used to make nonsensical overlaps (like forest over lake) visible to the users. So the effects of changing this should be considered.
  • This PR is compatible with rendering tidal channels (Support waterway=tidal_channel & retroactively waterway=stream over tidalflat #3865), differentiating water colours (Render ocean and seas differently than fresh water #3895), and adding a border to the ocean polygons (Restore linear coastline rendering #3926). But I want to keep this PR as small as possible.
  • One notable unknown is how performance is affected. Using composite operations and drawing some features multiple times must increase render times, but I don’t know of a good way to test this.

@dch0ph
Copy link
Contributor

dch0ph commented Feb 15, 2025

Very nice!

Is this using Kosmtik for display / rendering?

I've tried testing the PR with my renderd set-up (mapnik 3.1.0 claimed), but the ocean dst-over fails:

image

I haven't had much success with the mapnik compositing operations, but this may reflect an ageing mapnik version. There is little chance of working out from the horrific mapnik documentation what was working correctly in what versions.

P.S. I haven't looked over the code in any detail, but did notice that you have a repeat SQL query for the ocean cutouts and ocean fills (these can be cached e.g. as the amenity-points query).

@imagico
Copy link
Collaborator

imagico commented Feb 15, 2025

Thanks for working on this.

A number of remarks:

  • The approach used is sound and IMO suitable for this style. As is to not differentiate landcover-area-symbols over water and over land - if pattern differentiation is desired for landcover that pattern should be rendered in the main landcover layer and not the overlay. See also Render bare_rock pattern in landcover-area-symbols #3851 (comment)
  • Currently, as we render all water features in uniform color, the different layers for the backdrop are unnecessary, a single backdrop with a generated mercator square polygon is sufficient. See https://github.com/imagico/osm-carto-alternative-colors/blob/21d81b82ac14f6abfe08a9b96d88c1437fc5ef7b/project.mml#L536
  • Rendering the glaciers in the landcover layer won't work because of the outlining of the glaciers.
  • The idea to newly introduce a large number of new polygon fill colors is not going to work i think. I don't see a need to introduce any new colors at all with this change at all since the landcovers in question are well differentiated with patterns already.
  • It is important that the landcovers over water in appearance are similar to one another and at the same time substantially different from the land rendering so that the distinction between land and water is universally well visible. This is not the case with the suggested colors and as a result the land-water distinction is often unclear.
  • natural=sand should not be included in the landcovers rendered over water IMO. There is no consensus among mappers that natural=sand is equally usable for wet, waterlogged sand and for wind blown dunes. Mapping wet sand with natural=sand is globally a rare practice and us communicating to mappers that we do not specifically endorse overlaps between natural=sand and water seems a prudent choice. See Remove overlay pattern for natural=sand #3855 and sand vs shoal in tidal range #4513 for some further background on this.

@dch0ph
Copy link
Contributor

dch0ph commented Feb 15, 2025

  • natural=sand should not be included in the landcovers rendered over water IMO.

Yes, natural=shoal with different surfaces seems the accepted tagging. That said, I'm struggling to differentiate natural=shoal from wetland=tidalflat, except that the later seems to imply a default surface=mud. Do we want tidal natural=beach + surface=X and natural=shoal + surface=X to render in the same way?

  • The idea to newly introduce a large number of new polygon fill colors is not going to work i think. I don't see a need to introduce any new colors at all with this change at all since the landcovers in question are well differentiated with patterns already.

I agree that the landcover patterns should be doing most of the work. But I do think that some surfaces will be hard to differentiate on pattern alone against a blue background, and so limited use of new colours may be helpful e.g. surface=sand.

My main concern is whether to restrict the PR to ocean and optionally tidal natural=water (which we probably do want to render in a lighter shade as a next step). Effectively rendering landcover underneath permanent water, e.g. rivers, will firstly lead to a lot of changes; many rivers will be mapped directly over, say, natural=bare_rock and a lot of visual noise will be introduced if patterns are now rendered. Then it might encourage micromapping where mappers start to map the surfaces under water areas. The main benefit of this PR is for tidal areas, as shown above, where currently the tidal zone is obscured because the coastline is at the high tide limit.

@imagico
Copy link
Collaborator

imagico commented Feb 15, 2025

I'm struggling to differentiate natural=shoal from wetland=tidalflat, except that the later seems to imply a default surface=mud

World wide most wetland=tidalflat mapped without surface are sand or mixed sand and finer material.

Do we want tidal natural=beach + surface=X and natural=shoal + surface=X to render in the same way?

Currently beach and shoal are rendered identically - changing that would be a separate design decision. See also #3864.

@tpetillon
Copy link
Contributor Author

tpetillon commented Feb 17, 2025

Currently, as we render all water features in uniform color, the different layers for the backdrop are unnecessary, a single backdrop with a generated mercator square polygon is sufficient.

I included this, thanks.

P.S. I haven't looked over the code in any detail, but did notice that you have a repeat SQL query for the ocean cutouts and ocean fills (these can be cached e.g. as the amenity-points query).

No longer needed thanks to the point above.

natural=sand should not be included in the landcovers rendered over water IMO.

Removed it.

The idea to newly introduce a large number of new polygon fill colors is not going to work i think.

That was my first attempt, but it didn’t look very nice. I retried it, and it looks like we can find a single colour that works well after all.

Colours and patterns still need some fine tuning. I haven’t fixed glaciers yet.

My main concern is whether to restrict the PR to ocean and optionally tidal natural=water

My first draft only rendered landcover polygons over the ocean, and I’m very unsure about including regular water polygons in this PR, so I can remove them. Filtering on tidal=yes might be a good compromise.

Current state:

tidal-7

tidal-9

tidal-10

@tpetillon
Copy link
Contributor Author

Is this using Kosmtik for display / rendering?

Yes, with Mapnik 3.1.0.

@imagico
Copy link
Collaborator

imagico commented Feb 17, 2025

As a reference for discussing design:

current master:
old

this PR:
new

AC-Style:
ac

IMO using the tidalflat over water fill color also on the vegetated tidal wetlands has some appeal. It would avoid weird looking cases like here:

https://www.openstreetmap.org/#map=16/53.67236/6.97125

It will work less on non-tidal wetlands that are practically mapped over water, primarily reedbed and swamp (representing cases of vegetation growth in permanent standing water)

@dch0ph
Copy link
Contributor

dch0ph commented Feb 18, 2025

Is this using Kosmtik for display / rendering?

Yes, with Mapnik 3.1.0.

Thanks for clarifying. It seems that everything works fine with the raster output of mapnik 3.1.0, but there seem to be issues with the Cairo rendering (used for SVG/PDF). It would be a shame to lose vector output - high-quality print maps being an important personal use case. I suspect that these issues may be resolved in Mapnik 4.0.x (there do seem to be a lot of interesting fixes/editions in the 4.0 release), but I think that switching to Mapnik 4 is a discussion for another day, and breaking vector output is probably an acceptable trade-off for improved functionality.

My main concern is whether to restrict the PR to ocean and optionally tidal natural=water

My first draft only rendered landcover polygons over the ocean, and I’m very unsure about including regular water polygons in this PR, so I can remove them. Filtering on tidal=yes might be a good compromise.

I had misunderstood the significance of the river test (it was a river in a tidal/ocean area). But I do think including natural=water + tidal=yes would be useful. There are some interesting examples in the Humber:

The "sands" have been mapped as natural=mud even though at least Redcliff Sand looks like a sand bar. This is presumably because natural=mud is treated as an exception and is rendered "over" water in the landcover symbols layer. I guess this special case will be redundant in the current PR?

Currently beach and shoal are rendered identically - changing that would be a separate design decision. See also #3864.

That works. One design decision would be whether to only render natural=shoal in the tidal rendering, i.e. only select in "tidal landcover"?

tpetillon and others added 5 commits February 18, 2025 21:22
Because they're drawn behind everything else, we can use a single
polygon covering the entire planet.

Co-authored-by: Christoph Hormann <[email protected]>
And show beaches/shoals as early as mudflats.
@tpetillon
Copy link
Contributor Author

Tidal landcover is now only rendered over oceans and water polygon tagged tidal=yes.

IMO using the tidalflat over water fill color also on the vegetated tidal wetlands has some appeal. It would avoid weird looking cases like here:

https://www.openstreetmap.org/#map=16/53.67236/6.97125

That’s one of the main motivations for having a fill colour for these polygons. Currently in many places the areas closer to the coastline appear more water-y than the tidal flats further at sea, when they spend less time under water. With a fill colour for all tidal landcover we can have a more consistent land to intertidal zone to sea transition.

The "sands" have been mapped as natural=mud even though at least Redcliff Sand looks like a sand bar. This is presumably because natural=mud is treated as an exception and is rendered "over" water in the landcover symbols layer. I guess this special case will be redundant in the current PR?

Yes, tidal flats become just one among the list of landcover polygons rendered over tidal areas. This way the incentive to tag some areas incorrectly (or in a less precise way) in order to make them show on the map should disappear.

Is this using Kosmtik for display / rendering?

Yes, with Mapnik 3.1.0.

Thanks for clarifying. It seems that everything works fine with the raster output of mapnik 3.1.0, but there seem to be issues with the Cairo rendering

That’s unfortunate. It would be interesting to test with Mapnik 4. If these issues are fixed, it would at least give a way forward where no functionality is lost. But I don’t know if Kosmtik is even compatible with it.

@imagico
Copy link
Collaborator

imagico commented Feb 18, 2025

Tidal landcover is now only rendered over oceans and water polygon tagged tidal=yes.

We need to be careful with that for two reasons:

  • we don't want to nudge mappers to broadly add tidal=yes to water polygons to ensure overlapping landcover is rendered correctly. Often riverbank polygons for example are not split at a well defined limit of the tidal influence and forcing mappers to do so (without the necesssary local knowledge) or to broadly add tidal=yes to the whole thing to get correct rendering would be counterproductive.
  • not all landcovers that are rendered in a dedicated fashion over water with this PR are only mapped overlapping water in tidal settings. In particular beaches as a geomorphologically defined tag extend below the water level even in a non-tidal setting and can correctly be mapped as such. There is probably no consensus among mappers if natural=bare_rock is valid tagging for characterizing a permanently water covered lakebed/riverbed - but such mapping can be considered to make sense.

By the way: While, so far, the only landcovers suggested for special rendering over water are physical geography characterizations, this does not have to stay this way. It would, for example, be potentially worth considering rendering certain industrial landcovers (like wind parks, solar farms) in such fashion as well - and that would not be limited to a tidal setting.

It seems tidal=yes is used at least as much on landcover polygons as it is used on water polygons. Therefore interpreting it on landcovers to distinguish between tidal cases and non-tidal cases could be worth considering in some cases.

@dch0ph
Copy link
Contributor

dch0ph commented Feb 19, 2025

  • we don't want to nudge mappers to broadly add tidal=yes to water polygons to ensure overlapping landcover is rendered correctly. Often riverbank polygons for example are not split at a well defined limit of the tidal influence and forcing mappers to do so (without the necessary local knowledge) or to broadly add tidal=yes to the whole thing to get correct rendering

As I understand it, the PR would be introducing rendering that is especially useful for tidal cover - where the normal landcover is periodically covered by water. So I don't think there would be an incentive to add tidal=yes to "static" water areas - it would be seen as tagging for the renderer.

I agree that "riverbank polygons for example are not split at a well defined limit of the tidal influence", but the split between river and ocean is particularly variable - in some cases, the coastline is fixed at the river mouth (leaving large areas as tidal river, as the Humber above), while in others the coastline is pulled far inland. Rendering tidal patterns over ocean but not tidal river would drive a lot of coastline changes.

  • In particular beaches as a geomorphologically defined tag extend below the water level even in a non-tidal setting and can correctly be mapped as such. There is probably no consensus among mappers if natural=bare_rock is valid tagging for characterizing a permanently water covered lakebed/riverbed - but such mapping can be considered to make sense.

Yes, I can see this point. You can have a "river beach" or area of shingle that extends into the water, and I imagine that a lot of river / river areas are directly mapped over say natural=bare_rock. But it would add a lot of noise to start showing a pattern on thin areas. I think most mappers are content that single/sand "under" a river is not indicated - the "complaints" are focussed on tidal / ocean areas.

That’s unfortunate. It would be interesting to test with Mapnik 4

Let's ignore this. Using Carto for SVG/PDF is probably a fairly niche application. Mapnik 4.0.1 is available with the current (short life-time) Ubuntu release, so could in theory be tested. It will probably be next year before the Long Term Support version of Ubuntu defaults to Mapnik 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants