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

Update zoom levels by height for masts, towers and telescopes #3536

Merged
merged 6 commits into from
Jan 7, 2019

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Nov 29, 2018

Fixes #3414

Changes proposed in this pull request:

  • Adjust mast & tower zoom so that it is consistent

  • Start at z14 for >160m, z15 for >80m, z16 for >40m etc; this allows masts to be rendered the same as towers, by continuing the masts up to z14 at 160m, moving them 1 zoom level

  • Communications towers and radio telescopes will also start at z14 rather than z13

  • Remove white space from telescope code

  • Start rendering monuments and obelisks at z14 for >80m and at z15 for >40m EDITED: Removed

Explanation
Masts and towers are similar man-made features. While towers may have a heavier structure, masts have support wires which are also visually significant. From a distance towers and masts are both quite visible if they are sufficiently tall, and can serve as orientation points in rural areas. Therefore it is sensible to render them at the same zoom levels.

Currently, towers over 100m tall are rendered at z13. However, buildings are no longer being rendered at this level, so it no longer makes sense to render towers at z13 in the absence of even very large or tall buildings.

160 m is the height of a 45 to 50 storey building; such tall structures are quite rare, even in large cities, so a 160m tower will usually be the tallest man-made object in the area. 80m towers are taller than 20 stores buildings, and 40m towers are taller than 10 stores buildings; these structures are taller than the great majority of structures in towns and cities, while a 20m mast or tower is only the height of an ordinary 6 storey apartment building.

Radio telescopes may also be rendered at z13; this PR will change this initial zoom level to z14 for consistency. Communications towers are assumed to be very tall towers, and will change from z13 to 14.

Monuments are rendered at z16 and obelisks at z17, currently, but these historic or cultural features may be many meters tall. With this PR, monuments and obelisks will be rendered one zoom level sooner than towers and masts of the same height; monuments and obelisks are visually and culturally more significant that most towers and masts.

Test rendering with links to the example places:

Honolulu, Hawaii z13 before
honolulu-z13-with-towers
Honolulu z13 after
honolulu-z13-no-towers

Zwedru, Liberia
z13 Before
zwedru-z13-tower
z13 After
zwedru-z13-no-towers

Dover, England
z13 Before
dover-z13-towers-before
z13 After
dover-z13-after

Southwest of Dover
z14 Before (200m mast not rendered)
sw-dover-mast-before-z14
After
sw-dover-mast-z14-after

Washington DC radio towers
z13 Before
washington-z13-before
After
washington-z13-after-no-towers

[EDITED: Obelist/Monument changes removed
Washington Monument (tall obelisk >160m)
https://www.openstreetmap.org/#map=17/38.88910/-77.03514
z14 Before
washington-monument-z14-master
z14 After
washington-monument-z14

z16 Before (obelisk not rendered, yet smaller monuments are)
washington-z16-master
z16 After
washington-monument-z16

Use height to set zoom level for obelisk & monument and adjust mast & tower zoom so that it is consistent
Start at z14 for >160m, z15 for >80m, z16 for >40m etc
Communications towers and radio telescopes will also  start at z14 rather than z13
Remove redundant water and wastewater plant lines
Remove white space from telescope code
@kocio-pl kocio-pl changed the title Rationalize zoom levels by height for masts and towers, add earlier zoom level for tall monuments and obelisks Update zoom levels by height for masts, towers, monuments and obelisks Nov 29, 2018
@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 29, 2018

Testing tuning is usually harder than adding, because it means deeper understanding of changes, so this is rather low on my queue priority. I made the title shorter to be easier to read.

@Adamant36
Copy link
Contributor

Do we know how many obelisks or monuments even have the height tag? It might not worth adding the extra code just to make them consistant with towers if none (or to little) of them use it.

@jeisenbe
Copy link
Collaborator Author

Taginfo doesn't record this, so I used overpass-turbo in a couple of mid-sized, well-mapped countries which I expected to contain obelisks: Germany and Italy.

In Germany, height=* is used with man_made=obelisk 64 times on nodes and ways, while man_made=obelisk is used 139 times over all. In Italy 52 have height, vs 110 overall. So it's over 40%, much higher than I expected.

There are only 516 uses of man_made=obelisk overall (https://taginfo.openstreetmap.org/tags/man_made=obelisk) but format=obelisk is used 663 times?! https://taginfo.openstreetmap.org/tags/format=obelisk

Height is used with historic=monument 443 times on nodes in Germany versus 593 overall = 75%

But in Italy its' 693 out of 17805 nodes and ways; only 4%. This suggests Italy may be tagging many smaller historic=memorial as historic=monument.

https://overpass-turbo.eu example query:
`[out:json][timeout:25];
{{geocodeArea:Italy}}->.searchArea;
(
node"man_made"="obelisk";
way"man_made"="obelisk";
);
out body;

;
out skel qt;`

@kocio-pl
Copy link
Collaborator

Good question. Both less than 1k uses, so it's not visible on Taginfo:

https://taginfo.openstreetmap.org/tags/historic=monument#combinations

However it can be estimated closer - I suck at interpreting Overpass Turbo output, but it's about 600 monuments:

https://overpass-turbo.eu/s/E96

and - well - one (1) obelisk:

https://overpass-turbo.eu/s/E97
https://www.openstreetmap.org/node/2489330832

So only monument code makes sense at the moment. They tend to be high (we probably don't count other measures. like object volume) and dominating, yet not always visible early enough, like:

https://www.openstreetmap.org/#map=16/21.8371/73.7196

or some other big ones:

https://pl.wikipedia.org/wiki/Plik:Height_comparison_of_notable_statues.png

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 30, 2018

Oh, I see: format=obelisk is used 421 times in Germany, mainly for milestones and boundary stones, eg:

    "format": "obelisk",
    "format:top": "pyramidal",
    "height": "3",
    "heritage": "yes",
    "historic": "milestone",

@jeisenbe
Copy link
Collaborator Author

... but it's about 600 monuments:
https://overpass-turbo.eu/s/E96
and - well - one (1) obelisk:
https://overpass-turbo.eu/s/E97
https://www.openstreetmap.org/node/2489330832

This is incorrect because you did the query with a bounding box:
node["historic"="monument"]["height"]({{bbox}}); etc.
This means it's only searching for things in the map shown on your screen, so it won't be consistent. I'd recommend trying to search all of Poland (but drop relations) eg:

[out:json][timeout:60];
{{geocodeArea:Poland}}->.searchArea;
(
  // query part for: “historic=monument and height=*”
  node["historic"="monument"]["height"](area.searchArea);
  way["historic"="monument"]["height"](area.searchArea);
);
out body;
>;
out skel qt;

This finds 88 monument nodes and ways in Poland with height, vs 3008 overall. Either Poland has more monuments than German, or most are mistagged historic=memorial, as in Italy.
There are 6 obelisk ways and nodes with height in Poland, vs 41 obelisks total.

I accidentally searched most of the world by bounding box using your link, and found about 4619 monuments with height, so about 8% are tagged with height globally - but as I mentioned, many "monuments" should be tagged as memorials. Germany seems to have cleaned this up the most.

@kocio-pl
Copy link
Collaborator

I took the whole Earth as bbox, as you can see opening the link. If you were right, height would be probably visible on https://taginfo.openstreetmap.org/tags/historic=monument#combinations, isn't it?

This way or another, I consider monuments to be a valid case.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 30, 2018

I've updated the PR by changing the height for monuments and obelisks.
They will now render at z14 for >80m and z15 for >40m.

After researching these objects, I found that there are very few monuments over 80m and only a handful over 160.

Most monuments are in the 10 to 30m range, when it is tagged. Even in Washington DC there is only 1 over 39m of height.

Most obelisks are under 20m, but there are a number between 20-40m, and a couple dozen over 40m and 80m. Only the Washington Monument is over 160m. See https://en.wikipedia.org/wiki/Obelisk

So the majority of monuments and obelisks with render the same before and after this PR. I've found a few examples with great enough height to change:

Obelisk, 25m tall - renders at z17 before, z16 after
https://openstreetmap.org/#map=17/52.39588/13.06108
z16, current master:
alter-markt-master
z16 after:
alter-market-z16-after

Monument, Bismarkturm, 45m tall - currently renders at z16, would render at z15 after
https://openstreetmap.org/#map=16/50.8085592/12.5636949
z15 current master:
friedhof-glauchau-z15-master
z15 After
friedhof-glauchau-z15-after

Monument, "Hall of Liberation", 47.5m tall
https://openstreetmap.org/#map=16/48.9188/11.86035
z15 current
kelheim-master
z15 After
kelheim-z15-after

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 2, 2018

Here's a nice test of a really huge monument in a big city: the Monas, Jakarta (Monument Nasional). It's 132 meters tall, almost as tall as the Washington Monument.


https://en.wikipedia.org/wiki/File:Jakarta-Panorama.jpg

With this PR it would first render at z14 instead of z16:

Jakarta z14 after
monas-z14-after

Jakarta z15 after
monas-z15-after

Jakarta z16 - same
monas-z16

@Adamant36
Copy link
Contributor

Buildings and place names start rendering at z14. I cant think of any icon that does off the top of my head. So, I think thats to early for it. I wouldnt put a monument at the same importance level as a building. Plus, once again, we are trying to clean up those levels. If the icon blocks out a town name or something where it first starts rendering that would be an issus. Would monomunt names be displayed at that level? business and road names arent even yet.

Can you do a test rendering of a monument on z14 that's in an area with a bunch buildings instead of the middle of a park? My guess is, it wouldnt work.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 2, 2018

I also noticed that at z15, there is a 35 meter tower that is currently rendering at z15, even though the National Monument does not. With the changes above, this tower would first render at z17

z15 Jakarta - current master
z15-jakarta-current
(Tower in lower left)

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 2, 2018

"...start rendering at z14. I cant think of any icon that does..."

Please see the initial comment: #3536 (comment)

Currently, man_made=communications_tower renders at z13
Man_made=tower renders at z13 if height > 100m (330 feet) currently,
radio telescopes render at z13 if diameter >60m, and optical telescopes with diameter >8m render at z14.
Waterfalls with height >20 and alpine_hut plus wilderness_hut also render at z13, but I've not changed this in the PR.

If the icon blocks out a town name or something where it first starts rendering that would be an issus.

The icon is not displayed if there is a city or town label in the same place.

Would monomunt names be displayed at that level?

No, still at z16 as before

Can you do a test rendering of a monument on z14 that's in an area with a bunch buildings instead of the middle of a park?

I don't think this is possible. There are no monuments of over 80 meters height (that's over 250 'Merican feet) directly next to buildings, as far as I can tell. These sort of features are usually found in parks. Even 40 meter (130 foot) monuments are rare; the examples I found in Germany were in rural areas or outside of the city in a park. See #3536 (comment)

In the United States, the tallest monuments are all found in parks or rural areas; eg the Saint Louis Gateway Arch, Washington Monument, Statue of Liberty, San Jacinto Monument in Houston, etc.
https://real-estate.knoji.com/tallest-monuments-in-the-united-states/

Probably the most urban monument over 80m is the Soldier's and Sailors Monument, in a circular plaza in downtown Indianapolis. It's barely over 80m:

@Adamant36
Copy link
Contributor

Well, taking all that into account, it seems sane to me. I dont think the optical telescope comparison is a good one, but its not enough to say they shouldnt be rendered similarly.

Thanks for the extra details.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 3, 2018

I downloaded the area around the Soldier's and Sailors Monument in Indianapolis:
https://www.openstreetmap.org/?mlat=39.768333&mlon=-86.158056&zoom=16&layers=M

Indianapolis sure does like monuments; they also have a 30 meter obelisk and a 63 meter World War memorial that looks like the Mausoleum of Halicarnassus in a park just to the north. https://en.wikipedia.org/wiki/Indiana_World_War_Memorial_Plaza

Before all of these would only will render at z16 because the height was not tagged. I've added height tags.

z16 Soldier's and Sailors (same before/after)

  • the yellow area around the monument is a community centre, underneath the pedestal.
    indianapolis-z16-both

z16 World War Memorial and Mall
world-war-memorial-and-obelisk-z16

z15 Indianapolis Before
z15-indianapolis-master

z15 After
indianapolis-z15-after

z14 Indianpolis Before
z14-indianapolis-master

z14 After
z14-indianapolis-after

On z15 the icons are smaller than the outline of the monument.
At z14 the icon is larger, but still fits within the roundabout, "Monument Circle."

@Adamant36
Copy link
Contributor

@jeisenbe, is there a good place to find out the height of monuments out there or is it essentially searching through somewhere like Wikipedia/on the ground surveying?

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 3, 2018 via email

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 7, 2018

Monument (53 meter tall cross, "Salib Wio Silimo") in the center of Wamena, Indonesia. This will now show at z15 instead of z16 (and I won't be tempted to tag it as man_made=tower instead. ;-) )

Before, z15:
wamena-z15-before

After, z15:
wamena-z15-after

[OT: I wish the airport icon still rendered at z15, or started rendering on the airport terminal building at z16]

@matkoniecz
Copy link
Contributor

I like idea of changes proposed here and I plan on reviewing how the code works. I prefer to avoid making promises when I will complete this, but hopefully soon.

@jeisenbe
Copy link
Collaborator Author

Re: cranes and chimneys. If @Adamant36 has found any problems with the current rendering of tall chimneys and cranes at z16, I could add a commit to this PR that would address them.

Also, I fixed the merge conflict.

@Adamant36
Copy link
Contributor

I appreciate your willingness to update it. That being said, I wouldnt call map clutter, or the way things look depending how they render, "problems" per say. They aren't bugs. Obviously what looks good or not is up to personal prefrence (at least in these cases). I stated why I think rendering them at z17/z18 versus z16 looks better to me subectively. Along with why your justification of rendering them at z16 "becuase" is bad logic. A few others agreed and you choose to go with your way instead.

There was a pretty heated disscussion here a while back about how its on the person doing the PR to justify why their change is good. Not on everyone else to justify why what they want to impliment shouldn't be. I would defer to that. I think it was on the back with bench issue. Although I could be wrong.

Other then that, I was pretty clear why I think rendering them at z16 is a bad idea. I'm not going to waste my time going into it again. Especially since you blew it off the first time. Your free to read through it all again.

I'm less concered about the particulars of cranes at this point anyway and more just don't like the proccess and unwillingness to engage in feedback on your part in that instance because it wasnt from "maintainers." Its rather dissmisive of how much work people besides them put into the project. Consencious is an important thing, maintainers or not.

I know you have a clear idea for how you want things though and that's fine. So I'm willing to accept it and move on if you disagree with me (and the other people who thought z17 was better).

Thanks for throwing it out there though. I apologize if I'm being a little brash (honestly).

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 17, 2018

@Adamant36, you initially said that we should not use height to adjust the zoom level of cranes. (See #3501 (comment)) At that time I had proposed to render them at z14 to z17 based on height (see: #3501 (comment)). dieterdriest and kocio-pl both thought that height was reasonable to use (see #3501 (comment)).

I then changed the PR to render cranes only at z16 and 17, two zoom levels later, based on your comments, which renders cranes at the same initial zoom level as chimneys. (see #3501 (comment))

So the final discussion was about that zoom level z16 vs z17, and whether cranes and chimneys should be rendered differently. We disagreed about this, dieterdriest seemed to support the earlier rendering, kocio-pl said he didn't mind either way, and tomasz had previously suggested z17/z18, so there did not seem to be a consensus. Therefore I didn't change the PR.

I was going to suggest changing the initial zoom levels of chimneys as well as cranes, if there was a consensus that cranes should not be rendered at z16. (I haven't heard a convincing argument for why cranes should be rendered later than chimneys)

If anyone is interested in working on this issue, changes can be made in a systematic way in #3536
I would not object to rendering towers and cranes later than proposed. Perhaps tall towers, masts, monuments and obelisks can first render at z15 for height >160 or 80m, and tall chimneys & cranes at z17?

@Adamant36
Copy link
Contributor

@jeisenbe, I'll think about it. Unfortunately there's a lot of other, higher priority stuff right now that should be dealt with. Plus, its not like there's an abundance of cranes where I live. So I probably won't even notice it that much. I'm definitely down for revisiting it later though once there isn't a bunch of other things and its had some time to be rendered on the map.

Even if it can use some adjustment later on, its better its that its rendered then not. So I appreciate you doing the PR. Even if the zoom level discussion didn't workout.

@Adamant36
Copy link
Contributor

Adamant36 commented Dec 21, 2018

@jeisenbe, if your committed to this you want to add rendering based on height for water towers to? 1,188 of them have the tag. So you might as well. Otherwise, it doesn't make sense that monuments should start rendering at z16 while water towers start at z17.

It would at least be good if you either knew what the actual exact height values for each specific thing was for the ranges though or at least had standard height ranges that they all follow.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 21, 2018 via email

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 21, 2018 via email

@Adamant36
Copy link
Contributor

Hhhhmmm, it seems kind of pointless to even add the height value in the first place if they are all shorter then 50 feet. If so, your probably only looking at a 20 foot height difference or something between the tallest and shortest ones. Since they are all off the ground to some degree. Which would be really lame for basing rendering on.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 21, 2018 via email

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 3, 2019

I've fixed the merge conflict. This PR is ready for review, I believe.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 3, 2019

@Adamant36, if you have a chance perhaps you could fetch this branch and review the cartography, since you were interested in the subject previously? Or do you think it is ok to merge this now, and then consider further changes later, if we want to reduce the initial zoom level another level to z15 instead of just from z13 to z14?

@Adamant36
Copy link
Contributor

@jeisenbe, I think Im good with it for now. Theres no reason we cant modify it later if need be. Im having problems with docker I need to worked out to. I appreciate that you asked though.

@matthijsmelissen
Copy link
Collaborator

I'm very skeptical about using height as a way to distinguish prominence of monuments. I think it will also complicate our code. I'm also afraid it will make the data harder to use for other consumers (for example because people might start tagging minor poles/needles as obelisks).

What do others think?

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 5, 2019 via email

@kocio-pl
Copy link
Collaborator

kocio-pl commented Jan 5, 2019

I'm not afraid of that. I'm more concerned about common (mis)practice to tag monument instead of memorial.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 5, 2019 via email

@kocio-pl
Copy link
Collaborator

kocio-pl commented Jan 5, 2019

That would be great if adding it become more common. By definition not all monuments have to be high (there are also other criteria like that you can walk into), but height tag might help finding mistagged objects.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

I have had a quick look at this and so far i am not convinced.

The idea of basing importance rating for zoom level decisions on tagged heights was first introduced in #2671. It was somewhat questionable there but at least there was broad use of height tags on most of those (each >10k) and the height of a tower/mast is obviously a major characterizing property.

Still mappers were widely confused by this change and the resulting non-intuitive appearance of towers at different zoom levels. It is not really clear to the mapper when there are towers with maybe dozens of different characterizing tags that the discerning property is the height tag, which in itself is not shown on the map - in particular of course since the height does not affect the prioritization towards other icons. For structures where the height is not by definition the largest dimension this is likely even worse.

I would prefer to go the other way and aim to reduce the complexity in zoom level logic, especially as long as POI icon display prioritization is not in sync with the starting zoom levels. For example the use of telescope:diameter does not make much sense - use numbers in the range of the thresholds used are in the single or two digit range. For man_made=obelisk the numbers are even lower. I would also by the way remove it for waterfalls where height=importance makes no sense at all.

Simplifying the logic for towers and masts i would support as a step in the right direction. Having some data on the number of features in the different height classes would be useful of course.

@matthijsmelissen
Copy link
Collaborator

I'm going to reject the pull request as there is apparently no support for it.

@imagico
Copy link
Collaborator

imagico commented Jan 7, 2019

@matthijsmelissen - i have not seen any comment rejecting the idea of unifying the zoom level thresholds of masts and towers so if @jeisenbe wants to modify this PR to do just that this would probably be supported (and would probably resolve #3414)

@matthijsmelissen
Copy link
Collaborator

Good point, re-opening.

@matthijsmelissen matthijsmelissen changed the title Update zoom levels by height for masts, towers, monuments and obelisks [WIP] Update zoom levels by height for masts, towers, monuments and obelisks Jan 7, 2019
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 7, 2019

I've removed the changes for monuments and obelisks as requested. Do you want to reopen it, @matthijsmelissen?

It's not clear to me if @imagico requested that the changes for telescopes be removed, or if he wants more extensive changes. The current PR removes telescope rendering at z13, to be consistent with towers. Should there be a separate PR to discuss entirely removing rendering based on height for this feature and waterfalls?

@imagico
Copy link
Collaborator

imagico commented Jan 7, 2019

I am sorry if that was not clear - i don't think the telescope:diameter logic in general is a good idea but i just mentioned this to illustrate my point. Your change of that reduces the zoom level logic complexity and starting zoom level variability and that is a step in the right direction.

@imagico imagico reopened this Jan 7, 2019
Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

Looks fine now, i have not yet had the time to actually test it.

Although our style guide does not require this for not zoom level related selectors it might make sense to consistently use >= for tag value based selectors as well (currently you have > for towers/masts and >= for telescopes).

@jeisenbe jeisenbe changed the title [WIP] Update zoom levels by height for masts, towers, monuments and obelisks Update zoom levels by height for masts, towers and telescopes Jan 7, 2019
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 7, 2019

Good idea, I've pushed a commit with this change.

Cranes and chimneys also use height > 50 as a selector for zoom levels. Those can be fixed in another PR.

@imagico imagico merged commit 762b292 into gravitystorm:master Jan 7, 2019
@jeisenbe jeisenbe deleted the vertical-features-zoomlevels branch January 7, 2019 22:37
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.

6 participants