mthh / contour-rs

Contour polygon creation in Rust (using marching squares algorithm)
https://crates.io/crates/contour
Apache License 2.0
52 stars 10 forks source link

Area calculation via shoelace method ignores final vertex (which probably gives wrong result for small polygons) #11

Closed caspark closed 6 months ago

caspark commented 6 months ago

I think the area function in src/area.rs is not handling the wraparound case correctly:

pub fn area(ring: &[Pt]) -> Float {
    let mut i = 0;
    let n = ring.len() - 1;
    let mut area = ring[n - 1].y * ring[0].x - ring[n - 1].x * ring[0].y;
    while i < n {
        i += 1;
        area += ring[i - 1].y * ring[i].x - ring[i - 1].x * ring[i].y;
    }
    area
}

Say ring has 10 elements; then n == 9 and so the initialization of area is pairing up ring[0] and ring[8] (rather than ring[9]).

A correct implementation could be something like:

pub fn area(poly: &[Pt]) -> Float {
    let n = poly.len();
    let mut area = poly[n - 1].y * poly[0].x - poly[n - 1].x * poly[0].y;
    for i in 1..n {
        area += poly[i - 1].y * poly[i].x - poly[i - 1].x * poly[i].y;
    }
    area
}

(For anyone else who stumbles on this issue: I think normally you'd also want to divide the result by two, but this library is only using area to detect polygon winding and relative size of areas, so it shouldn't be necessary here.)

mthh commented 6 months ago

Thanks for the report.

You're right, I will submit a fix soon.

(For anyone else who stumbles on this issue: I think normally you'd also want to divide the result by two, but this library is only using area to detect polygon winding and relative size of areas, so it shouldn't be necessary here.)

Indeed, that's why the division by 2 is skipped (also, it was also skipped in the original implementation) but maybe a comment about that could help readers of the code.

mthh commented 6 months ago

Published as 0.12.1. Thanks again!

caspark commented 6 months ago

Thanks!