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

Add rendering for amenity=bicycle_parking, change rendering for amenity=parking #1364

Merged
merged 4 commits into from
Mar 21, 2015
Merged

Conversation

matkoniecz
Copy link
Contributor

Improved version of #1361

  • unified rules for amenity=parking and amenity=bicycle_parking
  • small parkings are in low-priority layer what fixes problems with bicycle parking next to bus station blocking an important label
  • reduces undue prominence of amenity=parking
  • icons for big parkings are rendered earlier
  • labels of small parkings are rendered alter or for really small ones not rendered

Still work in progress - I am reviewing how thresholds work for various areas and it will be necessary to squash commits.

Comparisons below, unless noted otherwise are made with rendering from #1361

https://cloud.githubusercontent.com/assets/899988/6600231/b8244224-c80e-11e4-8845-2b78425c7de2.png
https://cloud.githubusercontent.com/assets/899988/6600204/957a1a14-c80e-11e4-932c-f75ac788177d.png
https://cloud.githubusercontent.com/assets/899988/6600201/93096b86-c80e-11e4-955b-100544e5e8e5.png

@matkoniecz matkoniecz changed the title Bicycle parking Add rendering for amenity=bicycle_parking, change rendering for amenity=parking Mar 11, 2015
@matkoniecz
Copy link
Contributor Author

I am considering moving all parking symbols into low priority layer.

Also, here is example for label vs bicycle_parking (on z17):

https://cloud.githubusercontent.com/assets/899988/6600361/7789f122-c80f-11e4-8739-75ddeadad95d.png

@kocio-pl
Copy link
Collaborator

Looks perfect for me!

I would also love to move all parking symbols from z16 to z18. If the parking is big enough, we always see the yellow area, so it's hard to miss, but P symbol should not be bigger than small areas (like 5 places on the street), so z17 is still not enough:

http://www.openstreetmap.org/#map=17/52.22925/20.99511

Edit: You mean just a bicycle parking?

@SomeoneElseOSM
Copy link
Contributor

(re parking symbols at z18) isn't there a danger here that the rendering is being tailored to work only in micro-mapped central european cities?

For example, for my own use I changed the rendering of the "P" symbol to z13 so that if you're looking at a broad countryside area, you can see where to park:

2660

Edit: The location is here: http://www.openstreetmap.org/#map=13/53.2127/-1.8103

Obviously z13 is far too low a zoom level for a more general purpose map - I wouldn't suggest it for that. But z18 is surely too far in the other direction.

@matkoniecz
Copy link
Contributor Author

@SomeoneElseOSM

For example, for my own use I changed the rendering of the "P" symbol to z13 so that if you're looking at a broad countryside area, you can see where to park:

Can you link this location? Seems to be a well-mapped area with low density of features, I was looking for such place for a long time.

@SomeoneElseOSM
Copy link
Contributor

(location linked in previous comment)

@matkoniecz
Copy link
Contributor Author

@SomeoneElseOSM

re parking symbols at z18

In low density areas parking symbols would appear from z17. Areas are anyway visible much earlier.

isn't there a danger here that the rendering is being tailored to work only in micro-mapped central european cities?

I am currently rendering my other test locations, list of where I am testing is available at https://github.com/mkoniecz/CartoCSSHelper/blob/master/CartoCSSHelper.rb (function test_tag_on_real_data). I would welcome suggestions for additional well-mapped places with different situation.

@matkoniecz
Copy link
Contributor Author

@kocio-pl

Edit: You mean just a bicycle parking?

In this pull requests I am proposing applying the same rules for parkings and bicycle parkings.

http://www.openstreetmap.org/#map=17/52.22925/20.99511

In that case parkings would no longer block shop icons and parking icons would disappear from z17.

@matkoniecz
Copy link
Contributor Author

@SomeoneElseOSM

Edit: The location is here: http://www.openstreetmap.org/#map=13/53.2127/-1.8103

Thanks, added to my list. In that case parking was mapped as node so it would appear from z17 instead current z16. I mapped it as area, but it is not going to change anything as it is not covering significant area.

@RobJN
Copy link

RobJN commented Mar 11, 2015

Surely parking in the city is what we should prioritise the rendering to. In the countryside you can just pull up on the side of the road (assuming it's a safe location) as there aren't all the parking restrictions that you have in a city.

@matthijsmelissen
Copy link
Collaborator

@mkoniecz Can you try to find a version of the svg without halo?

@matkoniecz matkoniecz self-assigned this Mar 13, 2015
matthijsmelissen and others added 4 commits March 14, 2015 09:48
* Add bicycle_parking icon from z18 or (for areas) 750 pixels, whichever is
  earlier.
* Add yellow background for bicycle_parking (like regular parking).

Thanks to @nebulon42 for the icon.

This resolves #591 and part of #108.
small parkings are moved to low priority layer
names displayed earlier for big parkings, later for small ones
access status displayed for private bicycle parkings
@matkoniecz matkoniecz removed their assignment Mar 14, 2015
@matkoniecz
Copy link
Contributor Author

OK, now it is prepared for review. Is anybody interested in seeing rendering of some particular location?

@pnorman
Copy link
Collaborator

pnorman commented Mar 15, 2015

assigning to @gravitystorm - he came to the conclusion not to render in #591 (comment)

@matthijsmelissen
Copy link
Collaborator

Looks good to me.

Maybe add a comment to explain why the parking icon rule is duplicated? Or even better, avoid the duplication entirely by doing something like

.amenity-points[amenity = 'parking'][zoom >= 17],
.amenity-low-priority[amenity = 'parking'][way_pixels > 750] {

@pnorman
Copy link
Collaborator

pnorman commented Mar 19, 2015

You've mentioned changing the amenity=parking rendering but I don't see changes - what are they?

@matkoniecz
Copy link
Contributor Author

@pnorman

You've mentioned changing the amenity=parking rendering but I don't see changes - what are they?

currently:

Render amenity=parking icons from zoom 16.

after this PR:

Render amenity=parking, amenity=bicycle_parking icons for areas larger than 900 pixels and display all amenity=parking, amenity=bicycle_parking icons from zoom 17 as low priority icons.

Also, display labels for parkings for areas larger than 900 pixels.

Intent of this change is to lower prominence of parkings, especially in areas where are other more important features, render earlier icons for gigantic parking. Also, to render amenity=bicycle_parkings, especially really big ones,

@matthijsmelissen matthijsmelissen merged commit 947dacd into gravitystorm:master Mar 21, 2015
@matkoniecz matkoniecz deleted the bicycle-parking branch July 2, 2015 20:02
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.

8 participants