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

Roundabout maneuver icon depicts counterclockwise movement when driving on the left #2191

Closed
1ec5 opened this issue Oct 11, 2019 · 2 comments · Fixed by #2228
Closed

Roundabout maneuver icon depicts counterclockwise movement when driving on the left #2191

1ec5 opened this issue Oct 11, 2019 · 2 comments · Fixed by #2228
Labels
bug Defect to be fixed.

Comments

@1ec5
Copy link
Contributor

1ec5 commented Oct 11, 2019

A roundabout icon in a region that drives on the left should depict a clockwise movement, but instead it always depicts a counterclockwise movement; the icon is horizontally flipped.

#2044 attempted to fix this and some related issues by flipping the icon when the maneuver modifier is slight left, left, or sharp left:

boolean flip = SHOULD_FLIP_MODIFIERS.contains(maneuverModifier);
if (STEP_MANEUVER_MODIFIER_LEFT.equals(drivingSide) && STEP_MANEUVER_MODIFIER_UTURN.contains(maneuverModifier)) {
setScaleX(flip ? 1 : -1);
} else {
setScaleX(flip ? -1 : 1);
}
}

While this heuristic is correct for most maneuver types, it’s incorrect for roundabout maneuvers. By default, the navigation SDK sets roundabout_exits=true in the Directions API request, producing two maneuvers, one for entering the roundabout and the other for exiting it. In a region that drives on the left, you make a slight left turn to enter the roundabout and another slight left turn to exit it. These slight left turns are reflected in the two maneuvers’ modifiers.

On iOS, we use this heuristic that works well: for a roundabout, rotary, or roundabout turn maneuver, flip the image horizontally if and only if the driving side is left. The driving side is the only determining factor for circular junctions; the modifier is irrelevant.

/ref #396
/cc @mapbox/navigation-android

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 16, 2019

For the purposes of adapting the MapboxDirections.swift logic in this library:

The types and modifiers are described in comments above that chunk of code, or you can consult the Directions API documentation.

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 17, 2019

A fix could look something like this, though there may be an opportunity to reorganize the logic:

diff --git a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/instruction/maneuver/ManeuverView.java b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/instruction/maneuver/ManeuverView.java
index 218fd058..d3f57369 100644
--- a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/instruction/maneuver/ManeuverView.java
+++ b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/instruction/maneuver/ManeuverView.java
@@ -236,6 +236,9 @@ public class ManeuverView extends View {
       maneuverViewUpdate.updateManeuverView(canvas, primaryColor, secondaryColor, size, roundaboutAngle);
     }
     boolean flip = SHOULD_FLIP_MODIFIERS.contains(maneuverModifier);
+    if (ROUNDABOUT_MANEUVER_TYPES.contains(maneuverType)) {
+      flip = STEP_MANEUVER_MODIFIER_LEFT.equals(drivingSide);
+    }
     if (STEP_MANEUVER_MODIFIER_LEFT.equals(drivingSide) && STEP_MANEUVER_MODIFIER_UTURN.contains(maneuverModifier)) {
       setScaleX(flip ? 1 : -1);
     } else {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defect to be fixed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants