-
Notifications
You must be signed in to change notification settings - Fork 661
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
map matching uturn trimming fix #2558
Conversation
…occur at the end of the edge
} // No trimming needed | ||
else if (!edge_begin_info.trim) { | ||
edge_begin_info.distance_along = 0; | ||
edge_begin_info.vertex = edge_shape.front(); |
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.
when we are not trimming either the front or the end we need to make sure we still set these things because they are used regardless below in the shape trimming function
// Add edge shape to the trip and skip the first point when its redundant with the previous edge | ||
// TODO: uncommment correct removal of redundant shape after odin can handle uturns | ||
// trip_shape.insert(trip_shape.end(), edge_shape.begin() + !is_first_edge, edge_shape.end()); | ||
trip_shape.insert(trip_shape.end(), edge_shape.begin() + !edge_begin_info.trim, |
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.
the commented out line above is the correct code and causes no duplicates to be had in the shape. however at uturns, it seems that odin does not detect the uturn has occured unless the shape point is doubled where the uturn occurs. cc @dgearhart
EXPECT_EQ(shape.size(), expected_shape.size()); | ||
for (int i = 0; i < shape.size(); ++i) { | ||
EXPECT_TRUE(shape[i].ApproximatelyEqual(expected_shape[i])); | ||
} |
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 test case showed the above behavior so we simply expanded it to check the actual shape was what we thought it should be (this is how we found out that the odin bug with uturn maneuvers exists). we decided not to fix the odin uturn bug at this time and instead opted to leave the extra shape point in (at the uturn point) so that we get the correct maneuver
@@ -75,7 +75,7 @@ void via_discontinuity( | |||
// Insert a second discontinuity so the next (opposing) edge is trimmed at the end from | |||
// 1-dist along to 1 | |||
vias.insert({path_index + (flip_index ? 0 : 1), | |||
{{false, snap_ll, 1.0f - dist_along}, {true, PointLL(), 1.0f}}}); | |||
{{true, snap_ll, 1.0f - dist_along}, {false, PointLL(), 1.0f}}}); |
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 was a long time copy paste fail..
when a uturn is detected in a map match we need to trim the edges shape where the uturn occurs. we recently fixed code that always set trimming information even when it wasnt needed. that fix assumed that the downstream code actually cared to whether or not that boolean telling it to trim was set. this was not the case..
this may fix #2552