-
Notifications
You must be signed in to change notification settings - Fork 823
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
Adjust width of hedge & citywall rendering from z15 to z19 and add z20, change hedge to @forest #3847
Conversation
Render hedge thinner at z16 and z17, thicker at z19. Render citywalls slightly thinner at z15, same at z16 and z18, thicker at z17 and z19, compared to previous: 1.0px at z15 to 4px at z19.
The current hedge color was only 1 point saturation less than forest color; lightness and hue were identical.
Mersch, Luxembourg - citywalls (old town fort/castle)https://www.openstreetmap.org/#map=16/49.7510/6.1055 |
edfccac
to
b3f1d80
Compare
I've pushed a new commit which adds a color variable |
@jeisenbe Could you also split this PR into simple subjects? If not 3 separate tickets, at least citywalls in one PR and hedge in a second one, please. Another issue: when taking screenshots, could you please make them with the same view window? Some effects are so subtle, that I make image flipping to see the change, but when they are different size, I can't even spot the problem... |
I wanted to include the 2 features in 1 PR, because the new widths are the same at each zoom level, so I though it was good to consider the changes together. And the color change for hedges is very minor, so not worth discussing separately. But if you will review 2 separate PRs, I would be happy to split this one. ;-)
Some of the images above are the same before and after. ;-) I'm sneaky like that. Citywalls are unchanged at z16 and z18. Hedges are unchanged at z18, so z18 is the same in the before and after images, except for an imperceptible change in the color of hedges: I used to export the images from Kosmtik so that they would be aligned before/after, but @pnorman mentioned that it's better to take screenshots since exporting small chunks leads to difference in the rendering, especially label placement. Unfortunately this means the before and after image are not perfectly aligned. I'm not sure if there is an easy solution? |
There are screenshot tools allowing to redo screenshot of exactly the same part of the screen. I am using Shutter (on Linux) that has looped arrow icon on top left of interface allowing to do this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically this looks fine, but I haven't run it myself, and don't have an opinion on the cartographic changes since I don't deal with city walls much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine and is a good improvement on the line width stepping IMO.
Thanks. I'll merge this in a couple of days if there are no other comments or objections. |
Adjust width of hedge & citywall rendering from z15 to z19 and add z20, change hedge to @forest
Fixes #3845
Related to #3747
Changes proposed in this pull request:
barrier=hedge
andbarrier=city_wall
/historic=citywalls
at z20 with slightly greater width.Explanation
Background:
barrier=city_wall
orhistoric=citywalls
) are rendered from z15, while hedges only render from z16.Changes
Test rendering with links to the example places:
Luxembourg City
z15 before
z15 after
z16 before
z16 after (nearly the same; small hedge is different only)
z17 before
z17 after
z18 before
z18 after (unchanged)
z19 before
z19 after
z20 before
z20 after
Hedge vs treerow
z16 before
z16 after
z17 before
z17 after
z18 before
z18 after (unchanged except hedge color - not really visible)
z19 before
z19 after
z20 hedges after