fix deviation false positives with degenerate zero-area polygons - #209
fix deviation false positives with degenerate zero-area polygons#209mrshannon wants to merge 1 commit into
Conversation
|
Thanks for the contribution! Intuitively, this seems like an overkill for such a minor problem we're never hitting in practice — adding quite a bit of code for minuscule gain. Are there any alternatives that make |
Not without intruding a false negative: This solves the bug, but introduces a new one where an invalid zero-area triangulation would return 0 deviation. It is possible to run the error calculation only if The only other way I know to solve the issue is to put the decision on the caller by changing the public API shape by allowing the user to get both the triangle and polygon areas so they can use an appropriate error bounds themselves. The MR is the only way I know to determine this error bounds without knowledge of the domain.
I don't know about in mapbox but I am hitting this case in practice when using earcut as a library in another project. |
|
Found an easier solution for this in 0302a75 — should be good enough for most cases, right? |
2 participants
Problem
deviation()returns a spurious1for a correctly-triangulated zero-area (collinear) polygon. It guards the0/0case with an exactpolygonArea === 0, but the non-robust shoelace leaves a tiny nonzero residual for a non-axis-aligned collinear ring, so the guard is skipped and it computes|0 − ε| / ε = 1.Fix
Compute a per-ring floating-point error bound alongside the signed area, and treat
|polygonArea|within that bound as zero:signedAreaWithError()mirrorssignedArea()but also accumulatesΣ|terms|and returns{sum, error}, witherror = Σ|terms| · (n + 2) · u(u = EPSILON / 2,n= ring vertex count) - the worst-case roundoff bound of the shoelace sum.deviation()accumulatespolygonAreaandpolygonAreaErrorper ring, then returns0whenMath.abs(polygonArea) <= polygonAreaError && trianglesArea === 0, otherwise the original relative formula is unchanged.The test is scale-relative (correct at any coordinate magnitude), unlike a fixed epsilon.
signedArea()(on the hot path) is left untouched.Why it's safe
The new branch only fires when
trianglesArea === 0(earcut produced no triangles). Every polygon that yields triangles takes the identical relative branch as before, so no existingdeviationvalue changes. Verified over the whole fixture suite by computingdeviationwith and without the guard — they are byte-identical except the five zero-triangle rows:NOTE: deviation (raw) is with the existing exact 0.0 guard removed as well
Rows where the guard changes the result (all zero-triangle):
Takeaways:
error / areanever exceeds ~1.6e-11across the 54 triangulated fixtures — the bound is ~10+ orders of magnitude from ever affecting a real polygon.issue17: error2.13, area3.2e15, deviation1.6e-16) — the error scales with the area, and the row triangulates so the bound is never consulted.issue107(0 triangles, real area-1.7e-3) is not masked:|area| ≫ error, so it still reports1. The bound distinguishes "truly zero" from "small but real."Full table — every fixture + the inline regression case
Testing
deviation is zero for a degenerate collinear polygon.Notes
(n + 2)·urather than a looser2n·u. For a correctness check the fail-safe direction is toward flagging, not masking, so the smaller-error bound is preferable — and(n + 2)·uis the minimum that still guarantees catching every genuinely zero-area ring. Both bounds produce identicaldeviationresults across the entire suite; only the internalpolygonAreaErrordiffers.