Selective rendering of tracks and paths at z12-13 #1190
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Results evaluating commit 4b5a3ad (merged with base 1cea73c as 23c5ff8). See run details. PostgreSQL DB size in MB: 2791 ⇒ 2799 (0.3% change)
expand for details... |
|
Results evaluating commit dad76ad (merged with base 7f531c1 as 30dcc05). See run details. PostgreSQL DB size in MB: 2843 ⇒ 2849 (0.2% change)
expand for details... |
ZeLonewolf
changed the title
|
I’m happy with adding and moving roads to upper zooms. Also, the proposed zoom filter based on sac_scale, color, etc. is a really good idea. But please don’t add more attributes related to hiking - sac_scale and values network-* to this schema. The design of this schema is to be general-topographic. It would be better to add hiking stuff as a different layer with the behavior of #1003 turned off by default. We did it as a separate layer on global field https://www.maptiler.com/maps/outdoor/ look at schema here https://docs.maptiler.com/schema/outdoor/ |
|
Hi @ZeLonewolf, thanks for the PR. OpenMapTiles should provide the schema for the general-purpose map, or base map. Unfortunately, adding another hierarchy layer into the Unfortunately, the Could you please keep the selecting logic for I am preferring your approach than just move all the paths to zoom 12 (#1186). Thank you |
|
Thanks for the detailed feedback @TomPohys. I will work on making changes consistent with your description. Before I do that, I am planning to complete #1207 to refactor the zoom 12-14 logic which I find confusing to read. Separating this refactor will help ensure that this PR does not introduce unexpected bugs due to converting the nested |
|
Great, looking forward to it! |
@nnhubbard it seems reasonable to me, however it's certainly outside the scope of this issue, and there is resistance to adding hiking-related data into the main style. |
|
I have updated this PR consistent with the comments from @daliborjanak and @TomPohys. Accordingly, there are no new hiking network values and the symbol/colour tagging is not exposed to the tiles. The tile size increase is very small while still adding "significant" paths and track roads to the map at lower zooms. |
TomPohys
reviewed
Nov 16, 2021
TomPohys
reviewed
Nov 16, 2021
Co-authored-by: Tomas Pohanka <TomPohys@gmail.com>
|
This PR is updated to move the |
|
Hi, thank you very much! It looks awesome. Just two questions
I would like to merge this PR before #1295. It seems like that will be less effort to resolve conflicts. |
|
Thanks @TomPohys for your comprehensive review as always! I've posted a new updated to fix the issues noted. |
|
Thank you very much! It looks awesome! Great work! |
TomPohys
approved these changes
Nov 18, 2021
TomPohys
pushed a commit
that referenced
this pull request
Nov 25, 2021
I discovered this bug while investigating issues with the updates process related to #1190 #1292, and #814. The `transportation_name` layer produces slightly different `tags` hstore values in the `osm_transportation_name_linestring` table during the initial import versus when running an update. As currently written, the import code produces null-value keys in the `tags` column, while the update code suppresses them. This PR removes that difference and makes the import code use same method that is currently used in the update code. With a test case I've written, the import code produces a tags hstore that looks like this: `"name"=>"OpenMapTiles Secondary 2", "name:de"=>NULL, "name:en"=>NULL, "name_int"=>"OpenMapTiles Secondary 2", "name:latin"=>"OpenMapTiles Secondary 2"` ...while the update code produces a tags hstore that looks like this: `"name"=>"OpenMapTiles Secondary 2", "name_int"=>"OpenMapTiles Secondary 2", "name:latin"=>"OpenMapTiles Secondary 2"` Note the missing NULL values. This bug causes a small amount of space wastage after an update is run, because the update matching code detects the `tags` value as different, resulting in a duplicate copy of the tags value if that row is updated. This causes duplicate objects and breaks GROUP BY clauses that expect to group same-tagged features together. I've tested this by inspection of a generated mbtiles, database spot checks, and the unit test code included in this PR.
TomPohys
pushed a commit
that referenced
this pull request
Jan 5, 2022
This PR fixes a bug that causes that `track` lines disappear at z13. This bug was introduced in #1190, which adds rendering of paths and tracks at z12 and z13. Before this PR: z12: lines with `route_rank = 1` are added (no matter what `highway`). z13: lines with `route_rank BETWEEN 1 AND 2` and `highway = 'path'` are added. -> tracks with `route_rank=1` are added at z12 but not at z13. After this PR z12: lines with `route_rank = 1` and `highway IN ('path', 'track')` are added. z13: lines with `route_rank BETWEEN 1 AND 2` and `highway IN ('path', 'track')` are added . -> only tracks and paths are added at z12 and z13 (which was IMHO the goal of #1190) * Add only the most important paths and tracks (route_rank=1) to z12 and more important ones (route_rank between 1-2 or sac_scale or has name) to z13.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #271
This PR adds track and path rendering at lower zooms than currently provided, and also achieves near-parity with openstreetmap-carto on track and path rendering. A previously-abandoned attempt, with significant discussion, was #1169.
Discussion
The following table shows the zoom levels that paths and tracks currently appear in both openstreetmap-carto and OpenMapTiles:
highway=trackhighway=pathhighway=trackhighway=pathIn order to achieve parity between openstreetmap-carto and OpenMapTiles, it would be necessary to render paths in z13 and tracks in z12-13. The naive approach approach of simply adding these features to these zooms was attempted in #1186 which proposed to render all tracks and paths up to z12, which would match openstreetmap-carto at z13 and exceed it at z12 (because openstreetmap-carto renders tracks but not paths at this zoom). This resulted in a 20% increase in the tile size at z13 in the test area, which was an unacceptable increase in tile size. The tile size increase was even larger at z12, however, this is partly because paths are also extended to z12, which exceeds that of openstreetmap-carto.
While the large increase in tile size is unworkable, it is also not desirable that OpenMapTiles has a feature gap with openstreetmap-carto for tracks and paths. Therefore, this PR adds selective, progressive rendering of important paths at z12-13 and universal rendering of tracks at z12. This brings OpenMapTiles to parity with openstreetmap-carto for tracks while bringing a similar, but more nuanced rendering of paths at z12-14.
Selective Path Rendering
With selective path rendering, we select "more important" paths to render at zoom 13, and only the "most important" paths for zoom 12. In order to identify whether a path is more/most important, we can look at the following tagging:
name(unnamed paths are less important)sac_scalenetworkosmc:symbolcolourOf note in the discussion in #1186 was that there are likely important hiking trails that are tagged as unnamed
highway=pathin the map without any of the tagging described above. Since there is no way to distinguish unnamed/untagged high-importance hiking trails from unnamed/untagged low-importance hiking trails, all we can do is establish minimal rules for importance based on tagging. This provides mapper feedback to add this tagging, where applicable, to important hiking trails, so that they can appear in general-purpose styles at lower zooms.In particular, the selective rendering of trails of national importance at lower zooms is of interest to the openstreetmap-americana project, and American mapping in general. Long-distance trails such as the Appalachian Trail, Pacific Crest Trail, or Continental Divide Trail have significant cultural importance and run through rural areas with low map feature density. Therefore, it is cartographically appropriate to render these features at lower than usual zoom levels for walking paths.
Below is an example from Pennsylvania, USA, showing high-importance hiking trails being rendered at city-level zooms on a general-purpose (non-hiking-specific) map:
Proposed Zoom Filter
This PR implements the following rules:
Zoom 12
highway=tracknetwork=iwn/network=nwn)Zoom 13
highway=tracksac_scale,osmc:symbol, orcolourZoom 14
highway=trackhighway=pathRendering Samples
Example location: https://www.openstreetmap.org/#map=12/41.5867/-71.7063
The following samples were generated from zoom 11-14 tiles from the us-northeast extract.