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

Change Color or Layering of Golf - Rough to Darker Green #3650

Closed
chadrockey opened this issue Jan 19, 2019 · 17 comments
Closed

Change Color or Layering of Golf - Rough to Darker Green #3650

chadrockey opened this issue Jan 19, 2019 · 17 comments

Comments

@chadrockey
Copy link

chadrockey commented Jan 19, 2019

Expected behavior

Golf - Rough Area layer should be the lowest priority layer and the darkest in color.

Areas of rough, areas of fairway, and "greens" should be distinguishable and look good on the map.

Actual behavior

When added to a course bounding areas that contain rough, the rough covers or is the same color as greens and fairways. This causes the map to look bad and remove detail that users add.

Links and screenshots illustrating the problem

Here is an example golf course. When the rough was added to distinguish between the desert natural areas and the watered/lush areas, the greens and fairways are no longer visible:
https://www.openstreetmap.org/#map=18/36.17548/-115.28432

screenshot_2019-01-31 openstreetmap

I think they are just the same color, so changing Golf-Rough to be a darker shade of green should resolve the issue. I would make a pull request, but there's a lot of discussion about colors, especially green, so I'd appreciate guidance on how critical these "very zoomed in detail" colors are.

Possibly related to meta ticket:
#3517

@kocio-pl kocio-pl added this to the Bugs and improvements milestone Jan 19, 2019
@Adamant36
Copy link
Contributor

Adamant36 commented Jan 19, 2019

Also related to #661 and #426. Along with a few other issues. I had planned on working on #661 in the middle of 2018, but I got busy and forgot. It would be cool if you could instead. Rendering golf stuff is more complicated then other things for some reason that I was never able to understand. Luckily, both the German and French styles (I think) can be used for references on how to code things properly though.

@chadrockey
Copy link
Author

chadrockey commented Jan 19, 2019

@Adamant36 Thank you for the quick response.

After getting a night's rest, I didn't understand where to start on this because Github returned no search results for "rough". Looking at the XML and the issues you linked, this is because the coloring is done via the secondary tags, right? So for green and rough, there's a:

<tag k="landuse" v="grass"/>

And that's the actual coloring source and they are both grass, so my reported issue is clear to me now. So it would require adding specialized colorings for golf as in the issues linked. I'll look into this in the coming week. Thanks again!

@Adamant36
Copy link
Contributor

Your welcome. I haven't looked over the issues in a while, but that sounds correct.

@jeisenbe
Copy link
Collaborator

@Adamant36, I think this would be worth rendering, if you want to give it a try. Both the French style and the alternative-colors style have shown how it could be done.

@Adamant36
Copy link
Contributor

@jeisenbe, I mentioned that @chadrockey, above. I think he's going to give it a try based off of how they do it. I'm willing to assist him though if need be or do it if he decides not to.

@chadrockey
Copy link
Author

I'm going to start this in the next few days if no one else has started. Finally in my critical path.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Feb 3, 2019

@chadrockey, that would be great!

You might consider using something similar to this:

(Comparison with current rendering:
https://www.openstreetmap.org/#map=17/48.670941599608875/9.4178098440170295)

Here's a link to the code: https://github.com/imagico/osm-carto-alternative-colors/blob/master/golf.mss

The @golf course background color is changed to the same as @campsite in landcover.mss
And the landcover layer has this additional line in project.mml:

                ('golf_' || (CASE WHEN (tags->'golf') IN ('rough', 'fairway', 'driving_range', 'water_hazard', 'green', 'bunker') THEN tags->'golf' ELSE NULL END)) AS golf,

The one issue will be if the geometries of closed ways tagged only with golf=* are imported as polygons currently. @imagico, did you have to change anything in the Lua transformations file to get the golf courses to render properly? I hope these are already in the database.

@imagico
Copy link
Collaborator

imagico commented Feb 3, 2019

My database scheme is the same as here. I used the same approach as used by the French and German style - which all rely on what seems to be an undocumented feature of mapnik to process mixed geometry types as polygons.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Feb 3, 2019

Oh, I see how you did it:

https://github.com/imagico/osm-carto-alternative-colors/blob/master/project.mml

            UNION ALL -- this is only needed because closed ways with a golf tag are by default not treated as polygons
            SELECT
                way, COALESCE(name, '') AS name,
                'INT-generic'::text AS religion, '' AS crop, 0 AS way_area, layer,
                (CASE WHEN (tags->'golf') = 'rough' THEN -1 WHEN (tags->'golf') = 'fairway' THEN 10 WHEN (tags->'golf') = 'green' THEN 20 WHEN (tags->'golf') = 'bunker' THEN 30 ELSE 40 END) AS prio,
                ('golf_' || (CASE WHEN (tags->'golf') IN ('rough', 'fairway', 'driving_range', 'water_hazard', 'green', 'bunker') THEN tags->'golf' ELSE NULL END)) AS feature
              FROM planet_osm_line
              WHERE (tags->'golf') IS NOT NULL
          ) AS landcover_all
          ORDER BY COALESCE(layer,0), prio, way_area DESC

Would this also work for a tag like healthcare=* which currently doesn't render on closed ways?

@chadrockey
Copy link
Author

Looks good, thanks for the pointers to the specific code. I'll work out the correct layering and colors to look distinctive.

Two questions:

  1. Do I make the PR to @imagico or to gravitystorm? I don't see many golf features in the master fork.
  2. If I wanted to propose having a line type that's "golf cart path", how is this proposed? Most are currently done as "walking paths".

Thanks!

@imagico
Copy link
Collaborator

imagico commented Feb 4, 2019

To get a change into the main map you have to create a PR here. But note that accepting the PR will likely have to wait until the next database reload. This style sets the standards for database schema for various other projects and this creates the responsibility to actually do so and rejecting this function by using workarounds will probably not be accepted.

Also this style will not render newly invented tags but only those that are already being consistently used by mappers. If you want to work on tagging of golf courses that is a completely separate matter to making style changes here.

@Adamant36
Copy link
Contributor

Which tags in relation to this are "invented"? It was my understanding from research when I worked on it that all the tags related it are eatablished, but not used conistantly due to people wrongly tagging things for the renderer.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Feb 5, 2019 via email

@Adamant36
Copy link
Contributor

My bad. I didn't see that. I thought the comment was in reference to the issue in general. Something like that should be it's own issue anyway.

@chadrockey
Copy link
Author

@jeisenbe thanks for the information. I agree that highway=path and golf_cart=designated is a good and standard way to tag these paths.

My question specifically related to if there should be a "golf->path" Feature Picker so that the proposed coloring can account for the paths as well as ensure that these tags are faster to make and consistent for future edits.

The relevance to the wider conversation (specialized colors for specific tags), is that from an outside perspective, it would seem divergent to have the golf features bundled together, but cart paths considered to be a part of another system of road ways.

Specifically, here are screenshots from the UI that show most of the relevant features for this issue (these are the areas, but there is currently one line type: "golf->hole").

golfeditor

Once you select one of these, it auto populates the correct tags (landuse, recreation, sport, etc):

correct_tags

  1. So more specifically, is golf->path a valid (Feature?) to be included in these (templates?)? And if so, how does the Feature Template be proposed? I don't believe it's a new feature, just a shortcut for existing tagging?

  2. Is it okay to include styling for cartpaths or is it considered bad form to diverge from the global style for highway=path?

@Adamant36
Copy link
Contributor

Adamant36 commented Feb 5, 2019

iD Editor is a completely different project with different maintainers. This is a map style. Whereas its a mapping tool. There's no parity between them at all though aside from being on the same site. It uses a bunch of tags that aren't supported here either (or even on the wiki sometimes).

@jeisenbe
Copy link
Collaborator

jeisenbe commented Nov 9, 2019

This issue is a duplicate of #661 so I will close it, but I am still in favor of rendering these features after the next database reload - while they are not very important, they take up a large are of the map and can easily be shown by using some of the existing vegetation colors, and the current lack of golf= rendering in this style has encouraged less use of some other tags to get rendering.

@jeisenbe jeisenbe closed this as completed Nov 9, 2019
@jeisenbe jeisenbe removed this from the Bugs and improvements milestone Nov 9, 2019
@jeisenbe jeisenbe removed the landcover label Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants