fix merge errors in #3750 (bay/strait labeling)

[imagico]

Merging of #3750 substantially changed the functionality of the PR compared to the reviewed version. It also resulted in hard to understand code.

Current de facto rendering can be seen on

https://imagico.de/map/styleinfo/#style=osmcarto&section=tags&key=natural&value=bay

This change implements what @jeisenbe intended in #3750 (i.e. label all natural=bay/strait from z14 independent of how they are mapped). It also makes the code easier to understand by separating the labeling of water features with a blue fill (like natural=water) from the rendering of water features with label only rendering (natural=bay/strait).

[MeonStudios]

Hi, while I am not a maintainer, I took it upon myself to review this PR. I have two points

• Even though you 'added more code' I think this is a good change and improves the readability greatly & makes it easier make changes to bodies of water down the road.
• I have noticed that the check for way pixels is now gone in the new rendering for text of bays and straits, I assume this is intended? It would be nice to show some before/after samples.

[imagico]

I have noticed that the check for way pixels is now gone in the new rendering for text of bays and straits, I assume this is intended?

Yes, #3750 meant to do that, quote from there:

Stop using way_area to set zoom level and text size for natural=bay and natural=strait text labels

Discussion of the background can be found in issues linked to from there.

It would be nice to show some before/after samples.

The samples shown in #3750 all show the behavior that this change is meant to achieve, the difference is what was introduced during merging of #3750. I tested that the change actually does so in abstract tests. Anyway - yes, i can create samples demonstrating this:

Before, z13:

After, z13:

z14 (unchanged, before=after):

Location is here:

https://www.openstreetmap.org/#map=14/12.6769/43.4561

But please: No discussion of the cartography here. This PR is meant to revert a change inadvertently introduced during merging of #3750. A discussion of cartography based on the current, unintentional state of rendering does not make much sense.

[MeonStudios]

Thanks for you comment. Seeing your explanation and screenshots I think this PR is good to go.

But please: No discussion of the cartography here. This PR is meant to revert a change inadvertently introduced during merging of #3750. A discussion of cartography based on the current, unintentional state of rendering does not make much sense.

I didn't mean to start a discussion, just merely checking assumptions :).

[imagico]

ping @pnorman - if you have the time to look at this. You commented on the code needing cleanup in #3750 (comment).

[pnorman]

The code looks okay but I don't know when I'll get a chance to load this up to test. Probably not until I get home after SOTM EU.

[imagico]

I don't mind waiting a bit, getting this right is more important than getting it out fast. And i would guess that the OSMF operations team is largely busy with conference as well - so rushing a release now would be counterproductive anyway.

[imagico]

Ok, i merged this so we can have it in the next release. It is a simple fix for an accidental change made, it has been independently reviewed, @pnorman indicated principal approval and no objections have been voiced.