From a89f3788d2cce87cff21de487a8aad405ae02ea4 Mon Sep 17 00:00:00 2001 From: kimstik Date: Mon, 14 Sep 2026 19:07:41 +0200 Subject: [PATCH 1/3] Pick the tangent by side, not by angle pt_arc and arc_pt chose between the two tangency points by comparing angles already wrapped into [0, 2pi), which picks the wrong one whenever the pair straddles the seam. The march runs counter-clockwise, so the circle must lie left of the supporting segment - and that is always the first point for pt_arc and the second for arc_pt. --- cadquery/hull.py | 19 +++++-------------- tests/test_hull.py | 22 ++++++++++++++++++---- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/cadquery/hull.py b/cadquery/hull.py index 9d3e6787c..caea7c9e8 100644 --- a/cadquery/hull.py +++ b/cadquery/hull.py @@ -1,6 +1,6 @@ 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.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) - - ix = int(argmax(angles)) + _, _, x2, y2 = _pt_arc(p, a) - 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]: diff --git a/tests/test_hull.py b/tests/test_hull.py index 350f9f1ba..550eb9bdc 100644 --- a/tests/test_hull.py +++ b/tests/test_hull.py @@ -99,15 +99,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 +157,16 @@ 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 exactly on the +x axis of the circle, where + # pt_arc/arc_pt pick the tangent by raw angle across 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) From fa1b8992fe21f9197a3fc4c04cc811133d1f7712 Mon Sep 17 00:00:00 2001 From: kimstik Date: Mon, 14 Sep 2026 19:13:30 +0200 Subject: [PATCH 2/3] Close the hull when nothing is left to reach, not on the first return The march stopped as soon as it came back to the start entity, but an arc can lie on the hull more than once - a large circle between two smaller ones does. Keep going instead; the hull is closed when the march stands on the start and every remaining candidate lies behind it. Fixes #931. --- cadquery/hull.py | 20 ++++++++++---------- tests/test_hull.py | 25 +++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/cadquery/hull.py b/cadquery/hull.py index caea7c9e8..d8f5d21de 100644 --- a/cadquery/hull.py +++ b/cadquery/hull.py @@ -331,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] @@ -341,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: @@ -396,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 = [] @@ -409,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 ) diff --git a/tests/test_hull.py b/tests/test_hull.py index 550eb9bdc..92d964e8b 100644 --- a/tests/test_hull.py +++ b/tests/test_hull.py @@ -160,8 +160,7 @@ def test_intersecting_circles(): def test_hull_contains_input(): - # the line end at (20, 0) lies exactly on the +x axis of the circle, where - # pt_arc/arc_pt pick the tangent by raw angle across the 0/2pi seam + # 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)), @@ -170,3 +169,25 @@ def test_hull_contains_input(): 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))) + + +# 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) From cbdf987d88e608a84472e7fd0177cebe5a55950f Mon Sep 17 00:00:00 2001 From: kimstik Date: Tue, 15 Sep 2026 13:01:36 +0200 Subject: [PATCH 3/3] Keep the march order when assembling the hull wire Fixes #1891. --- cadquery/hull.py | 4 ++-- tests/test_hull.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cadquery/hull.py b/cadquery/hull.py index d8f5d21de..ed962e39e 100644 --- a/cadquery/hull.py +++ b/cadquery/hull.py @@ -2,7 +2,7 @@ from math import pi, sin, cos, atan2, sqrt, inf, degrees 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 @@ -374,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: diff --git a/tests/test_hull.py b/tests/test_hull.py index 92d964e8b..e8b84eba6 100644 --- a/tests/test_hull.py +++ b/tests/test_hull.py @@ -5,6 +5,7 @@ import cadquery as cq from cadquery import hull +from cadquery.func import face def area(edges): @@ -182,6 +183,17 @@ def shape(dx, dy): 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():