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

Remove deduplication of unpacked_path_segments in MM collapsing #4911

Merged
merged 3 commits into from
Feb 24, 2018

Conversation

karenzshea
Copy link
Contributor

@karenzshea karenzshea commented Feb 23, 2018

Issue

Fixes the issue in indicated by #4909
image

Bug

Leg collapsing for some traces with high sample rates, resulted in dropped geometries like shown by the red line in the image above, particularly around corners.

Root cause

The bug was caused by a mishandling of unpacked_path_segments collapsing. The scenario arose when two points in a trace were located on the same edge, resulting in a 0 length segment, which, because the leg collapsing was incorrectly removing the last unpacked_path_segment value from the previously last collapsed segment, would sometimes result in missing coordinates in the end result.

Tasklist

  • CHANGELOG.md entry
  • add tests (see testing documentation
  • review
  • adjust for comments
  • cherry pick to release branch

Requirements / Relations

5.16 cc @chaupow

@karenzshea karenzshea changed the title emove deduplication of unpacked_path_segments Remove deduplication of unpacked_path_segments in MM collapsing Feb 23, 2018
@karenzshea karenzshea requested a review from oxidase February 23, 2018 12:56
@karenzshea
Copy link
Contributor Author

Also, thanks to @danpat for teasing out a good test case expressing the bug

@danpat danpat self-requested a review February 24, 2018 00:47
Copy link
Member

@danpat danpat left a comment

Choose a reason for hiding this comment

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

This fixes the problems I was having, LGTM 👍

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

Successfully merging this pull request may close these issues.

2 participants