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

Clean up z13 #3467

Merged
merged 5 commits into from
Nov 3, 2018
Merged

Conversation

matthijsmelissen
Copy link
Collaborator

@matthijsmelissen matthijsmelissen commented Oct 21, 2018

This pull request cleans up the rendering on z13 and z14, by removing a number of features from these zoom levels.

  • Drop buildings from z13 and z14
  • Drop footways from z13
  • Drop service roads from z13
  • Drop pedestrian roads from z13
  • Different rendering of living_street on z13
  • Drop playground outlines from z13 and z14
  • Lighten up the color of trams
  • Drop streams, ditches and drains from z13 (and intermittent ones from z14)

Fixes #3331.

The primary purpose of this pull request is to remove visual clutter from z13 and z14. These zoom levels contain lots of tiny elements that are hard to interpret, and lead to noise in the images. As a consequence of removing these features, the remaining features (such as the street pattern) become easier to read.

The secondary purpose is to give more attention to landuse rendering on these zoom levels. Especially landuse=retail is currently hardly visible on the map on any zoom level, this pull request should improve this.

@matthijsmelissen
Copy link
Collaborator Author

matthijsmelissen commented Oct 21, 2018

Preview z13:
Before:
screen shot 2018-10-21 at 13 22 40

After:
screen shot 2018-10-21 at 13 22 01

@matthijsmelissen matthijsmelissen force-pushed the z13-cleanup branch 2 times, most recently from a7c35fe to aa52f30 Compare October 21, 2018 11:20
@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 21, 2018 via email

@dieterdreist
Copy link

dieterdreist commented Oct 21, 2018 via email

@SomeoneElseOSM
Copy link
Contributor

This seems to go against the first item at https://github.com/gravitystorm/openstreetmap-carto/blob/master/CARTOGRAPHY.md "an important feedback mechanism for mappers". Also providing examples only of a densly-populated Central European city will do nothing to placate critics who've accused this style as being targeted at those areas only in the past ...

@mboeringa
Copy link

Honestly, no one is going to understand this change... Especially dropping buildings from z13/14 will be totally alien to our openstreetmap community and heavily contested if implemented.

@kocio-pl
Copy link
Collaborator

I will try to reply to the critics later (also look closer at all the proposed changes individually), for now just Warsaw example:

z13

Before
hkivvtea

After
tkskupza

z14

Before
mpgc8zxx

After
dvc6lzkr

@kocio-pl
Copy link
Collaborator

It's good to look back to see how the database grows over the years creating more noise. Using current version of style (without this PR) for the last CC BY-SA licensed planet file (2012) and comparing with current data in Warsaw:

z13, 2012

tafxyayv

z13, 2018

hkivvtea

z14, 2012

b pm7f0j

z14, 2018

h2hvqj2d

@mboeringa
Copy link

mboeringa commented Oct 22, 2018

@kocio-pl ,

This adds nothing new to the existing images. What it actually mainly shows IMO is the less than ideal choice of a highly saturated highway=footway color in openstreetmap-carto, which is the main cause of the clutter, and the addition of many sidewalks.

Unfortunately, although your Polish fellow mappers have been very active adding such sidewalk detail, they have been far less diligent in adding additional tags like footway=sidewalk/crossing, that might help filter out this clutter at medium zooms like Z13/14.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Oct 27, 2018

This seems to go against the first item at https://github.com/gravitystorm/openstreetmap-carto/blob/master/CARTOGRAPHY.md "an important feedback mechanism for mappers".

I think it doesn't. The key is that only some buildings are visible on midzoom, and they are related to some special areas, which are eclipsed by them. z13 is the first zoom level where people can have proper feedback about landuses that are medium-level structures. It also supports diversity and helps to see where they are omitted.

This is related to #2896 - there's no single magical solution to show bigger entities/structures without eclipsing them with smaller features, since it would require considering very special relations (like "this park is part of the hospital, so render it one step later"), which is unlikely. However showing all the possible landuses without buildings on one zoom level at least gives the general impression what bigger areas are there before we go into details and loose this general view.

Having said that, I think that one special zoom level for landuses is enough and I would not remove buildings from z14.

@matthijsmelissen matthijsmelissen changed the title Clean up z13 and z14 Clean up z13 Oct 28, 2018
@matthijsmelissen
Copy link
Collaborator Author

How about moving intermittent and seasonal streams down one zoom level but
leaving the regular streams as-is?
At any rate I think intermittent streams should be rendered one zoom level
later than year-round streams.

Good idea. I have removed intermittent streams/ditches/drains from z14 too now.

This seems to go against the first item at https://github.com/gravitystorm/openstreetmap-carto/blob/master/CARTOGRAPHY.md "an important feedback mechanism for mappers".

No, the opposite: the feedback mechanism for retail landuse is currently almost absent, this PR will improve this. Seeing the buildings one level later make not much difference in terms of feedback for building mappers.

Also providing examples only of a densly-populated Central European city will do nothing to placate critics who've accused this style as being targeted at those areas only in the past ...

I have checked other areas as well, but I'm not planning on making screenshots of every arbitrary place on earth. Anyone who is interested in how this change behaves in other areas can run the code for themselves (and I'm happy to hear about potential issues in other areas).

Having said that, I think that one special zoom level for landuses is enough and I would not remove buildings from z14.

I have adapted this pull request to only drop buildings from z13 for now.

@matthijsmelissen
Copy link
Collaborator Author

@kocio-pl As far as I concerned this is ready to be merged now, what do you think?

@kocio-pl
Copy link
Collaborator

Some more examples of cleaning the noise on z13 and giving better feedback on landuses (thanks to @Komzpa I was able to import the whole Europe to check on his server):

London

screenshot_2018-10-28 openstreetmap carto kosmtik
screenshot_2018-10-28 openstreetmap carto kosmtik 1

Moscow

screenshot_2018-10-28 openstreetmap carto kosmtik 2
screenshot_2018-10-28 openstreetmap carto kosmtik 3

Stockholm

screenshot_2018-10-28 openstreetmap carto kosmtik 4
screenshot_2018-10-28 openstreetmap carto kosmtik 5

Madrid

screenshot_2018-10-28 openstreetmap carto kosmtik 6
screenshot_2018-10-28 openstreetmap carto kosmtik 7

Barcelona

screenshot_2018-10-28 openstreetmap carto kosmtik 8
screenshot_2018-10-28 openstreetmap carto kosmtik 9

Athens

screenshot_2018-10-28 openstreetmap carto kosmtik 10
screenshot_2018-10-28 openstreetmap carto kosmtik 11

@kocio-pl
Copy link
Collaborator

I'm not sure however about tram lines - it makes individual stops more visible, which makes more noise (dots instead of lines).

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 28, 2018 via email

@kocio-pl
Copy link
Collaborator

Resolves part of #2629.

@matthijsmelissen
Copy link
Collaborator Author

If we are going by a size rule

Please note that this is not an official rule of the style, or a rule agreed on by the maintainers.

@matthijsmelissen
Copy link
Collaborator Author

Stockholm and Barcelona look odd in their medieval urban centers. Is this because the streets are pedestrianized?

Yes, I think so. Think of it this way: if an car driver is driving from north to south Stockholm, he should know that he should navigate around the city centre.

@kocio-pl
Copy link
Collaborator

Interesting take! I haven't thought about it this way, but it makes a lot of sense for me - medium scale is more about transportation than pedestrian-related objects.

@vholten
Copy link
Contributor

vholten commented Oct 29, 2018

This PR changes the tram line color to #8E8E8E for zooms 13 and up, while it remains #6E6E6E at z12. Is it intentional that tram lines become lighter when zooming in from z12 to z13?

@dieterdreist
Copy link

dieterdreist commented Oct 29, 2018 via email

@dieterdreist
Copy link

dieterdreist commented Oct 29, 2018 via email

@SomeoneElseOSM
Copy link
Contributor

Interesting take! I haven't thought about it this way, but it makes a lot of sense for me - medium scale is more about transportation than pedestrian-related objects.

That's definitely something that very much depends on the user. Speaking personally, z13 is what I'd normally use for pedestrian route planning. Obviously (due to changes over a number of years, actually) the style is a bit rubbish for that already (compare for example https://map.atownsend.org.uk/maps/map/map.html#zoom=13&lat=54.0967&lon=-1.0322 , which considers pedestrian horse/cycle routes important to show, and https://www.openstreetmap.org/#map=13/54.0971/-1.0530 in the standard style, which seems to give visual priority to streams and ditches, and the national park and admin boundaries.

@mboeringa
Copy link

mboeringa commented Nov 2, 2018

@kocio-pl As far as I concerned this is ready to be merged now, what do you think?

I am almost certain this is going to lead to a barrage of complaints of our user base who are not involved in style development, and won't understand this change.

So unless you are "ready" to deal with that, and possibly have to revert this change ASAP if the masses arrive here on the Github repository or in the forums to complain, I don't think you are ready yet to implement this.

@dktue
Copy link

dktue commented Dec 22, 2018

Oh noooooooooo!

Please bring back the buildings!

Look how strange the yellow and purple areas look without them (clinics, schools, other big buildings).


Z13: Akward "empty" areas
image

Z14: Ahhhhhhh... now I understand the map
image

@kocio-pl
Copy link
Collaborator

This was done month ago. They look great to me, this zoom level is clear now, especially after moving tram and subway stations to z14+ in some bigger cities in the latest release. The visual noise is smaller.

@dktue
Copy link

dktue commented Dec 22, 2018

This was done month ago. They look great to me, this zoom level is clear now, especially after moving tram and subway stations to z14+ in some bigger cities in the latest release. The visual noise is smaller.

I disagree and share @dieterdreist 's optinion:

missing buildings are considerably worse than having them at z13. Look for instance at the bigger buildings, how the situation becomes misrepresented by appearing void without them.
Even without being able to discern every single small building, the sum of these create the morphology of the city (together with the roads), and this scale is interesting for seeing buildings and bigger urban structures and patterns.

My opinion is the one of a confused mapper that though that there was a bug. Please treat this seriously.

@Adamant36
Copy link
Contributor

Look how strange the yellow and purple areas look without them
Maps are always an abstraction. Considering the building were barely discernible at that level anyway, I don't know what the problem with being able to see the landuse areas only are. Especially since the buildings often got in the way of seeing it.

There's nothing strange about someone going "oh look a retail area" and then zooming in one more level see the buildings if they want. Its already done at z12 to z13 to see specific landuse. Or is that strange also?

Hopefully this help people micro map landuse more now that it has its own zoom level at least. Instead of mapping whole towns as residential like they normally do.

@kocio-pl
Copy link
Collaborator

My opinion is the one of a confused mapper that though that there was a bug. Please treat this seriously.

I do really treat it very seriously, believe me. It's always bugging me when someone has a problem with this style, since I'd like it to be good for all the people if possible. But it's not possible and I just have different view on z13 buildings problem than you.

For me this is natural progression of the urban structure:

  • On z12 we see the difference between green areas (natural and semi natural) and the rest of human landuses as uniform grey. And on this scale this is enough for me.
  • Next, on z13 we see the difference between landuses - for example lack of buildings gives me general view how big industrial landuses are compared to residential areas.
  • Then on z14 buildings add more details to the picture, however we loose a bit of clarity (small details, especially dark ones, add high frequency noise), but this is next step of zoom and it makes sense to make it more detailed than z13.

Using intermediate step between what is shown on z12 and z14 is a nice idea for me. Smooth changes of details level is what I like.

@Komzpa
Copy link

Komzpa commented Dec 23, 2018

Hi @kocio-pl,

Looking at provided screenshot I tend to read what was clear "University" previously as "A large field or maybe parking". Ways of mitigating this that I see:

  • modify university color on the zoom by blending it towards building color. It works well on industrial areas that are clearly industrial even though buildings are not there.

  • show building that are as large as an average map icon, to represent themselves "as an icon". That approach was taken on the Belroad map style you can have a look at http://latlon.org/#13/53.8428/27.6855?layers=L

image

@kocio-pl
Copy link
Collaborator

Both need some testing to make sure how it works. However I don't believe in easy finding new university color (see the problems we have with #3544), so maybe would you like to prepare code implementing second idea? I guess this might be harder than you expect, but why not try.

@Komzpa
Copy link

Komzpa commented Dec 23, 2018

Relevant code in Belroad:
https://github.com/jekhor/belroad/blob/master/project.mml#L273
(where ... and way_area > 3*3*!pixel_height!*!pixel_width!)

I can blindly adapt openstreetmap-carto to match it, I don't have any map rendering setup at hand now to test.

@kocio-pl
Copy link
Collaborator

@dktue
Copy link

dktue commented Dec 23, 2018

@kocio-pl : I do agree that cleaning z13 is a good idea. But I do think that simply leaving out the buildings was not a good thing to do. @Komzpa 's idea looks like a good approach to clean z13 but still maintaining the relevant structures.

My vote is to undo this change until an improved version is found for buildings.

@Adamant36
Copy link
Contributor

Adamant36 commented Dec 23, 2018

Maybe it could be improved once the major/minor building thing is implimented or a similar solution is found like rendering based on building type.

IMHO though a regression based on the negitive opinions of a few people that dont like how its rendered wouldnt be a good idea. As it not an actual bug/problem. So it should be treated like every other rendering improvement where a solution is found instead. Plenty of things that could look better in a few instances wait for solutions. Theres no reason this issue shouldnt be any different. Its not really that drastically bad.

Plus, it did sit there for like a month without serious critism and recived the thumbs up from multiple people before it was merged. So...

@dktue
Copy link

dktue commented Dec 23, 2018

@Adamant36 : I completely agree with you that there are many other issues but I definitely think this should be treated differently.

Not having any opposing votes in this repository is not an indicator that people mostly think that this is a good change. I didn't read about this in here even if I do contribute here from time to time.

I came here because I was confused. And I do think that many other people are, too but don't know where to address their confusion.

It's easy to get in a trap when you're the developer and then live in a bubble where everything makes perfect sense. And you can, indeed, explain why this is a good idea. But almost nobody knows about those ideas behind it and is left behind alone with it's confusion.

We should definitively revert that change and re-release it once it's done right.

@jragusa
Copy link
Contributor

jragusa commented Dec 23, 2018

Problems such those underlined by @Komzpa happen with large areas without a great density of road or large buildings. Inversely, if you consider dense area with important road network such as some capitals (Paris, London or Berlin, I have also tried with Geneva), the removal of buildings is beneficial and you can even better identify some specific areas (amenity=school, landuse=retail...).

@matkoniecz
Copy link
Contributor

In general it is better to open a new issue rather than start commenting in old.

This discussion may easily end forgotten unlike new issue that would need to be explicitly closed.


My vote is to

Note that we are not voting. Single comment with a new argument should have weight greater than 100 +1 comments.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Dec 23, 2018 via email

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

Successfully merging this pull request may close these issues.