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

Route Access Restrictions visualisation #3603

Merged
merged 9 commits into from Dec 7, 2021
Merged

Conversation

Udumft
Copy link
Contributor

@Udumft Udumft commented Nov 17, 2021

This PR adds possibility to draw portions of a route which pass restricted roads.
The overlay is implemented as a separate customizable route line layer

Simulator Screen Shot - iPhone 12 Pro Max - 2021-11-17 at 17 30 44
Simulator Screen Shot - iPhone 12 Pro Max - 2021-11-17 at 17 31 02
.

@Udumft Udumft added platform parity Required to keep on par with Android. UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. Apps Work related to application behaviour. labels Nov 17, 2021
@Udumft Udumft added this to the v2.1-rc milestone Nov 17, 2021
@Udumft Udumft self-assigned this Nov 17, 2021
@@ -47,7 +47,7 @@ class NavigationMapViewTests: TestCase {
])

XCTAssertEqual(congestionSegments.count, 1)
XCTAssertEqual(congestionSegments[0].0.count, 10)
XCTAssertEqual(congestionSegments[0].0.count, 6)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is caused by the change in merging method. Previously it would duplicate bordering points. For example when merging segments with coords 1 -> 2 -> 3 it would result in [1, 2, 2, 3]. With change above it would result in [1,2,3].
I didn't notice changes in resulting route lines drawing, but TBH I didn't test it too much. Does anybody knows if such merging was intended.

/**
Attribute name for the route line that is used for identifying restricted areas along the route.
*/
public let RestrictedRoadClassAttribute = "isRestrictedRoad"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also few convenience constants exposed to user, like table of route line widths depending on zoom level (RouteLineWidthByZoomLevel). The only value for restricted areas display I could think of is dash array attributes which may be potentially useful, but I doubt it since if user is implementing his own layers he won't need to align dashing parameters to anything mapbox-provided.

@mapbox-github-ci-issues-3
Copy link

No breaking changes detected in MapboxCoreNavigation

@mapbox-github-ci-issues-3
Copy link

No breaking changes detected in MapboxNavigation

@Udumft Udumft marked this pull request as ready for review November 18, 2021 08:35
@Udumft Udumft requested a review from a team November 18, 2021 08:36
@mapbox-github-ci-issues-5
Copy link

No breaking changes detected in MapboxCoreNavigation

@mapbox-github-ci-issues-5
Copy link

No breaking changes detected in MapboxNavigation

var fractionTraveled = 0.0 // not traversed at all
var routeLineGradient = navigationMapView.routeLineRestrictionStops(features, fractionTraveled: fractionTraveled)
XCTAssertEqual(routeLineGradient[0.0], navigationMapView.routeRestrictedAreaColor)
XCTAssertEqual(routeLineGradient[1.0], navigationMapView.traversedRouteColor)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After modifications of route line gradients calculation in this commit this test starts failing. I see that it has changed how ending segment is handled, but it still looks odd to me: In this particular case routeLineGradient[0.75] == routeRestrictedAreaColor (0.75 is the border between 2nd restricted segment and last non-restricted segment). Since it is "not soft" gradient, I would expect that routeLineGradient[0.75.nextUp] == traversedColor, but that is not the case and in fact it's routeLineGradient[0.75.nextUp.nextUp] == traversedColor. I am not sure why there is this little gap and how it appears.
@ShanMa1991 maybe you could help me with it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, yes, this gap exits. Because right now when we choose soft, we expect that the gradient stop of one segment is the [segmentStartPercentTraveled + stopGap, segmentEndPercentTraveled - stopGap]. Because the stopGap is based on the length of current congestion segment length, which would avoid the out of bound issue between a very long segment and a very short segment.

And when we choose sharp gradient, we actually still use the constraints of the current segment as [segmentStartPercentTraveled.nextUp, segmentEndPercentTraveled.nextDown]. So for example, for the 3rd segment in this test, the actual end percentage is 0.7500000000000001, but we record the 0.75 for its end. And for the 4th segment, the actual start percentage is 0.7500000000000001, because it's different than the 3rd one, we record it with 0.7500000000000002 as the start. Thus there's a 0.0000000000000002 gap between the 3rd segment and the 4th segment in the line gradient stops. It's larger than the previous 0.0000000000000001 but could avoid the check of next segment and the segment length issue.

And for the 0.0000000000000002 gap, because right now when we choose sharp gradient, we're using the following Expression:

        lineLayer.lineGradient = .expression(
            Exp(.step) {
                Exp(.lineProgress)
                UIColor.blue
                0.75
                UIColor.blue
                0.7500000000000002
                UIColor.red
            }
        )

And we have a following result, which still shows a sharp transition in the lowest zoom level.
double

And the edge case is that what if the fractionTraveled is in this gap, for example, 0.7500000000000001. In this case, the minimumSegment would record (0.75, associatedFeatureColor), and then apply to the gradientStops. There's still a sharp change between the two segments.

@mapbox-github-ci-issues-5
Copy link

No breaking changes detected in MapboxCoreNavigation

@mapbox-github-ci-issues-5
Copy link

No breaking changes detected in MapboxNavigation

@ShanMa1991 ShanMa1991 requested a review from a team November 30, 2021 00:51
@mapbox-github-ci-issues-3
Copy link

No breaking changes detected in MapboxCoreNavigation

@mapbox-github-ci-issues-3
Copy link

No breaking changes detected in MapboxNavigation

…ine; added constants for colors; added delegate methods for layer and shape configuration; included restrictions layer to layers handling pipeline; update calculating features, stops, road classes; added new route line types for layer identifier
…adsFeatures calculations; refactored route line gradients stops calculation for congestion and restricted areas;
…restrictions class to route fixture; removed obsolete argument for restrictedRoadsFeatures calculation.
…abling restricted areas gradient drawing depending on corresponding property and routeLineTracksTraversal.
@mapbox-github-ci-issues-5
Copy link

No breaking changes detected in MapboxCoreNavigation

@mapbox-github-ci-issues-5
Copy link

No breaking changes detected in MapboxNavigation

@mapbox-github-ci-issues-4
Copy link

No breaking changes detected in MapboxCoreNavigation

@mapbox-github-ci-issues-4
Copy link

No breaking changes detected in MapboxNavigation

public var showsRestrictedAreasOnRoute: Bool = false {
didSet {
if let routes = self.routes {
show(routes, legIndex: self.currentLegIndex)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if we make resume button to switch on/off of navigationMapView.showsRestrictedAreasOnRoute during active navigation, we have the following result, because if we call the navigationMapView.show(_:legIndex) directly, the previous stored route points and upcoming route point index are cleared. There'll be a 1 second gap of showing the whole route line when routeLineTracksTraversal enabled. Until the next routeProgress comes in and re-store the upcoming route point index and fractionTraveled, could we get the right route line display.
switch

But we could make this as a tail work because right now the CarPlay start with the showsRestrictedAreasOnRoute enabled or disabled, the gap is very small and almost non. There's not much case that it would be turned on/off during active navigation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a ticket for tracking this.

@MaximAlien MaximAlien modified the milestones: v2.1-rc, v2.2 Dec 3, 2021
@Udumft Udumft merged commit dd8ec59 into main Dec 7, 2021
@Udumft Udumft deleted the vk/844-access-restrictions branch December 7, 2021 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Apps Work related to application behaviour. platform parity Required to keep on par with Android. UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants