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

Initial pass at rendering golf=* features #4212

Closed
wants to merge 62 commits into from

Conversation

jgruca
Copy link
Contributor

@jgruca jgruca commented Sep 29, 2020

Initial pass at rendering golf=* features. Based largely on imagico/osm-carto-alternative-colors.

Fixes #661

Changes proposed in this pull request:

  • 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 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)

Corrections, feedback, critiques welcome.

Examples:

https://www.openstreetmap.org/#map=15/41.7849/-93.7247

Before/After

Z15
carto-1-zoom15

Z16
carto-1-zoom16

Z17
carto-1-zoom17

https://www.openstreetmap.org/#map=16/41.8705/-93.2144

Before/After

Z14
carto-2-zoom14

Z15
carto-2-zoom15

Z16
carto-2-zoom16

Z17
carto-2-zoom17

https://www.openstreetmap.org/#map=16/41.2767/-95.7367

Before/After

Z15
carto-3-zoom15

Z16
carto-3-zoom16

Z17
carto-3-zoom17

hiddewie and others added 4 commits July 5, 2020 15:29
- 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.
@jgruca jgruca mentioned this pull request Sep 29, 2020
Copy link
Collaborator

@jeisenbe jeisenbe left a 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.

symbols/golf_pin.svg Outdated Show resolved Hide resolved
symbols/golf_pin.svg Outdated Show resolved Hide resolved
style/golf.mss Outdated Show resolved Hide resolved
style/golf.mss Outdated Show resolved Hide resolved
style/golf.mss Outdated Show resolved Hide resolved
symbols/golf_pin.svg Outdated Show resolved Hide resolved
@jeisenbe jeisenbe added the new features Requests to render new features label Sep 29, 2020
@jeisenbe jeisenbe added this to the New features milestone Sep 29, 2020
@imagico
Copy link
Collaborator

imagico commented Sep 29, 2020

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:

  • the overall golf course color is unified with the campsite color (removing one more green tone from this style)
  • fairway and driving_range are in grass color
  • green is in pitch color
  • bunker is using sand color
  • rough is using a fine grain pattern mixing grass and scrub
  • symbols and lines are in black.

Line labels for artificial features in @oblique-fonts is also not matching our normal text styling traditions.

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
@jgruca
Copy link
Contributor Author

jgruca commented Sep 29, 2020

I have pushed up some changes to try to address some of the comments above.

  • SVG files have been cleaned up and specific color references moved to .mss file
  • Reduced the number of new colors introduced to just one, a lightened @grass for the green
  • Updated the golf course color to the same as campsite
  • tee, fairway, rough, and driving_range are now all @grass color
  • The golf=hole line is now a solid @leisure-green line, labeled with a book font
  • Added a texture for golf=rough

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.

Here are updated screenshots, same locations as above.
carto-1-zoom15
carto-1-zoom16
carto-1-zoom17

carto-2-zoom15
carto-2-zoom16
carto-2-zoom17

carto-3-zoom15
carto-3-zoom16
carto-3-zoom17

@imagico
Copy link
Collaborator

imagico commented Sep 30, 2020

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

- id: stations
and for linestring features using a separate layer - see amenity-points:

- id: amenity-points

and amenity-line:

- id: amenity-line

You should also consider integrating point symbol/label rendering into the normal point symbol layer because otherwise you will get difficulties with correct prioritization.

@jgruca
Copy link
Contributor Author

jgruca commented Oct 1, 2020

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()

So in this case, golf=hole, pin, and tee points should be moved into a separate layer, leaving golf=hole linestrings in the golf-lines layer. I imagine this can drastically simplify the SQL for golf-lines. Should the ref label for golf=hole be moved into the text-line layer?

You should also consider integrating point symbol/label rendering into the normal point symbol layer because otherwise you will get difficulties with correct prioritization.

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
@jgruca
Copy link
Contributor Author

jgruca commented Oct 5, 2020

I have attempted to address some of the issues with these latest changes.

  • golf-line, as a layer, now contains just golf=hole as a line
  • golf=hole and golf=pin, as points, have been moved to amenity-points. Please let me know if this is not the appropriate layer.
  • The rendering for the ref label of golf=hole has been moved to the text-line layer
  • The point previously rendered in the center of golf=tee polygons has been removed entirely, as I haven't found any other golf course maps that show this information
  • The rendering for golf=rough has been tweaked a little by setting opacity on the pattern; see the screenshot below.

carto

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.

@tpetillon
Copy link
Contributor

tpetillon commented Oct 8, 2020

Wouldn't it be more consistent to use @stadium (or @leisure, as they're identical) for the background? Semantically, a golf course is closer to a football pitch or a tennis court than a camp site.

It would look like this:
image
(Copying the outline of other sports facilities as well.)

Going further, @leisure and @campsite could probably be merged, decreasing the number of shades of green in the style.

@jgruca
Copy link
Contributor Author

jgruca commented Oct 9, 2020

I have no issues with a switch to @leisure for the background color. How do others feel?

@jgruca jgruca requested a review from jeisenbe October 9, 2020 01:16
@imagico
Copy link
Collaborator

imagico commented Oct 13, 2020

Sorry, but no, this does not work for me at all. The introduction of the color that is defined as @leisure as well as its extension as a residual color for a broad range of semantically largely unrelated features - playground, fitness_station, recreation_ground (but not park!), dog_park, stadium, water_park - was done without consensus and countered previous efforts in designing a more consistent, systematic and homogeneous color scheme. See #2268, #2891, #2954, #2964, #3843, #2250 (comment), #2270.

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.

@jgruca
Copy link
Contributor Author

jgruca commented Oct 14, 2020

Thanks for the direction there. I was unaware of the history of some of the color choices, and appreciate the context. I have reverted from @leisure back to @campsite.

I'd like this PR to limit itself to rendering golf=* features rather than a comprehensive refactor of sports infrastructure.

My hesitation in using @pitch as the color for the golf green is that 1) a sports pitch normally includes the full area of play and 2) on a course with water hazards, it is a blue enough green that it might be confused for water:

image

Having said that, you are correct in saying that the current golf=green color is too close to @campground. In fact, I propose that simply switching the color to use @campground accomplishes the twin goals of differentiating the green from the fairway and decreasing the number of colors in the style.

I infer from your comments a pattern for golf=bunker is unnecessary. I have removed that.

Since a number of things have changed since the first commit, I am summarizing the style changes in this PR below.

Tag Example Summary of changes Justification
leisure=golf_course
leisure=miniature_golf
image Fill now @campground Removing @golf_course, a single-use color. Semantically, a campground could be considered similar to a golf course: both are large areas, usually featuring natural vegetation, paths, and maintained areas for recreational activities.
golf=green image Fill now @campground, outline now @grass A golf green is an important feature to differentiate from the fairway. Using @campground allows contrast against the @grass of the fairway and does not introduce a new color. Providing an outline guarantees it will stand out even when it is wholly separate from the fairway.
golf=tee
golf=fairway
golf=driving_range
image image Fill now @grass Filling these areas with @grass -- their standard surface -- formalizes rendering of these tags, which are frequently mistagged as landuse=grass so they will render at all.
golf=bunker image Fill now @sand Filling these areas with @sand matches natural=sand and will help discourage those who would mistag bunkers as natural=sand for rendering purposes.
golf=rough image Fill now @grass, pattern now golf_rough.svg Using a pattern in these areas differentiates between the textures of two kinds of grass on the course without introducing a new color.
golf=hole (way) image Line with stroke of @leisure-green, labeled with ref of hole Labels the hole and direction of play; the color matches golf course label.
golf=hole (node)
golf=pin
image Icon golf_pin.svg, labeled with ref of hole, both @leisure-green color Labels the actual hole, called out with an icon for visibility, in a color matching other new labeling.

I think the only potential disagreements here are 1) the color of golf=green and 2) the current pattern on golf=rough along with its effect on the perceived color of that area. The changes I have outlined here are merely personal preferences, and if you tell me that changing them to x and y will result in a merged PR, I will make those changes.

@imagico
Copy link
Collaborator

imagico commented Oct 14, 2020

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.

@jgruca
Copy link
Contributor Author

jgruca commented Oct 15, 2020

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.

Yes, this is a valid concern. I have addressed it in the current PR by specifying an outline (line-width: 2; line-color: @grass;) around the green. In my admittedly-unscientific survey of local golf courses, there were three configurations of fairway and green that I saw. Most common was when the green was totally surrounded by fairway; in this case, there is no difference in rendering. Second most common was when the fairway was adjacent to and sharing nodes with the green. In this case, the outline blends well with the fairway and serves to delineate the opposite side of the green. And in the case you describe, where a green is placed totally separate from the fairway, the green is outlined with the grass color to distinguish it from the surrounding course.

image image
image image
image image

@imagico
Copy link
Collaborator

imagico commented Oct 15, 2020

Though visually this idea looks smooth it suffers from several problems IMO:

  • it does not highlight the significance of the green for the game.
  • the line signature can be easily mistaken for a mapped geometry (a slim grass/fairway strip around the green) which impairs intuitive mapper feedback.
  • it still does not provide functional feedback on the green neing properly mapped in case the green is fully surrounded by a fairway polygon.

@jgruca
Copy link
Contributor Author

jgruca commented Oct 16, 2020

How would you feel about @pitch for tee, fairway, and driving_range, and @grass for green?

image

I'm still not a huge fan of @pitch, however, this accomplishes:

  • The areas of the map colored @pitch are less likely to be the same size/shape as smaller bodies of water
  • The larger area covered in that color helps drive home that the areas are all associated with one another, especially if golf=hole is mapped on them
  • golf=green is visible apart from the fairway and distinguishable from a cutout in a fairway

You could still make the argument that then golf=green is not distinguishable from a simple grass area, and that using @grass confers no sports-related semantics. However, this is the same situation you have when golf=fairway is @grass and golf=green is @pitch, just reversed.

@jgruca
Copy link
Contributor Author

jgruca commented Feb 7, 2021

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?

@jeisenbe
Copy link
Collaborator

jeisenbe commented Feb 8, 2021

@jgruca I still think it does not work to use pitch color plus a texture to represent areas of golf=rough while using @grass for golf=green, and @imagico has also mentioned some arguements in favor of using @grass for the fairway and @pitch for the green ( #4212 (comment) and #4212 (comment)).

@jgruca
Copy link
Contributor Author

jgruca commented Feb 16, 2021

I see. So is there consensus for using @grass for golf=fairway/tee/driving_range and @pitch for golf=green? Personally, I still prefer the idea of using @pitch for the fairway (see my reasoning #4212 (comment)) which @imagico mentioned made some sense (#4212 (comment)). However, I'm willing to swap those colors if the concerns aren't warranted.

That just leaves the matter of how to handle golf=rough. @jeisenbe, you specifically mention @pitch with a texture doesn't work for you; would @grass with a texture be acceptable?

rough-texture

kocio-pl and others added 14 commits April 2, 2021 10:32
…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
@jgruca
Copy link
Contributor Author

jgruca commented Apr 19, 2021

I have updated the PR with the previously-mentioned proposal; namely, @grass for golf=fairway/tee/driving_range/rough and @pitch for golf=green. Additionally, golf=rough has a texture applied to differentiate it from the fairway.

@pnorman
Copy link
Collaborator

pnorman commented Apr 20, 2021

With the update you're now including unrelated commits in your branch. It looks like a screwed up rebase happened.

@jgruca
Copy link
Contributor Author

jgruca commented Apr 20, 2021

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?

@pnorman
Copy link
Collaborator

pnorman commented Apr 20, 2021

I think that'd be easiest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new features Requests to render new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rendering for golf=*