Change in behavior of LineString::simplify from 0.23.1 to 0.24.0

[rjmac]

The results of calling simplify on a linestring has changed from version 0.23.1 to 0.24.0 - at least sometimes, it appears to be doing nothing at all.

I have a reproduction here: https://github.com/rjmac/simplify - it just does

let unsimplified = line_string![
    (x: 0.0, y: 0.0),
    (x: 5.0, y: 4.0),
    (x: 11.0, y: 5.5),
    (x: 17.3, y: 3.2),
    (x: 27.8, y: 0.1)
];
let simplified = unsimplified.simplify(&30.0);
println!("Before: {unsimplified:?}");
println!("After: {simplified:?}");

With geo-23 it prints

Before: LineString([Coord { x: 0.0, y: 0.0 }, Coord { x: 5.0, y: 4.0 }, Coord { x: 11.0, y: 5.5 }, Coord { x: 17.3, y: 3.2 }, Coord { x: 27.8, y: 0.1 }])
After: LineString([Coord { x: 0.0, y: 0.0 }, Coord { x: 27.8, y: 0.1 }])

and with 24,

Before: LineString([Coord { x: 0.0, y: 0.0 }, Coord { x: 5.0, y: 4.0 }, Coord { x: 11.0, y: 5.5 }, Coord { x: 17.3, y: 3.2 }, Coord { x: 27.8, y: 0.1 }])
After: LineString([Coord { x: 0.0, y: 0.0 }, Coord { x: 5.0, y: 4.0 }, Coord { x: 11.0, y: 5.5 }, Coord { x: 17.3, y: 3.2 }, Coord { x: 27.8, y: 0.1 }])

This is probably related to #943 - I haven't verified it yet, but that's the only thing between the two versions that changed simplify code that I've noticed.

[michaelkirk]

This is probably related to https://github.com/georust/geo/pull/943

That seems likely. I think we were only considering polygons when we did #943, which seems like a bug to me.

[michaelkirk]

I think we were only considering polygons when we did https://github.com/georust/geo/pull/943

Sorry, that's not true. I should have reviewed #943 better. It does account for LineString vs. Polygon - but that doesn't preclude an error in the implementation.

https://github.com/georust/geo/pull/943/files#diff-3fcb9f88c9a1d5c03f34a7b2f0b6a174722a15c47ff93ec5f0d4752618b119d5R4

[rjmac]

Ah-ha, so this conditional block is where the behavior changed - here, in the example above, *simplfied_len and INITIAL_MIN are both 2, so it's bailing and returning its input; this condition was not here in 0.23 and it fell through to the next line.

From my understanding of what #943 was doing, this condition should probably be <, not <=? Though changing that makes recursion_test fail. That test is asserting that a simplifcation pass doesn't do anything to a list of coords, but I'm not sure what that test is testing, exactly.