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

Dont't display residential and apartment buildings as light, they are wi... #434

Closed
wants to merge 1 commit into from

Conversation

MorbZ
Copy link

@MorbZ MorbZ commented Mar 23, 2014

...dely accepted as "normal" buildings

@matthijsmelissen
Copy link
Collaborator

This issue has been reported before: #68.

I agree this issue is important, also in line with the Dutch BAG import, which looks rather ugly with the current rendering style: http://www.openstreetmap.org/#map=18/51.58763/4.76092

Because this issue is duplicate, I close it now.

@matthijsmelissen
Copy link
Collaborator

Sorry, I should learn to read before I type. I didn't notice that this was actually a pull request rather than a bug report. I reopen the issue.

I think 'house' should be removed from the set of light building outlines as well. I don't see a reason to render houses lighter than buildings of which we don't know the function. @MorbZ What do you think?

I think this should be a high-priority issue, also given the Dutch import.

@pnorman pnorman added cartography and removed bug labels Mar 28, 2014
@MorbZ
Copy link
Author

MorbZ commented Mar 28, 2014

I'm unsure about "house" because it's quite unspecific and could be almost anything. On the other hand building=yes is even more unspecific and it's displayed as strong, so I would probably remove "house". I would also consider adding "shed" to the list. When there are no further comments on this topic I will open another pull request.

@vincentdephily
Copy link

House, detached, and terrace definitely should get the normal rendering, not the light one. House is the main culprit that originally motivated me (detached and terrace being a subtype of house IMHO). I had made this list (sorted by frequency in the db) in my patch on issue #68 (which I never followed through, sorry):

'garage','roof','garages','service','shed','shelter','cabin','storage_tank','tank','support','glasshouse','mobile_home','kiosk','silo','canopy','tent'

For reference, the original is

 'residential','house','garage','garages','detached','terrace','apartments'

and MorbZ's patch suggests

'house','garage','garages','detached','terrace'

You could argue about my list VS MorbZ's, but I unsurprisingly like my one better. I've argued about house/detached/terrace already. Shed is a no-brainer given that we have garage. Support, canopy, and tent should be uncontroversial. The others are a greyer area, but I stand by them.

vincentdephily referenced this pull request in vincentdephily/openstreetmap-carto Mar 28, 2014
As discussed in issue gravitystorm#68, remove various kinds of residential values, and add a few other values.
@matthijsmelissen
Copy link
Collaborator

I would also prefer @vincentdephily's list (except for building=service, of which I'm not sure what it means).

I also wouldn't mind not having the distinction at all. Are there any other maps in the world that make a similar distinction between important and unimportant buildings?

@gravitystorm
Copy link
Owner

It's a common thing on Ordnance Survey maps to highlight "important" buildings - schools, town halls etc - with a bold outline. Go to the OS OpenSpace page, scroll down, type "sw1a 1aa" into the postcode box and zoom in and out a bit to see different map types.

But what we have at the moment is more of a mess, since the vast majority of buildings have the same tag (building=yes) when those same buildings will generally look different with a more accurate tag.

I haven't played much with buildings, but I'd prefer a small number of important buildings and have building=yes in with the rest of houses/terraces etc since there's a 95% chance that's what =yes means. This PR is doing one of those things (making =yes and =house the same).

@matthijsmelissen
Copy link
Collaborator

Yes, I think highlighting buildings of interest (public buildings, churches, museums etc.) is common on maps and something that would be nice to have in OSM as well. What I do not like that much, and what is not common on other maps AFAIK, is painting sheds etc. different from regular buildings.

In any case I think @MorbZ's and @vincentdephily's proposals would be a large improvement.

@vincentdephily
Copy link

We already have some important buildings rendered differently, for example churches are grey. I'm not sure wether switching from an amenity= matching to a building= one would be better. It seems reasonable to believe that important buildings will have plenty of tags to identify them, whereas unimportant ones are likely to just have a building tag.

Because most buildings in the db are of the indeterminate "yes" type, whatever gets a different rendering from "yes" should constitue a minority of the data, to reduce the number of false-negatives (building which would change rendering if they were tagged more precisely). Removing 'house' and 'residential' (the most common values after 'yes') from the light-list reflects that.

Considering building=service, I admit I'm not sure what it corresponds to. Maybe it's safer to remove it.

On the "remove all distinction" argument, see issue #68 for other opinions. In my tests, looking at a residential area (mainly house, garage, shed) really looks more readable with sheds and garages dimed.

Lastly, if you're going to commit a change to the list of light buildings... Please consider changing the list of non-buildings too. Current:

'no','station','supermarket','planned'

Proposed:

'entrance','collapsed','construction','no','ruins','ruin','proposed','planned'

@matthijsmelissen
Copy link
Collaborator

I'm not sure wether switching from an amenity= matching to a building= one would be better.

I don't like implicitly assuming that amenity= implies a building. For example, I believe that amenity=place_of_worship might also suitable for some places that are not buildings. The tag amenity=public_building has a similar problem (although the name does suggest it should only be applied to buildings).

Apart from that, I agree with everything @vincentdephily has said.

@vincentdephily
Copy link

My mention of a "amenity= matching" was a simplification. I didn't check how matching was done, I just know that it isn't just looking at the building value. The point is that for important features we can make a more informed decision by looking at more than the building tag; for unimportant features we typically have no other info than the building tag.

@dieterdreist
Copy link

2014-03-28 19:01 GMT+01:00 vincentdephily notifications@github.com:

My mention of a "amenity= matching" was a simplification. I didn't check
how matching was done, I just know that it isn't just looking at the
building value. The point is that for important features we can make a more
informed decision by looking at more than the building tag; for unimportant
features we typically have no other info than the building tag.

+1, I agree that rendering building types is mostly out of the scope of a
general purpose map. On the other hand it will incentive more detailed
tagging of building types if this kind of information was visualized. E.g.
railway=station is or will sooner or later be tagged on the whole station
(area with buildings, rails and platforms etc.) but with
building=train_station you could still see the main station building.

@MorbZ
Copy link
Author

MorbZ commented Apr 3, 2014

For me it's important to remove "apartment" and "residential" from the list. @vincentdephily did this in his list too, plus he obviously put more thought into it so I would go with his as well.

matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this pull request Apr 21, 2014
- The building types residential, house, detached, terrace and
  apartments are no longer light.
- The building types roof, service, shed, shelter, cabin,
  storage_tank, tank, support, glasshouse, mobile_home, kiosk
  silo, canopy and tent are now light.

This commit is based on a comment by @vincentdephily:
gravitystorm#434 (comment)

This closes gravitystorm#68 and supersedes gravitystorm#434.
@opani-dk
Copy link

I'm missing "greenhouse" (used 51000 times) from the list of light buildings while the much less used "glasshouse" (used 1700 times) is there. They probably mean the same thing and greenhouse is the value documented in the wiki.

@matthijsmelissen
Copy link
Collaborator

Thanks for pointing out, I added greenhouse to the list of light buildings in #490.

@vincentdephily
Copy link

While I don't want to bikeshed this, I had intentionally left greenhouse out but glasshouse in. The rationale is that modern greenhouse growing can take place in huge warehouse-like buildings, sometimes not transparent at all, which would not be considered "light" when ground-truthing. Glasshouse on the other hand suggest the orginal garden-variety building, which I agree should be "light". It's hard to make a jugement call on this one, because I don't think that current tagging is consistent or informative enough.

@dieterdreist
Copy link

2014-04-30 15:27 GMT+02:00 vincentdephily notifications@github.com:

While I don't want to bikeshed this, I had intentionally left greenhouse
out but glasshouse in. The rationale is that modern greenhouse growing can
take place in huge warehouse-like buildings, sometimes not transparent at
all, which would not be considered "light" when ground-truthing. Glasshouse
on the other hand suggest the orginal garden-variety building, which I
agree should be "light". It's hard to make a jugement call on this one,
because I don't think that current tagging is consistent or informative
enough.

I don't think that "glasshouse" is a building type, while "greenhouse" is.
"Glasshouse" is roughly refering to the construction material but is an
ambivalent term (e.g. glass facades vs. structural glazing), IMHO should be
avoided completely as building type.
I am also not sure whether it makes sense to distinguish between "light"
and "default" constructions, as also this term is ambiguous (e.g. wooden
frame buildings are generally considered "light constructions" as are
concrete shells). A more useful distinction might be density (i.e. number
of floors) or usage / functions.

@matthijsmelissen
Copy link
Collaborator

I think glasshouse versus greenhouse needs further discussion on the tagging mailing list. I propose not to wait for that, and include greenhouse in the list of light buildings for the time being, i.e. to go ahead with the propodal as it is now.

@vincentdephily
Copy link

+1 for merging this as-is, and tuning details later.

dieterdreist's comment shows that we're still unclear about the purpose of the 'light' category. To me the construction method/material isn't important. I'm not sure how you'd apply a density concept. It's more about the function : light buildings are there to "support" the main buildings. You wouldn't see a light building without an associated normal building nearby. This is obvious for "shed", "tank", and "canopy", less so for "greenhouse". I also include "not-really-a-building" features like "roof", "shelter", and "tent", because it just feels right.

But once again: let's commit this improved version ASAP, please don't wait until it's tuned to perfection.

@matthijsmelissen
Copy link
Collaborator

This has been superseded by #490. @MorbZ thank you for initial work.

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.

7 participants