Replace normal_triangle with normal_polygon for robust concave polygon handling
Polygon.normal was calculating normals using normal_triangle with the centroid and first two points, which fails for concave polygons when those points lie on concave ears.
Changes
src/compas/geometry/polygon.py
- Replace
normal_triangle([self.centroid] + self.points[:2])withnormal_polygon(self.points)in theplaneproperty - O(n) algorithm considers all vertices instead of O(1) triangle-based approach
tests/compas/geometry/test_polygon.py
- Add
test_polygon_normal_concavevalidating L-shape and arrow-shape concave polygons with both winding orders
tests/compas/geometry/test_triangulation_earclip.py
- Update
test_earclip_polygon_wrong_windingexpected faces to reflect correct normal calculation for the complex concave polygon
Example
from compas.geometry import Polygon
# L-shaped concave polygon
points = [
[0, 0, 0], [2, 0, 0], [2, 1, 0],
[1, 1, 0], [1, 2, 0], [0, 2, 0]
]
polygon = Polygon(points)
# Now correctly computes normal considering all vertices
print(polygon.normal) # Vector(x=-0.000, y=-0.000, z=1.000)
print(polygon.is_convex) # False
The improved normal calculation also affects earclip triangulation orientation since it depends on polygon.normal to establish the transformation plane. The new triangulation is equally valid but differs in vertex ordering.
Original prompt
This section details on the original issue you should resolve
<issue_title>
Polygon.normalwithnormal_polygoninstead ofnormal_triangle?</issue_title> <issue_description>Currently, the polygon's normal is calculated as the normal of one of its triangles:normal_triangle([polygon.centroid, polygon.points[0], polygon.points[1]]). However, this is problematic when the polygon is simple but concave, because the first two points might be on the concave ears, as pointed out by the comments insidepolygon.plane.The
compas.geometry.normal_polygonseem to offer a more robust method of calculating such normal, with O(n) instead of O(1) for calculating the normal.Is it possible to incorporate such method into
polygon.normal, since the code is already available in compas? Or let the user choose if they would sacrifice computation efficiency for more accurate handling with concave polygons?</issue_description>Comments on the Issue (you are @copilot in this section)
- Fixes compas-dev/compas#1487
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 62.16%. Comparing base (8221339) to head (9b320ce).
:warning: Report is 13 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #1498 +/- ##
==========================================
- Coverage 62.21% 62.16% -0.05%
==========================================
Files 208 208
Lines 22437 22437
==========================================
- Hits 13959 13949 -10
- Misses 8478 8488 +10
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.