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

Islet names rendering #990

Merged
merged 2 commits into from
Oct 6, 2014

Conversation

sommerluk
Copy link
Collaborator

This fixes #159 and #988 using the system proposed in #941.

Is rendering from zoom level >=7 okay (performance)? Would probably be low enough to solve #988. Can we imagine even bigger islands that should get a label?

@matthijsmelissen
Copy link
Collaborator

Thanks for the pull request.

The tag place=island is also quite commonly used for tiny islands/islets, so I think we can change [feature = 'place_island'][zoom >= 14] also to 19 or so. If the island is large enough it will render anyway.

This also does not seem to work on island/islet polygons that also have a natural/landcover tag, but if that is something that needs to be solved, we can do so later.

@sommerluk
Copy link
Collaborator Author

The tag place=island is also quite commonly used for tiny islands/islets, so I think we can change [feature = 'place_island'][zoom >= 14] also to 19 or so.

The downside would be that we do not force people to make a difference between place=island and place=islet. (On the other hand, the wiki is also not completly clear about the difference. There is only a soft description that says that islets have less than 1 km²).

@matthijsmelissen
Copy link
Collaborator

The downside would be that we do not force people to make a difference between place=island and place=islet.

I suspect the fact that the renderer rendered island names from z12 might be the reason that place=islet was proposed in the first place.

@sommerluk
Copy link
Collaborator Author

Maybe something to be discussed on the tagging mailing list?

@matthijsmelissen
Copy link
Collaborator

I have tested this. We need to take a decision on the [feature = 'place_island'][zoom >= 14] line, and after that I think it can be merged.

@imagico
Copy link
Collaborator

imagico commented Sep 29, 2014

In general if the labeling is size triggered you can well start at lower zoom levels - you should however first label place=archipelago then. The french style does this starting at z=4 with quite good results (except for the usual mapnik ugliness) although the size thresholds are very low, see

https://github.com/cquest/osmfr-cartocss/blob/master/placenames.mss#L55
http://tile.openstreetmap.fr/?zoom=4&lat=76.89573&lon=57.71091&layers=B0000000FFFFFFF

where way_area is the area of the convex hull - which is probably not really necessary though.

@sommerluk
Copy link
Collaborator Author

Okay. So we have to decide on the [feature = 'place_island'][zoom >= 14] line and on rendering on lower zoom levels. If we decide this, I can make the necessary changes on this PR.

In the meantime, I’ve played around a little bit with a maximum values for way_pixels. The idea: While it is usefull to have a label on big islands like http://www.openstreetmap.org/relation/3889836#map=9/49.5109/-63.1274&layers=H at zoom level 9, it is probably more distracting than helpful to have a label on z15. So a maximum way_pixels value could maybe be usefull – in order to not render the label anymore when the island occupies the whole screen at some zoom level. Something like [feature = 'place_island'][way_pixels < 99999][zoom >= 7][way_pixels > 3000]. Of course, 99 999 is not big enough. But if I use a greater value than 99 999, it does not work anymore (means: The condition [way_pixels < 99999] seems to have no effect anymore, while the other conditions continue to work like expected). Does anyone has an idea why?

@matthijsmelissen
Copy link
Collaborator

Yes, it seems to be a bug in Carto. I run into that as well. See mapbox/carto#370 and mapbox/carto#369.

If you find a way around it, let me know.

@gravitystorm gravitystorm merged commit 1221966 into gravitystorm:master Oct 6, 2014
@gravitystorm
Copy link
Owner

I've set the zoom levels to 16 and 17 respectively. We can see how this turns out and adjust as necessary - my feeling was that 19 was definitely too high for pretty much anything, never mind a feature that might be measured in square kilometres but marked temporarily as a node. But like I say, we can adjust if needed.

Thanks @sommerluk for the changes.

@sommerluk
Copy link
Collaborator Author

@math1985

If you find a way around it, let me know.

You could leave one of the way_pixels contions at its place and move the other one into the braces bloc:

.text {
  [feature = 'place_island'][zoom >= 7][way_pixels > 3000],
  [feature = 'place_island'][zoom >= 16],
  [feature = 'place_islet'][zoom >= 14][way_pixels > 3000],
  [feature = 'place_islet'][zoom >= 17] {
    text-name: "[name]";
    [way_pixels > 900000] {text-name: ""; }
    text-fill: #000;
    text-size: 9;
    [way_pixels > 12000] { text-size: 12; }
    [way_pixels > 48000] { text-size: 15; }
    text-face-name: @oblique-fonts;
    text-halo-radius: 1;
    text-placement: interior;
  }

Works, but the code is ugly.

@sommerluk sommerluk deleted the islet_names_rendering branch October 11, 2014 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing name on islets
4 participants