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

Fix rendering of water body labels #3919

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

sommerluk
Copy link
Collaborator

This is a bugfix PR for water body labels.

Changes proposed in this pull request:

  • Since Stop label rendering for too big areas #3764 (Stop label rendering for too big areas) water body labels disappear once the surface is quite big. In the same PR I had introduced a bug that makes labels appear again from z17. Consider the Lagune Pouto: The label correctly appears at z11, correctly disappears at z16 and incorrectly appears again at z17. The current PR will fix this.
  • natural_bay has rendering rules both in amenity-points.mss (rendering a label in regular font) and water.mss (rendering a label in italic font). Consider the Baie des sirènes: The label is rendered in a regular font up to z16, but starting from z17 with an italic font. This PR puts all the rendering rules for natural_bay at the same place and unifies the font face (using italic).
  • In water.mss there is almost duplicate code for natural_strait. This PR unifies the code in one single block.

The XML code that Carto produces decreases slightly with this PR.

@jeisenbe jeisenbe added the bug label Oct 3, 2019
@jeisenbe jeisenbe self-assigned this Oct 3, 2019
@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 3, 2019

I introduced 2/3rds of the bugs, so I will review this. :-)

Copy link
Collaborator

@jeisenbe jeisenbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test renderings look good. Thank you for fixing these bugs.

text-name: "[name]";
text-size: 10;
text-wrap-width: 25; // 2.5 em
text-line-spacing: -1.5; // -0.15 em
[way_pixels > 12000] {
[way_pixels > 12000],
[zoom >= 15][feature = 'natural_strait'] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should 'natural_bay' also be rendered the same way at >= z15? (Doesn't need to be in this PR)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer making small and clear PRs if possible, please.

@jeisenbe
Copy link
Collaborator

@sommerluk do you have time to merge this PR and your other approved PRs this week?

@sommerluk sommerluk merged commit da4d48d into gravitystorm:master Oct 23, 2019
@sommerluk
Copy link
Collaborator Author

@sommerluk do you have time to merge this PR and your other approved PRs this week?

Done. Thanks!

@sommerluk sommerluk deleted the waterareas01 branch February 15, 2020 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants