fix shape trimming to avoid single point edges #1876
Conversation
| @@ -30,45 +30,6 @@ using namespace valhalla::thor; | |||
|
|
|||
| namespace { | |||
|
|
|||
| template <class iter> | |||
There was a problem hiding this comment.
this shape trimming code has the possibility for it to only add 1 shape point for a given edge which has downstream negative consequences. there is another shape trimming method below which doesnt not suffer this problem, so i've switched to using that
src/thor/triplegbuilder.cc
Outdated
| // We need to clip the shape if its at the beginning or end and is not full length | ||
| float total = static_cast<float>(directededge->length()); | ||
| // Trim the shape | ||
| TrimShape(edge_shape, is_first_edge ? start_pct * total : 0, |
There was a problem hiding this comment.
here we use the other shape trimming method. if its the first edge of the route we want to trim the front of the shape starting at the start vertex otherwise we want the whole front. if its the last edge we want to trim the back of the shape up to the end vertex otherwise we want the whole back. i can split this into two ifs for clarity
| auto walked_encoded_shape = walked.get<std::string>("shape"); | ||
| auto walked_shape = midgard::decode<std::vector<midgard::PointLL>>(walked_encoded_shape); | ||
| /*if (walked_shape.size() != shape.size()) { | ||
| throw std::logic_error("Differing shape lengths " + std::to_string(shape.size()) + |
There was a problem hiding this comment.
once i had made the change to do the trimming properly, this test started failing with some wild numbers in terms of distance between points. so i eventually found out that when you take a route and you then edge_walk map match its shape, the shape that you get back has 1 more point in it. i added this code to determine that. even after reverting all my o ther changes but leaving this check, the code still breaks. so this commented code is another bug to fix unrelated to this pr about why the shape walk of a regular route has one more shape point.
| else { | ||
| bx = bx * scale + u.first; | ||
| by = by * scale + u.second; | ||
| } |
There was a problem hiding this comment.
while i was goose-chasing the oddity with map matching test failure i thought that maybe it had something to do with the difference in precision between lokis projection and meilis. so to remove that concern i refactored them to share the same code.
| * where the point is long lived and we survey many many shape segments such as is done in | ||
| * both loki and in meili | ||
| * */ | ||
| struct projector_t { |
There was a problem hiding this comment.
this was lifted directly out of loki and is now called from both meili and loki
in some scenarios the partial shape trimming for the beginning and end of the route can lead to an edge with a single point, this causes many problems downstream for guidance and eventually on the client side as well. i'll annotate the PR with the series of things going on to address that in this PR.