-
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
Initial pass at rendering golf=* features #4212
Conversation
- Update landcover layer to include golf area features - Add golf-lines layer - Add style/golf.mss and specify rendering for: - golf=green and golf=tee (light green color between @grass and @park; tee with gray point labeled with ref) - golf=fairway and golf=driving_range (@grass) - golf=bunker (color and pattern for @sand and beach) - golf=rough (darker green between @scrub and @grass) - golf=hole as way (dashed line, darker green derived from @golf_course; labeled with ref) - golf=hole as point and golf=pin (symbols/golf_pin.svg; labeled with ref) Based largely on imagico/osm-carto-alternative-colors.
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.
Thanks for working on this, @jgruca
I think it will be beneficial to show these features, and this initial draft already looks pretty good.
But there are a few lines in the code that can be simpified, and I would like us to think about the color choices. It would be best to avoid adding too many new fill colors for areas, and any new colors chosen need to be intuitive and not easily confused with other features. This can be tricky to get just right.
Thanks for taking on working on this rather complicated topic. As @jeisenbe mentioned the choice of colors is not going to work - we already have a real lot of green polygon fill colors in this style so introducing even more of them is not a good idea. The green line color is also going to be confusing with the color of bridleways. I would suggest trying to do this without introducing new colors. For reference - the color scheme used in the ac-style can be seen here:
Line labels for artificial features in Note there are quite a few subtle implications in the code from the ac-style regarding interpretation of the tagging that would need to be discussed - in particular what variants of tagging ref are supported and how to render them. For consistency with the rest of the style you should probably also not use a compound geometry type layer but unify points and polygons using ST_PointOnSurface() and separate out the line features to a different layer. |
- golf course color now same as campsite - pin/hole icon colored green - @leisure-green used for dark green items - text no longer oblique - hole line no longer dashed - fold in higher zoom rule - add rough pattern
I have pushed up some changes to try to address some of the comments above.
I have no particular attachment to these colors and I am happy to adjust as needed, as they still differ a bit from the ac-style you linked above. If you could point me to examples/docs of separating line features on a separate layer, or further elaborate on that, I will certainly give that a try. |
The golf-lines layer combines point, polygon and linestring features in a single layer. This we so far don't do anywhere else in this style. The common practice is to combine points and polygons for label/symbol rendering using ST_PointOnSurface() - see for example openstreetmap-carto/project.mml Line 1316 in b10aef3
openstreetmap-carto/project.mml Line 1444 in b10aef3
and amenity-line: openstreetmap-carto/project.mml Line 1656 in b10aef3
You should also consider integrating point symbol/label rendering into the normal point symbol layer because otherwise you will get difficulties with correct prioritization. |
So in this case,
Is the amenity-points layer considered the normal point symbol layer here? Organizationally, does it still make sense to have a separate .mss file just for golf features, or would you prefer to have style rules moved into the files more closely associated with their layers? |
- rename golf-lines to golf-line to match other line layers - specifically select golf=hole linestrings for golf-line layer - move golf=hole/golf=pin points to amenity-points layer - add golf=hole to text-line layer for label rendering
I have attempted to address some of the issues with these latest changes.
I have made changes to the SQL for some of these layers with limited knowledge of the size or indexes of these tables, so if there are optimizations or changes that are needed there please let me know. |
I have no issues with a switch to |
Sorry, but no, this does not work for me at all. The introduction of the color that is defined as If you want to have a common color for different kinds of sports infrastructure that is fine with me - provided that the color suggested for that fits with the overall color system. But engrossing playground, dog_park and recreation_ground with that will not work. Regarding your modified color scheme - i think your choice of color for green is too close to the campsite color: http://davidjohnstone.net/pages/lch-lab-colour-gradient-picker#def6c0,daf0c4 Using the pitch color seems the natural choice here - and in my eyes it also would highlight the structure of a golf course. For rough - you would need to check how it relates to the other darker greens in our style, in particular scrub and wood. A rough on a golf course does not feature taller vegetation, it is usually just longer grass so if the color is closer to scrub or wood than to grass then it is possibly quite misleading. Generally speaking given the problems with adding more colors to the style for general readability i would expect you to support any introduction of a new color or color-pattern combination to be supported with convincing reasoning. We cannot support this merely as a local convenience choice. For rough the need for distinction between the cut grass of a fairway and the longer grass for the rough area is a clear necessity. For bunker and green i see no convincing arguments so far. |
This reverts commit 13e8f14.
Thanks for the additional details on your reasoning. I get your point about the pitch color's limited contrast to water - which is however not limited to use on golf courses. Cause of this is the pitch color used here (see #2363) - the ac-style uses a different color which does not have this problem. Regarding the argument that leisure=pitch usually covers the full area a sport is played on - that is not universal, table tennis is an obvious example, long distance running where most of it happens on the street while the finish is on a stadium track is another. The issue with using the same color for golf=green and leisure=golf_course is that there would be no visible distinction between golf=green being mapped and not being mapped. The use for the same color for campsite and golf_course is in my eyes not so much a case of functional similarity but of classical overloading. Both are rendered with a symbol in addition to the polygon fill so there is very little risk of confusion. This is not ideal but a reasonable solution IMO given the color constraints. |
Yes, this is a valid concern. I have addressed it in the current PR by specifying an outline ( |
Though visually this idea looks smooth it suffers from several problems IMO:
|
How would you feel about I'm still not a huge fan of
You could still make the argument that then |
It has been a few months now. Are there any other opinions, objections, or suggestions regarding this PR? Anything I can do to move this forward? |
@jgruca I still think it does not work to use |
I see. So is there consensus for using That just leaves the matter of how to handle |
…e-v5 Update README.md on v5
…tgis Update DB dockerfile to use the postgis image
- Update landcover layer to include golf area features - Add golf-lines layer - Add style/golf.mss and specify rendering for: - golf=green and golf=tee (light green color between @grass and @park; tee with gray point labeled with ref) - golf=fairway and golf=driving_range (@grass) - golf=bunker (color and pattern for @sand and beach) - golf=rough (darker green between @scrub and @grass) - golf=hole as way (dashed line, darker green derived from @golf_course; labeled with ref) - golf=hole as point and golf=pin (symbols/golf_pin.svg; labeled with ref) Based largely on imagico/osm-carto-alternative-colors.
- golf course color now same as campsite - pin/hole icon colored green - @leisure-green used for dark green items - text no longer oblique - hole line no longer dashed - fold in higher zoom rule - add rough pattern
- rename golf-lines to golf-line to match other line layers - specifically select golf=hole linestrings for golf-line layer - move golf=hole/golf=pin points to amenity-points layer - add golf=hole to text-line layer for label rendering
This reverts commit 13e8f14.
…streetmap-carto into render-golf-features
I have updated the PR with the previously-mentioned proposal; namely, |
With the update you're now including unrelated commits in your branch. It looks like a screwed up rebase happened. |
Oops, yes, this includes me rebasing my changes on top of master. Given the number of changes that have occurred in this PR, would it be easier to open a new PR with the current squashed set of changes and an accurate summary at the top? |
I think that'd be easiest. |
Initial pass at rendering golf=* features. Based largely on imagico/osm-carto-alternative-colors.
Fixes #661
Changes proposed in this pull request:
landcover
layer to include golf area featuresgolf-lines
layergolf=green
andgolf=tee
(light green color between@grass
and@park
;tee with point labeled with
ref
)golf=fairway
andgolf=driving_range
(@grass
)golf=bunker
(color and pattern for@sand
and beach)golf=rough
(darker green between@scrub
and@grass
)golf=hole
as way (dashed line, darker green derived from@golf_course
; labeled withref
)golf=hole
as point andgolf=pin
(symbols/golf_pin.svg; labeled withref
)Corrections, feedback, critiques welcome.
Examples:
https://www.openstreetmap.org/#map=15/41.7849/-93.7247
Before/After
Z15
Z16
Z17
https://www.openstreetmap.org/#map=16/41.8705/-93.2144
Before/After
Z14
Z15
Z16
Z17
https://www.openstreetmap.org/#map=16/41.2767/-95.7367
Before/After
Z15
Z16
Z17