gravitystorm / openstreetmap-carto

A general-purpose OpenStreetMap mapnik style, in CartoCSS
Other
1.55k stars 822 forks source link

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

Closed chadrockey closed 5 years ago

chadrockey commented 5 years ago

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: https://github.com/gravitystorm/openstreetmap-carto/issues/3517

Adamant36 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

jeisenbe commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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

jeisenbe commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

The original poster just asked:

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".

This should be discussed on the Tagging mailing list or the forums.

(But I’d suggest that highway=path with golf_cart=designated is an appropriate way to tag paths that are designed for use by golf carts. You can also add bicycle=yes/no etc as appropriate. If motorcars are allowed, then highway=service is more appropriate. See https://wiki.openstreetmap.org/wiki/Key:golf_cart ) On Tue, Feb 5, 2019 at 9:01 AM Adamant36 notifications@github.com wrote:

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.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/gravitystorm/openstreetmap-carto/issues/3650#issuecomment-460462256, or mute the thread https://github.com/notifications/unsubscribe-auth/AoxshFWo-OPE55ZhuWiTZnIv6CRw3ml6ks5vKMnmgaJpZM4aJEYL .

Adamant36 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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.