(2/3) Fix wrong hulls on ordinary input

cadquery/hull.py

@@ -1,8 +1,8 @@
 from typing import Dict, List, Tuple, Union, Iterable, Set
 from math import pi, sin, cos, atan2, sqrt, inf, degrees
-from numpy import lexsort, argmin, argmax
+from numpy import lexsort, argmin
 
-from .occ_impl.shapes import Edge, Wire
+from .occ_impl.shapes import Edge, Wire, wire
 from .occ_impl.geom import Vector
 
 
@@ -214,26 +214,17 @@ def _pt_arc(p: Point, a: Arc) -> Tuple[float, float, float, float]:
 def pt_arc(p: Point, a: Arc) -> Tuple[float, Segment]:
 
     x, y = p.x, p.y
-    x1, y1, x2, y2 = _pt_arc(p, a)
+    x1, y1, _, _ = _pt_arc(p, a)
 
-    angles = atan2p(x1 - x, y1 - y), atan2p(x2 - x, y2 - y)
-    points = Point(x1, y1), Point(x2, y2)
-    ix = int(argmin(angles))
-
-    return angles[ix], Segment(p, points[ix])
+    return atan2p(x1 - x, y1 - y), Segment(p, Point(x1, y1))
 
 
 def arc_pt(a: Arc, p: Point) -> Tuple[float, Segment]:
 
     x, y = p.x, p.y
-    x1, y1, x2, y2 = _pt_arc(p, a)
-
-    angles = atan2p(x - x1, y - y1), atan2p(x - x2, y - y2)
-    points = Point(x1, y1), Point(x2, y2)
+    _, _, x2, y2 = _pt_arc(p, a)
 
-    ix = int(argmax(angles))
-
-    return angles[ix], Segment(points[ix], p)
+    return atan2p(x - x2, y - y2), Segment(Point(x2, y2), p)
 
 
 def arc_arc(a1: Arc, a2: Arc) -> Tuple[float, Segment]:
@@ -340,7 +331,7 @@ def update_hull(
     angles: List[float],
     segments: List[Segment],
     hull: Hull,
-) -> Tuple[Entity, float, bool]:
+) -> Tuple[Entity, float]:
 
     next_e = entities[ix]
     connecting_seg = segments[ix]
@@ -350,7 +341,7 @@ def update_hull(
 
     hull.extend((connecting_seg, next_e))
 
-    return next_e, angles[ix], next_e is hull[0]
+    return next_e, angles[ix]
 
 
 def finalize_hull(hull: Hull) -> Wire:
@@ -383,7 +374,7 @@ def finalize_hull(hull: Hull) -> Wire:
             Edge.makeCircle(el_n.r, Vector(el_n.c.x, el_n.c.y), angle1=a1, angle2=a2)
         )
 
-    return Wire.assembleEdges(rv)
+    return wire(*rv)
 
 
 def find_hull(edges: Iterable[Edge]) -> Wire:
@@ -405,10 +396,9 @@ def find_hull(edges: Iterable[Edge]) -> Wire:
 
     current_e = start
     current_angle = 0.0
-    finished = False
 
     # march around
-    while not finished:
+    while True:
 
         angles = []
         segments = []
@@ -418,18 +408,19 @@ def find_hull(edges: Iterable[Edge]) -> Wire:
             angles.append(angle if angle >= current_angle else inf)
             segments.append(segment)
 
-        next_ix = int(argmin(angles))
-
-        if angles[next_ix] == inf:
+        # nothing left to reach: closed if back at the start, stuck otherwise
+        if min(angles, default=inf) == inf:
             # nothing reaches the largest circle: everything else is inside it
             if len(rv) == 1 and start is max(arcs, key=lambda a: a.r, default=None):
                 return Wire.assembleEdges(
                     [Edge.makeCircle(start.r, Vector(start.c.x, start.c.y))]
                 )
+            if current_e is not start or len(rv) == 1:
+                raise ValueError("Hull could not be closed")
+            break
 
-            raise ValueError("Hull could not be closed")
-
-        current_e, current_angle, finished = update_hull(
+        next_ix = int(argmin(angles))
+        current_e, current_angle = update_hull(
             current_e, next_ix, entities, angles, segments, rv
         )
 

tests/test_hull.py

@@ -5,6 +5,7 @@
 
 import cadquery as cq
 from cadquery import hull
+from cadquery.func import face
 
 
 def area(edges):
@@ -99,15 +100,16 @@ def test_single_circle():
     assert area([cq.Edge.makeCircle(5.0, (0, 0, 0))]) == pytest.approx(25 * pi)
 
 
-def test_stalled_march():
-    # valid input the march cannot close; it used to loop forever
+def test_march_closes():
     edges = [
         cq.Edge.makeCircle(6.0, (0, 12, 0)),
         cq.Edge.makeLine(cq.Vector(-2, 5), cq.Vector(9, 10)),
     ]
 
-    with pytest.raises(ValueError):
-        hull.find_hull(edges)
+    h = cq.Face.makeFromWires(hull.find_hull(edges))
+
+    for v in edges[1].Vertices():
+        assert h.distance(v) == pytest.approx(0.0)
 
 
 def test_arc_endpoints():
@@ -156,3 +158,48 @@ def test_intersecting_circles():
 
     assert len(edges) == 6
     assert area(edges) == pytest.approx(38 * 70 + pi * 35 ** 2)
+
+
+def test_hull_contains_input():
+    # the line end at (20, 0) lies on the +x axis of the circle, the 0/2pi seam
+    edges = [
+        cq.Edge.makeCircle(5.0, (0, 0, 0)),
+        cq.Edge.makeLine(cq.Vector(20, 0), cq.Vector(20, 10)),
+    ]
+
+    h = cq.Face.makeFromWires(hull.find_hull(edges))
+
+    assert h.distance(cq.Vertex.makeVertex(20, 0, 0)) == pytest.approx(0.0)
+
+
+def test_rotation_invariance():
+    def shape(dx, dy):
+        return [
+            cq.Edge.makeCircle(20.0, (0, 0, 0)),
+            cq.Edge.makeCircle(10.0, (dx, dy, 0)),
+            cq.Edge.makeCircle(10.0, (-dx, -dy, 0)),
+        ]
+
+    assert area(shape(0, 40)) == pytest.approx(area(shape(40, 0)))
+
+
+def test_hull_face_normal():
+    # #1891: func.face reads the edges in storage order - keep the march order
+    edges = [
+        cq.Edge.makeLine(cq.Vector(0, 0), cq.Vector(4, 0)),
+        cq.Edge.makeLine(cq.Vector(4, 0), cq.Vector(0, 3)),
+        cq.Edge.makeLine(cq.Vector(0, 3), cq.Vector(0, 0)),
+    ]
+
+    assert face(hull.find_hull(edges)).normalAt().z == pytest.approx(1)
+
+
+# temporary: two plain circles stall the march until the arc rework (3/3)
+@pytest.mark.xfail(strict=True, raises=ValueError, reason="fixed by the arc rework")
+def test_two_circles():
+    edges = [
+        cq.Edge.makeCircle(0.7, (-3.8, 2.9, 0)),
+        cq.Edge.makeCircle(2.4, (0.8, 4.6, 0)),
+    ]
+
+    assert area(edges) == pytest.approx(25.943, abs=1e-3)