-
Notifications
You must be signed in to change notification settings - Fork 819
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
Do not render small water areas on low zoom #2952
Do not render small water areas on low zoom #2952
Conversation
On zoom levels 0-8, don't render small water areas (smaller than 32px).
@rrzefox would it be possible to test this on your server? |
Could you also publish some before/after renderings? Probably the places that you gave as example of problems with small water areas would be good. |
It currently bothers me both on zoom levels 6 and 7, and perhaps 8 and 9. So i think we definitely need measures for the zoom levels up to 7 (on levels 0-5 the small water areas are not very useful to me either). I eyeballed 32px as a cut-off, let's see how it looks like when we have a rendering demo. |
I don't have enough data in my database at the moment for realistic example renderings at these zoom levels. |
Done, and Z0-8 have already been rerendered. |
Unfortunately it reintroduces #2580 on z0-z2. |
This looks like an improvement to me. |
On z0-z1 we don't see any of the biggest lakes (except Caspian, of course). On z2 Ontario, Erie and Onega are still not visible. |
I also think that filtering produces nice effects, so it's worth applying. On the other hand La Plata and big lakes are special, so it would be good to preserve them. The simplest solution would be to start filtering from z3 instead of z0. What do you think about it? |
Yes, I agree. |
polygon-gamma: 0.6; | ||
[landuse = 'basin']::landuse { | ||
[zoom >= 7][way_pixels >= 32], | ||
[zoom >= 8] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On zoom levels 0-8, don't render small water areas (smaller than 32px).
So it should be:
[zoom >= 9] {
water.mss
Outdated
} | ||
} | ||
|
||
[natural = 'water']::natural, | ||
[landuse = 'reservoir']::landuse, | ||
[waterway = 'riverbank']::waterway { | ||
[zoom >= 0] { | ||
[zoom >= 0][way_pixels >= 32], | ||
[zoom >= 8] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same as above: [zoom >= 9] {
Thanks! I was also thinking about progressive filtering and with these values it seems to be the best of both worlds, indeed. I would merge this version. |
@matthijsmelissen Do you plan to embrace the @rrzefox proposition? If it's just lack of time, I can fine tune the code and merge it soon. |
Generally I prefer steps of factor 4, as for every zoom level, lines get twice as large and areas four times as large. So for example 4-16-64 or 2-8-32. However, 4-16-32 might work as well (it means on z2 some additional water areas pop up, which might make sense). Aside from the exact numbers, I'm happy with @rrzefox's approach. I was planning to get back at this some time, but I'd be glad if you can pick it up earlier. |
@rrzefox Were there some quirks which would be visible with 4-16-64 or 2-8-32 approach or was it just a free form handcrafting? If the latter is the case, could you test one of the proposed "4x" schemes? |
OK, let's see the 4-16-64 then. BTW - because Matthijs has changed the account name, the link to compare is different now: http://bl.ocks.org/matthijsmelissen/raw/af7a602c222dbf1ff1a2c0d84ed755b7/#2.00/18.8/14.7 |
4-16-32 works in fact quite well, I added it to the PR now. |
I changed the documentation, it is now 0-7. |
I've now deployed 4-16-64 for testing (Z0-3 rerendered) |
It looks OK for me. On z0 La Plata is not visible, but the border too, so it's not a problem. Lack of Ontario, Erie, Ladoga, Onega and Victoria on z0 are also not visible if you're not looking for them. Anything on z1+ looks decent in both 4-16-32 and 4-16-64 for me, so it's up to you which version do you like more, Matthijs. |
Me being from Argentina I can say that this is a nice improvement, and I don't think anybody would complain about Río de la Plata not being present in z0. |
@rrzefox This does not work. It will still render water areas of size 8 on z1 because the first line is still true. |
@rrzefox I have fixed this in my branch, would you be able to deploy my branch to your server? I used 4-16-32, as filtering with 4-16-64 will almost certainly be too aggressive. |
It was already tested and the screenshots are here: #2952 (comment) |
Nope, that test contained the bug I mentioned in #2952 (comment) . |
Just to be clear, the 4-16-32 deploy actually showed 4-4-4, the 2-8-32 deploy actually showed 2-2-2, and the 4-16-64 deploy actually showed 4-4-4 again. |
The fixed 4-16-32 version has been deployed and Z0-7 rerendered. |
That's OK for me, we can merge it any time soon. |
I'm confused, it looks like z1 and z2 are wrong now - La Plata and smaller Big Lakes are missing now on both proposed and current versions (OSMF re-rendering is happening right now)... http://bl.ocks.org/matthijsmelissen/raw/af7a602c222dbf1ff1a2c0d84ed755b7/#2.00/27.4/42.3 |
Another thing that we could make better would be to not filter riverbanks, as suggested here: http://www.openstreetmap.org/user/imagico/diary/42975#comment40531 |
My intention was not to render riverbanks at all. The solution is exactly the opposite of what @imagico suggests: these riverbanks are way too big, and should be cut up in smaller parts. |
Current code still allows showing them, so maybe it'd be better to get rid of them completely on some initial zoom levels to avoid problems with some segments being visible and some others invisible. |
Makes sense. |
Do you have any idea what could cause this? |
I guess we just got lost with testing all the possibilities on @rrzefox server. 😄 It was probably some small, stupid error but nobody checked it on his own machine. |
On zoom levels 0-7, don't render small water areas (smaller than 32px).