Closes #786 Asymmetric Chamfer Error

cadquery/cq.py

@@ -1320,6 +1320,7 @@ def chamfer(self: T, length: float, length2: Optional[float] = None) -> T:
         :param length2: optional parameter for asymmetrical chamfer
         :raises ValueError: if at least one edge is not selected
         :raises ValueError: if the solid containing the edge is not in the chain
+        :raises ValueError: if the asymmetric mode and can't find face in stack
         :returns: CQ object with the resulting solid selected.
 
         This example will create a unit cube, with the top edges chamfered::
@@ -1332,12 +1333,29 @@ def chamfer(self: T, length: float, length2: Optional[float] = None) -> T:
         """
         solid = self.findSolid()
 
-        edgeList = cast(List[Edge], self.edges().vals())
-        if len(edgeList) < 1:
+        edges_list = cast(List[Edge], self.edges().vals())
+        if len(edges_list) < 1:
             raise ValueError("Chamfer requires that edges be selected")
 
-        s = solid.chamfer(length, length2, edgeList)
-
+        def find_faces(x: T) -> List[Face]:
+            faces = []
+            for o in x.objects:
+                # filter faces
+                if isinstance(o, Face):
+                    faces.append(cast(Face, o))
+                # break if reached deeper stack level
+                elif any(isinstance(o, t) for t in [Solid, Compound]):
+                    return []
+
+            # return found faces
+            if len(faces) > 0:
+                return faces
+
+            # if faces not fond go to the next stack level
+            return find_faces(x.end())
+
+        faces_list = find_faces(self)
+        s = solid.chamfer(length, length2, edges_list, cast(List[Face], faces_list))
         return self.newObject([s])
 
     def transformed(

cadquery/occ_impl/shapes.py

@@ -2888,39 +2888,85 @@ def fillet(self: Any, radius: float, edgeList: Iterable[Edge]) -> Any:
         return self.__class__(fillet_builder.Shape())
 
     def chamfer(
-        self: Any, length: float, length2: Optional[float], edgeList: Iterable[Edge]
+        self: Any,
+        length: float,
+        length2: Optional[float],
+        edgeList: Iterable[Edge],
+        facesList: Iterable[Face],
     ) -> Any:
         """
         Chamfers the specified edges of this solid.
 
         :param length: length > 0, the length (length) of the chamfer
         :param length2: length2 > 0, optional parameter for asymmetrical chamfer. Should be `None` if not required.
-        :param edgeList:  a list of Edge objects, which must belong to this solid
+        :param edgeList: a list of Edge objects, which must belong to this solid
+        :param facesList: a list of Face objects, which must belong to this solid
+        :raises ValueError: if the asymmetric mode and can't find face in stack
         :return: Chamfered solid
         """
-        nativeEdges = [e.wrapped for e in edgeList]
+        native_edges = [e.wrapped for e in edgeList]
+        native_faces = [f.wrapped for f in facesList]
+
+        # note: we prefer 'length' word to 'radius' as opposed to FreeCAD's API
+        chamfer_builder = BRepFilletAPI_MakeChamfer(self.wrapped)
+
+        # note: finding right face is very simple if only one fase selected
+        if len(native_faces) == 1:
+            for edge in native_edges:
+                chamfer_builder.Add(
+                    length, length2 or length, edge, TopoDS.Face_s(native_faces[0])
+                )
+            return self.__class__(chamfer_builder.Shape())
+
+        if (len(native_faces) == 0) and (length2 is not None):
+            raise ValueError(
+                "If chamber length2 not None edges must be selected on faces"
+            )
 
         # make a edge --> faces mapping
         edge_face_map = TopTools_IndexedDataMapOfShapeListOfShape()
         TopExp.MapShapesAndAncestors_s(
             self.wrapped, ta.TopAbs_EDGE, ta.TopAbs_FACE, edge_face_map
         )
 
-        # note: we prefer 'length' word to 'radius' as opposed to FreeCAD's API
-        chamfer_builder = BRepFilletAPI_MakeChamfer(self.wrapped)
+        # note: if edges selected directly
+        if len(native_faces) == 0:
+            for edge in native_edges:
+                face = edge_face_map.FindFromKey(edge).First()
+                chamfer_builder.Add(length, length, edge, TopoDS.Face_s(face))
+            return self.__class__(chamfer_builder.Shape())
 
-        if length2:
-            d1 = length
-            d2 = length2
-        else:
-            d1 = length
-            d2 = length
+        # note: selected multiple faces
+        # we need determinate face for each edge
+
+        for edge in native_edges:
+            faces = edge_face_map.FindFromKey(edge)
+            edge_selected_faces = []
+            for face in native_faces:
+                if any(face.IsSame(f) for f in faces):
+                    edge_selected_faces.append(face)
+
+            if len(edge_selected_faces) == 0:
+                raise ValueError(
+                    "Unexpected error. Faces selected but dont contain current edge."
+                )
+
+            elif len(edge_selected_faces) == 1:
+                chamfer_builder.Add(
+                    length,
+                    length2 or length,
+                    edge,
+                    TopoDS.Face_s(edge_selected_faces[0]),
+                )
+
+            else:
+                chamfer_builder.Add(
+                    length2 or length,
+                    length2 or length,
+                    edge,
+                    TopoDS.Face_s(edge_selected_faces[0]),
+                )
 
-        for e in nativeEdges:
-            face = edge_face_map.FindFromKey(e).First()
-            chamfer_builder.Add(
-                d1, d2, e, TopoDS.Face_s(face)
-            )  # NB: edge_face_map return a generic TopoDS_Shape
         return self.__class__(chamfer_builder.Shape())
 
     def shell(

tests/__init__.py

@@ -62,4 +62,5 @@ def assertTupleAlmostEquals(self, expected, actual, places, msg=None):
     "TestImporters",
     "TestJupyter",
     "TestWorkplanes",
+    "TestChamfer",
 ]

tests/test_chamfer.py

@@ -0,0 +1,199 @@
+from unittest import TestCase, main
+from typing import Optional, Union, List, Tuple, cast
+from cadquery import Workplane
+from cadquery.occ_impl.shapes import Vertex
+from math import atan, sqrt, fabs, degrees
+
+from cadquery.vis import show
+
+
+class TestCase3D(TestCase):
+    def assertAlmostEqualVertices(
+        self,
+        first: Workplane,
+        second: Union[Workplane, List[Tuple[float, float, float]]],
+        places: Optional[int] = None,
+        msg: Optional[str] = None,
+        delta: Optional[float] = None,
+    ):
+        first = sorted([cast(Vertex, x).toTuple() for x in first.vertices().objects])
+        if isinstance(second, Workplane):
+            second = sorted(
+                [cast(Vertex, x).toTuple() for x in second.vertices().objects]
+            )
+        else:
+            second = sorted(second)
+
+        # print('['+', '.join([str(x) for x in first])+']')
+        # print('['+', '.join([str(x) for x in second])+']')
+
+        self.assertEqual(len(first), len(second))
+
+        for f, s in zip(first, second):
+            distance = fabs(
+                sqrt((f[0] - s[0]) ** 2 + (f[1] - s[1]) ** 2 + (f[2] - s[2]) ** 2)
+            )
+            message = msg or f"{f} != {s} with distance {distance}"
+            self.assertAlmostEqual(
+                distance, 0.0, places=places, msg=message, delta=delta
+            )
+
+
+class TestChamfer(TestCase3D):
+    def test_symmetric_chamfer_all(self):
+        obj1 = Workplane().box(10, 10, 10).chamfer(1)
+        obj2 = obj1.rotate((0, 0, 0), (1, 0, 0), 90)
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_asymmetric_chamfer_x_1(self):
+        obj1 = Workplane().box(10, 10, 10).faces("<X").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .box(8, 10, 10)
+            .faces("<X")
+            .workplane()
+            .rect(10, 10)
+            .extrude(2, taper=degrees(atan(0.5)))
+            .translate((1, 0, 0))
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+        show(obj1)
+
+    def test_asymmetric_chamfer_x_2(self):
+        obj1 = Workplane().box(10, 10, 10).faces(">X").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .box(8, 10, 10)
+            .faces(">X")
+            .workplane()
+            .rect(10, 10)
+            .extrude(2, taper=degrees(atan(0.5)))
+            .translate((-1, 0, 0))
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_asymmetric_chamfer_y_1(self):
+        obj1 = Workplane().box(10, 10, 10).faces("<Y").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .box(10, 8, 10)
+            .faces("<Y")
+            .workplane()
+            .rect(10, 10)
+            .extrude(2, taper=degrees(atan(0.5)))
+            .translate((0, 1, 0))
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_asymmetric_chamfer_y_2(self):
+        obj1 = Workplane().box(10, 10, 10).faces(">Y").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .box(10, 8, 10)
+            .faces(">Y")
+            .workplane()
+            .rect(10, 10)
+            .extrude(2, taper=degrees(atan(0.5)))
+            .translate((0, -1, 0))
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_asymmetric_chamfer_z_1(self):
+        obj1 = Workplane().box(10, 10, 10).faces("<Z").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .box(10, 10, 8)
+            .faces("<Z")
+            .workplane()
+            .rect(10, 10)
+            .extrude(2, taper=degrees(atan(0.5)))
+            .translate((0, 0, 1))
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_asymmetric_chamfer_z_2(self):
+        obj1 = Workplane().box(10, 10, 10).faces(">Z").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .box(10, 10, 8)
+            .faces(">Z")
+            .workplane()
+            .rect(10, 10)
+            .extrude(2, taper=degrees(atan(0.5)))
+            .translate((0, 0, -1))
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_asymmetric_chamfer_xy_1(self):
+        obj1 = Workplane().box(10, 10, 10).faces("<Y or <X").edges("|Z").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .polyline([(5, 5), (5, -3), (4, -5), (-3, -5), (-5, -3), (-5, 4), (-3, 5)])
+            .close()
+            .extrude(5, both=True)
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+        show(obj1)
+
+    def test_asymmetric_chamfer_x_z(self):
+        obj1 = Workplane().box(10, 10, 10).faces("<X").edges("|Z").chamfer(1, 2)
+        obj2 = (
+            Workplane()
+            .polyline(
+                [
+                    (-3.0, -5.0),
+                    (-5.0, -4.0),
+                    (-5.0, 4.0),
+                    (-3.0, 5.0),
+                    (5.0, 5.0),
+                    (5.0, -5.0),
+                ]
+            )
+            .close()
+            .extrude(5, both=True)
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_asymmetric_chamfer_x_z_edges(self):
+        obj1 = (
+            Workplane()
+            .box(10, 10, 10)
+            .faces("<X")
+            .edges("|Z")
+            .edges()
+            .edges()
+            .chamfer(1, 2)
+        )
+        obj2 = (
+            Workplane()
+            .polyline(
+                [
+                    (-3.0, -5.0),
+                    (-5.0, -4.0),
+                    (-5.0, 4.0),
+                    (-3.0, 5.0),
+                    (5.0, 5.0),
+                    (5.0, -5.0),
+                ]
+            )
+            .close()
+            .extrude(5, both=True)
+        )
+        self.assertAlmostEqualVertices(obj1, obj2)
+
+    def test_cylinder_symmetric(self):
+        obj1 = Workplane().circle(10).extrude(10, both=True).faces("|Z").chamfer(1, 2)
+        self.assertAlmostEqualVertices(
+            obj1,
+            [
+                (9.0, 0.0, -10.0),
+                (9.0, 0.0, 10.0),
+                (10.0, 0.0, -8.0),
+                (10.0, 0.0, 0.0),
+                (10.0, 0.0, 8.0),
+            ],
+        )
+
+
+if __name__ == "__main__":
+    main()