-
Notifications
You must be signed in to change notification settings - Fork 822
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
Render offices as dots + names #3163
Conversation
Rebased to a new master branch
I guess we could close #3163 then? |
Thanks. I started testing and it looks like dark blue is too strong - maybe orange (like in natural=peak) would be better, because it blends better with brown buildings. Example place: |
I agree darkblue stands out a bit too much among surrounding labels. I'll try to tone it down a bit. The colour is otherwise fine to me. Landform-color is much too weak (barely readable) and it doesn't "feel like an office" to me (example). |
I think the dot is OK (but might be darker too), but the name could be then made darker to balance the problem how thin the letters are when using halo. |
Another option would be simple gray text, or perhaps gray text with a gray dot. |
I have to leave. Let me know if you've got other ideas to try and/or state your preference. I am not pushing anything yet. |
Any objections to I changing the colour to #4863A0 (steel blue)? @kocio-pl, looks like you liked the gray version. is that what you prefer? Any other ideas? |
I prefer steel blue. I was trying to indicate that I agree with your comment. |
I've now deployed current master with this PR (with color-value changed to #4863A0) as well as 3164 on my server. Links to the demo-places from above: |
I've had a look at a few places on the link above and I've noticed that Also, |
The intention was to render "amenity-like" offices the same way as other amenities. I don't have a strong opinion on that. I would like know what others prefer. I haven't seen office=taxi neither in the wiki (unlike amenity=taxi) nor in iD presets. There are only 150 office=taxi tags in taginfo. Again, if popular opinion is that it is good to add it - I will do so. It may be good to go through https://taginfo.openstreetmap.org/keys/office#values and check what other office= values should be rendered. Volunteers? |
I still think we should use dark-blue for all offices, and make only one exeption - black government ones. I don't see reason for using purple or brown, which may be confusing for map users. |
sent from a phone
On 7. Apr 2018, at 00:56, lakedistrictOSM ***@***.***> wrote:
I've had a look at a few places on the link above and I've noticed that office=architect, office=accountant and office=insurance all render in purple. Is this intentional?
I think it’s strange architect and accountant (people you will have to commission and then they’ll work a significant time on your project or accounting) are in the same category as an insurance office (not sure what that’s about, but it’s probably either an office where you might go to get an insurance contract (i.e. a kind of “shop”), or a back office where you can’t go at all as a client.
|
I am unsure what to do about it myself. We can approach this in two ways:
Can you state your preferences or other ideas? At the moment I understand the vote is:
By the way, in my opinion offices are equally important to amenity points but we have previously negotiated making them less prominent. Not too concerned about it (in fact, I would be OK making amenity points less prominent in general). Please voice your opinion on that as well. |
Below is a list of the first 240 (out of 3708) office tag values from taginfo. The checkbox means the value is implemented in this PR.
What do you think about rendering all office tag values instead? Perhaps after filtering out discouraged tags. I think this may be a good approach for mapping feedback, would simplify the code and is not harmful (other than a few exceptions we can render all of them the same way). The list:
|
sent from a phone
On 7. Apr 2018, at 10:56, Andrzej ***@***.***> wrote:
Below is a list of the first 240 (out of 3708) office tag values from taginfo. The checkbox means the value is implemented in this PR.
I don’t think it makes sense to implement low usage tags of common things, and I am not sure the definitions are commonly agreed on (e.g. the suggestion to tag some public administration offices with office=government and subtags).
I also don’t know for example about office=property_management, whether this really should relate only to real estate, IMHO it would be much better to use a term like real_estate_management for the avoidance of doubt
In any case it would also be a subclass of financial.
Looking at the list, I believe we should at this time not implement most of these values, while a few (generally the most used) are probably fine. Still the whole office tagging doesn’t look mature for now
|
sent from a phone
On 7. Apr 2018, at 10:56, Andrzej ***@***.***> wrote:
yes 35 367 8.53% ✔ Generic tag for unspecified office type.
rendering office=yes would be giving a strong incentive to people to not add any office class (especially if it is not currently rendered) but rather keep it “yes”. Should only be considered if we render all kinds of offices
|
If possible, all values of office=* should be rendered. IMO, using purple makes them look like shops which they are not - all values should be in dark blue. The only exception might be estate agents, but I don't have a strong opinion either way. |
OK, looks like the consensus is to render all office tags in steelblue colour (and not e.g. purple like I currently do for some of them). I will change that. Things to decide:
|
- Use a toned down steelblue colour instead of darkblue - Removed shop-like offices. Two classes of offices left: larger and regular. Differ only with filtering options. - No longer rendering office=yes
Changed colour of all office tags to steelblue. Removed office=yes. Other than the two potential modifications mentioned above, which would require going back to a drawing board and a new PR, I think the code is in a good shape for merging. |
I am OK with that. I also prefer to first release what we have and focus on possible improvements later. The simplest and the cleanest solution would be to replace the current "other offices" sections with a catch all clause. But, if you want, we can indeed introduce a 3rd category of office pois rendered from z19. BTW, the is_building test is not sufficient. I have seen occurrences of office ways with area=yes, landuse=*, or even nothing at all (just office). It would be good to have something like is_way/is_point. |
These informations are on the database layer level, but then they are somehow unified, so we might add some special values, but I'm not sure if it's really needed. Not using the dots just for the buildings would be OK for me. |
On the other hand - we render shop dots like that (dots are always rendered), so maybe we might keep it the same, at least for now. |
I think it all boils down to Mapnik not implementing: "Render this label but if it doesn't fit render this icon/shape instead". Or at least I think it cannot do that. Without that, the only solution that guarantees that something will remain visible is to always render the icon (plus, optionally, the label). |
Anything left to do in the current version? As suggested, I am deferring the blacklist-type implementation changes until after this PR is merged. |
I have now updated the version on my server to the current version of this PR, but not rerendered all tiles yet. |
http://bl.ocks.org/matthijsmelissen/raw/af7a602c222dbf1ff1a2c0d84ed755b7/#18.00/52.06148/4.52525 Thanks for your work @andrzej-r! |
@kocio-pl I think we can merge this. As you have been most active in this discussion, I'll leave that to you. |
The only thing that we could change before I merge it is that we could start rendering all the "other" offices with just a dot (without name) from z17, because when they have an address number, it looks to me both more cluttered than dots and less intuitive (one building should normally have just one address, multiple numbers suggest more complicated addressing, not more POIs). This place is a good example of this problem. What do you think about it? |
sent from a phone
On 16. Apr 2018, at 19:47, kocio-pl ***@***.***> wrote:
one building should normally have just one address, multiple numbers suggest more complicated addressing, not more POIs
in Italy every access door and gate gets a housenumber, so buildings will usually have more than one housenumber. Basically in Italy addresses are not about buildings but about entrances (and even potential entrances).
Cheers,
Martin
|
Exactly - that's more complicated addressing system and I think that bunch of offices should look different than this. I have created a ticket #962 long time ago for avoiding this. |
I can do that but afair you wanted to implement this feature outside of this pr, so I don't have anything ready just yet. It should be fairly easy to do but it may still take several days because I can't access my computer atm. |
OK, sure. Sorry for such a late suggestion, but this is non obvious change of cartography, so I try to judge all the details from different angles, even if the general idea and code are quite simple. This change would make office priority system less clear, which was quite nice idea, but dots are not as big source of clutter as labels (or address numbers) are, so it makes sense for me to render all the office dots on z17+ and indicate the priority just by initial zoom level for labels (z17+ for important ones, z18+ for less important ones - and z19+ for all the rest in the next step). |
OK. One caveat: filtering options for labels and dots should be in sync. I don't remember the mechanics but that's what was causing rendering errors you found a couple of months ago. I imagine the same is true for other points too. Perhaps, if we find rendering too congested, we can later downgrade all points by 1 zoom level? |
Sure, we can always change anything if it doesn't work. I don't remember - what errors are you talking about? |
I'm referring to the issues you reported in #3061 (comment) although (afair) in that case the dot was rendered at a higher zoom level than the label. I understand you would like to render the dot from z17 and the labels from z17-19 - it may work this way around. I will try that. |
- dots at z17+ - labels of large offices at z17+ - labels of documented office types at z18+ - labels of all other offices at z19+
Done. Works fine here. No double labels or other artifacts we've seen in the past. An example of a previously unsupported office='design' tag/value combination: |
sent from a phone
On 17. Apr 2018, at 02:31, kocio-pl ***@***.***> wrote:
dots are not as big source of clutter as labels (or address numbers) are
the dots are “thick” compared to other signatures, and in color, they probably have a much bigger visual impact than fine black lines like housenumbers or gates.
|
Office dot sizes are consistent with that of other points (shops, amenities). I don't mind changing them all (I agree they could be smaller) but IMHO that's outside the scope of this PR. Edit: at z17 they are smaller (just like other dots). |
z17 z18 z19 |
z17 z18 z19 |
Just my thoughts about first comment here, and what this PR is actually resolving:
|
Thanks for the work, I think it's mature enough and it's time to release it. Further tuning is always possible and might be even needed (just like with towers), but that's a solid base which should not be talked to death on this stage. Soon we will know what other people think and I'm curious about it... It's also good to review which issues have been really fixed by this code, but it's a separate problem which we could solve later. |
Rebased #3061 to a new master branch. No changes to the code. Incremental commits merged into a single one. So far testing yields the same results as with #3061.
RE #108
Fixes #271
Fixes #1922
Fixes #2570