Coalesce duplicate route concurrencies #1361
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 c8e51d3 (merged with base efa6b27 as 9a24837). See run details. PostgreSQL DB size in MB: 4863 ⇒ 4863 (0.0% change)
expand for details... |
zstadler
reviewed
Feb 20, 2022
zstadler
reviewed
Feb 20, 2022
ZeLonewolf
force-pushed
the
coalesce_same_routes
branch
from
0c75e49
to
bddfd90
Compare
|
The route relation concurrency information seems to be specific for the OSM Americana style. Wouldn't is be better to have this information as a private layer of an Americana fork of this repository? |
Route concurrencies are a global feature for any renderer that wishes to display pictoral route shields. Concurrent pictoral route symbols are shown in paper maps all over the world. The fact that OSM Americana was the first style to figure out how to do it technically doesn't take away from it being a general-purpose feature. |
TomPohys
approved these changes
Jan 26, 2023
|
Looks great! Thank you for investigating! |
This was referenced Feb 23, 2023
wcedmisten
added a commit
to wcedmisten/openmaptiles
that referenced
this pull request
Feb 26, 2023
This reverts commit bc9bbd2.
Merged
TomPohys
pushed a commit
that referenced
this pull request
Mar 21, 2023
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.
Problem description
#1128 introduced route relation concurrency information in OpenMapTiles via the
route_Xattributes. The original implementation assumed that there would be a single route relation for eachnetworkandrefpair. However, it is increasingly common practice to tag a separate route relation for each direction of a route in order to provide awareness to routers and other data consumers of the directionality of a route. This standard and growing practice is described on the OSM wiki page on route directions. Thus, the naïve implementation of #1128 caused duplicate entries to be added asroute_Xattributes in the case where separate route relations were used for directional routes.The following super-relation is an example of a route modeled with multiple route relations based on direction:
https://www.openstreetmap.org/relation/162114
Below is the resultant rendering in openstreetmap-americana, showing an incorrect shield duplication of Ohio, USA state route 104:
Solution description
This PR adds grouping when computing route concurrency information, such that duplicate entries are coalesced in a predictable way. Since this grouping is done only within a route membership join of a single member way, the computational complexity should be trivial.
Below is a re-rendering of the same region on openstreetmap-americana after implementing this change:
