isi-vista / adam

Abduction to Demonstrate an Articulate Machine
MIT License
11 stars 3 forks source link

Strokes are merged in incorrect order in some cases #1217

Closed spigo900 closed 1 year ago

spigo900 commented 1 year ago

While creating visualizations for the Milestone 6/final implementation report, I discovered a few subtle bugs in the stroke merging logic:

  1. Identical consecutive control points are included in the final merged strokes. I suspect they might screw up or bias the B-spline simplification logic, so because of that should be left out.
  2. The orientation preservation logic is slightly wrong. It breaks in cases like: --> <-- <-- --> (processing from left to right). In this case we should reverse the middle two strokes (or the outer two strokes) but what actually happens at the moment is that we flip the second stroke and the final stroke, getting --> --> <-- <--. The fix is easy to code, though slightly hard to follow: We should reverse the next stroke if and only if the next stroke is oriented consistent with the current stroke and the current stroke is going to be reversed.
  3. Sometimes the clustered strokes are concatenated in the reverse of the intended control point order. This results in some messed-up looking strokes. We have enough of those already without merging, so I want to avoid that. Fortunately, the fix is straightforward: Just reverse the list of strokes if, after reorienting strokes, the second stroke doesn't begin where the first one ends.

I've implemented these fixes and will get a PR up shortly.