-
Notifications
You must be signed in to change notification settings - Fork 1k
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
vector tile: add no-thru traffic restrictions layer #6146
base: dev-2.x
Are you sure you want to change the base?
Conversation
24c6206
to
06f97dc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6146 +/- ##
=============================================
+ Coverage 69.92% 69.94% +0.01%
- Complexity 17722 17738 +16
=============================================
Files 1996 1996
Lines 75394 75467 +73
Branches 7713 7721 +8
=============================================
+ Hits 52720 52783 +63
- Misses 19997 20008 +11
+ Partials 2677 2676 -1 ☔ View full report in Codecov by Sentry. |
1bb4fc1
to
b10b81d
Compare
+ group no-thru traffic and permission layers by mode
b10b81d
to
336fb13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice to get rid of the checkboxes for all permission combinations.
I haven't looked at the vectortiles before but I think this looks good in general. I tested it out and added my thoughts.
@@ -181,9 +197,8 @@ private static List<StyleBuilder> edges(VectorSourceLayer edges) { | |||
.group(EDGES_GROUP) | |||
.typeLine() | |||
.vectorSourceLayer(edges) | |||
.lineColor(MAGENTA) | |||
.edgeFilter(EDGES_TO_DISPLAY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why we remove this filter?
.toList(); | ||
} | ||
|
||
private static String permissionColors(StreetTraversalPermission p) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static String permissionColors(StreetTraversalPermission p) { | |
private static String permissionColor(StreetTraversalPermission p) { |
.lineColor(MAGENTA) | ||
.edgeFilter(EDGES_TO_DISPLAY) | ||
.lineWidth(LINE_WIDTH) | ||
.lineColor(GRAY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is hard to see.
.lineColor(GRAY) | ||
.lineOpacity(0.2f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very hard to see
public static String streetPermissionAsString(StreetTraversalPermission permission) { | ||
return ( | ||
permission == StreetTraversalPermission.ALL | ||
? "PEDESTRIAN_AND_BICYCLE_AND_CAR" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does add a bit of noise to the map since the labels that used to say ALL
now says PEDESTRIAN BICYCLE CAR
. Maybe that's ok since it's more explicit?
Summary
The new vector tile layers did not include no-thru traffic details. This adds layers reusing the existing colors from the permission layer.
The edge layers are grouped by street mode. This allows coloring all edges usable by the selected modes.
Issue
Fixes #6145