Skip to content

fix(center-of-mass): calculate polygon centroids ring by ring - #3174

Open
vera-rubin wants to merge 1 commit into
Turfjs:masterfrom
vera-rubin:fix/center-of-mass-holes
Open

vera-rubin wants to merge 1 commit into
Turfjs:masterfrom
vera-rubin:fix/center-of-mass-holes

Conversation

@vera-rubin

Copy link
Copy Markdown

centerOfMass joined polygon rings, adding artificial edges and returning the wrong centroid for polygons with holes.
We sum each ring separately and subtract hole areas and moments regardless of winding.
We tested the reported example, both polygon input forms and ring windings, and ran the package and repository checks.

Fixes #3173

  • Meaningful title including the package name.
  • Summary of the changes above.
  • No breaking API changes.
  • Resolves the linked issue.
  • Contributor details added to package.json.
  • Read the pull request preparation guide.

— ChipLab

@hafizahmedzia

Copy link
Copy Markdown

Additional validation of vera-rubin's change on base 0c20329d and head e51817d0: two polygons with two disjoint interior holes were checked against centroids calculated by subtracting rectangle/triangle areas and moments.

An outer [0,0]–[12,10] rectangle with holes [1,1]–[3,4] and [8,6]–[11,9] has area 105 and centroid [83/14, 69/14]. Base returned [5.925143453312468, 4.923317683881065]; changing only the second hole's starting vertex changed it to [5.996781416176882, 4.937027707808564]. Head matches the expected center.

Across both fixtures, every cyclic ring start, three translations, and Feature/Geometry forms, head matched the independent oracle for all 672 diagnostic inputs within 1e-10. The 24 exterior-only controls passed on both revisions. Translation invariance also passed on base. Focused package checks passed on head (19 assertions plus type checking); the full repository suite was not run.

AI-assisted validation and draft.

This branch has not been deployed

No deployments
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.

centerOfMass joins polygon rings and returns the wrong centroid

2 participants