Merge pull request #1424 from compas-dev/fix-polygon-surface

Polygon fixes and Brep from step for Rhino

Author: tomvanmele

CHANGELOG.md

@@ -9,9 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+* Added implementation for `compas_rhino.geometry.RhinoBrep.from_step`.
+
 ### Changed
 
 * Changed `requirements.txt` to allow `numpy>=2`.
+* Fixed bug in `compas.geometry.Polygon.points` setter by removing duplicate points if they exist.
+* Fixed bug in `compas.geometry.Polygon.plane` by aligning the normal of the bestfit plane with the approximate normal of the polygon faces.
+* Changed the order of face vertices in `compas.geometry.Surface.to_vertices_and_faces` to a counter clockwise cycling direction and outward facing normals for curved surfaces.
 
 ### Removed
 

src/compas/geometry/polygon.py

@@ -16,6 +16,7 @@
 from compas.geometry import earclip_polygon
 from compas.geometry import is_coplanar
 from compas.geometry import is_polygon_convex
+from compas.geometry import normal_triangle
 from compas.geometry import transform_points
 from compas.itertools import pairwise
 from compas.tolerance import TOL
@@ -135,9 +136,15 @@ def points(self):
 
     @points.setter
     def points(self, points):
-        if points[-1] == points[0]:
-            points = points[:-1]
-        self._points = [Point(*xyz) for xyz in points]
+        previous = Point(*points[0])
+        self._points = [previous]
+        for xyz in points[1:]:
+            if previous == xyz:
+                continue
+            previous = Point(*xyz)
+            self._points.append(previous)
+        if self._points[0] == self._points[-1]:
+            del self._points[-1]
         self._lines = None
 
     @property
@@ -173,7 +180,19 @@ def normal(self):
 
     @property
     def plane(self):
-        return Plane.from_points(self.points)
+        # by just taking the bestfit plane,
+        # the normal might not be aligned with the winding direciton of the polygon
+        # this can be solved by comparing the plane normal with the normal of one of the triangles of the polygon
+        # for convex polygons this is always correct
+        # in the case of concave polygons it may not be
+        # to be entirely correct, the check should be done with one of the polygon ears after earclipping
+        # however, this is costly
+        # and even then it is only correct if we assume th polygon is plat enough to have a consistent direction
+        normal = normal_triangle([self.centroid] + self.points[:2])
+        plane = Plane.from_points(self.points)
+        if plane.normal.dot(normal) < 0:
+            plane.normal.flip()
+        return plane
 
     @property
     def frame(self):

src/compas/geometry/shapes/sphere.py

@@ -176,7 +176,7 @@ def from_point_and_radius(cls, point, radius):  # type: (...) -> Sphere
     # Discretisation
     # ==========================================================================
 
-    def compute_vertices(self):  # type: () -> list[float]
+    def compute_vertices(self):  # type: () -> list[list[float]]
         """Compute the vertices of the discrete representation of the sphere.
 
         Returns

src/compas/geometry/surfaces/surface.py

@@ -252,9 +252,9 @@ def to_vertices_and_faces(self, nu=16, nv=16, du=None, dv=None):
         faces = [
             [
                 i * (nv + 1) + j,
-                (i + 1) * (nv + 1) + j,
-                (i + 1) * (nv + 1) + j + 1,
                 i * (nv + 1) + j + 1,
+                (i + 1) * (nv + 1) + j + 1,
+                (i + 1) * (nv + 1) + j,
             ]
             for i, j in product(range(nu), range(nv))
         ]

src/compas/geometry/triangulation_earclip.py

@@ -18,7 +18,7 @@ class Ear(object):
         Coordinates of the vertex of the Ear triangle.
     next : int
         Index of the next vertex of the Ear triangle.
-    prew : int
+    prev : int
         Index of the previous vertex of the Ear triangle.
     neighbour_coords : list
         Coordinates of the next and previous vertices of the Ear triangle.
@@ -32,10 +32,10 @@ def __init__(self, points, indexes, ind):
         index_in_indexes_arr = indexes.index(ind)
         self.next = indexes[(index_in_indexes_arr + 1) % length]
         if index_in_indexes_arr == 0:
-            self.prew = indexes[length - 1]
+            self.prev = indexes[length - 1]
         else:
-            self.prew = indexes[index_in_indexes_arr - 1]
-        self.neighbour_coords = [points[self.prew], points[self.next]]
+            self.prev = indexes[index_in_indexes_arr - 1]
+        self.neighbour_coords = [points[self.prev], points[self.next]]
 
     def is_inside(self, point):
         """Check if a given point is inside the triangle formed by the Ear.
@@ -120,7 +120,7 @@ def get_triangle(self):
             List of vertex indices forming the Ear triangle.
 
         """
-        return [self.prew, self.index, self.next]
+        return [self.prev, self.index, self.next]
 
 
 class Earcut(object):
@@ -168,7 +168,7 @@ def add_ear(self, new_ear):
 
         """
         self.ears.append(new_ear)
-        self.neighbours.append(new_ear.prew)
+        self.neighbours.append(new_ear.prev)
         self.neighbours.append(new_ear.next)
 
     def find_ears(self):
@@ -229,16 +229,16 @@ def triangulate(self):
             current = self.ears.pop(0)
 
             indexes.remove(current.index)
-            self.neighbours.remove(current.prew)
+            self.neighbours.remove(current.prev)
             self.neighbours.remove(current.next)
 
             self.triangles.append(current.get_triangle())
 
-            # Check if prew and next vertices form new ears
-            prew_ear_new = Ear(self.vertices, indexes, current.prew)
+            # Check if prev and next vertices form new ears
+            prev_ear_new = Ear(self.vertices, indexes, current.prev)
             next_ear_new = Ear(self.vertices, indexes, current.next)
-            if prew_ear_new.validate(self.vertices, indexes, self.ears) and prew_ear_new.index not in self.neighbours:
-                self.add_ear(prew_ear_new)
+            if prev_ear_new.validate(self.vertices, indexes, self.ears) and prev_ear_new.index not in self.neighbours:
+                self.add_ear(prev_ear_new)
                 continue
             if next_ear_new.validate(self.vertices, indexes, self.ears) and next_ear_new.index not in self.neighbours:
                 self.add_ear(next_ear_new)
@@ -270,8 +270,6 @@ def earclip_polygon(polygon):
         If no more ears were found for triangulation.
 
     """
-
-    # Orient the copy of polygon points to XY plane.
     from compas.geometry import Frame  # Avoid circular import.
     from compas.geometry import Plane  # Avoid circular import.
     from compas.geometry import Transformation  # Avoid circular import.

src/compas_rhino/geometry/brep/__init__.py

@@ -1,23 +1,23 @@
-import Rhino  # noqa: F401
+import Rhino  # type: ignore # noqa: F401
 
 from compas.plugins import plugin
 
 from .brep import RhinoBrep
 
 
 @plugin(category="factories", requires=["Rhino"])
-def new_brep(*args, **kwargs):
-    return object.__new__(RhinoBrep)
+def from_boolean_difference(*args, **kwargs):
+    return RhinoBrep.from_boolean_difference(*args, **kwargs)
 
 
 @plugin(category="factories", requires=["Rhino"])
-def from_extrusion(*args, **kwargs):
-    return RhinoBrep.from_extrusion(*args, **kwargs)
+def from_boolean_intersection(*args, **kwargs):
+    return RhinoBrep.from_boolean_intersection(*args, **kwargs)
 
 
 @plugin(category="factories", requires=["Rhino"])
-def from_native(*args, **kwargs):
-    return RhinoBrep.from_native(*args, **kwargs)
+def from_boolean_union(*args, **kwargs):
+    return RhinoBrep.from_boolean_union(*args, **kwargs)
 
 
 @plugin(category="factories", requires=["Rhino"])
@@ -31,8 +31,13 @@ def from_cylinder(*args, **kwargs):
 
 
 @plugin(category="factories", requires=["Rhino"])
-def from_sphere(*args, **kwargs):
-    return RhinoBrep.from_sphere(*args, **kwargs)
+def from_extrusion(*args, **kwargs):
+    return RhinoBrep.from_extrusion(*args, **kwargs)
+
+
+@plugin(category="factories", requires=["Rhino"])
+def from_loft(*args, **kwargs):
+    return RhinoBrep.from_loft(*args, **kwargs)
 
 
 @plugin(category="factories", requires=["Rhino"])
@@ -41,20 +46,20 @@ def from_mesh(*args, **kwargs):
 
 
 @plugin(category="factories", requires=["Rhino"])
-def from_loft(*args, **kwargs):
-    return RhinoBrep.from_loft(*args, **kwargs)
+def from_native(*args, **kwargs):
+    return RhinoBrep.from_native(*args, **kwargs)
 
 
 @plugin(category="factories", requires=["Rhino"])
-def from_boolean_difference(*args, **kwargs):
-    return RhinoBrep.from_boolean_difference(*args, **kwargs)
+def from_sphere(*args, **kwargs):
+    return RhinoBrep.from_sphere(*args, **kwargs)
 
 
 @plugin(category="factories", requires=["Rhino"])
-def from_boolean_intersection(*args, **kwargs):
-    return RhinoBrep.from_boolean_intersection(*args, **kwargs)
+def from_step(*args, **kwargs):
+    return RhinoBrep.from_step(*args, **kwargs)
 
 
 @plugin(category="factories", requires=["Rhino"])
-def from_boolean_union(*args, **kwargs):
-    return RhinoBrep.from_boolean_union(*args, **kwargs)
+def new_brep(*args, **kwargs):
+    return object.__new__(RhinoBrep)