Fix workplane cylinder center when generated using a custom direction (#1593)

* add test for bugfix

* fix

* format

* refactor

* format

* add test for ValueError

* Simplify the code

* update testCylinderCenteringAndDirection

* apply suggestions

---------

Co-authored-by: AU <[email protected]>

Author: kmarchais

cadquery/cq.py

@@ -4156,7 +4156,7 @@ def cylinder(
         self: T,
         height: float,
         radius: float,
-        direct: Vector = Vector(0, 0, 1),
+        direct: Union[Tuple[float, float, float], Vector] = Vector(0, 0, 1),
         angle: float = 360,
         centered: Union[bool, Tuple[bool, bool, bool]] = True,
         combine: CombineMode = True,
@@ -4190,7 +4190,6 @@ def cylinder(
 
         If combine is false, the result will be a list of the cylinders produced.
         """
-
         if isinstance(centered, bool):
             centered = (centered, centered, centered)
 
@@ -4202,7 +4201,10 @@ def cylinder(
         if centered[2]:
             offset += Vector(0, 0, -height / 2)
 
-        s = Solid.makeCylinder(radius, height, offset, direct, angle)
+        # first center and then apply the direction
+        s = Solid.makeCylinder(radius, height, offset, Vector(0, 0, 1), angle).moved(
+            Plane(Vector(), normal=direct).location
+        )
 
         # We want a cylinder for each point on the workplane
         return self.eachpoint(lambda loc: s.moved(loc), True, combine, clean)

tests/test_cadquery.py

@@ -2606,6 +2606,45 @@ def testCylinderCentering(self):
                 s0.val().Center().toTuple(), s1.val().Center().toTuple(), 3
             )
 
+    def testCylinderCenteringAndDirection(self):
+        radius = 10
+        height = 40
+
+        main_directions = [
+            (1, 0, 0),
+            (0, 1, 0),
+            (0, 0, 1),
+            (-1, 0, 0),
+            (0, -1, 0),
+            (0, 0, -1),
+        ]
+
+        centered_values = [
+            (True, True, True),
+            (False, False, False),
+            (True, False, False),
+            (False, True, False),
+            (False, False, True),
+            (True, True, False),
+        ]
+
+        expected_results = [
+            (0, 0, 0),
+            (radius, 0.5 * height, radius),
+            (0, radius, 0.5 * height),
+            (-0.5 * height, 0, -radius),
+            (radius, 0, -radius),
+            (0, 0, -0.5 * height),
+        ]
+
+        for direction, centered, expected_center in zip(
+            main_directions, centered_values, expected_results
+        ):
+            s = Workplane("XY").cylinder(
+                height, radius, centered=centered, direct=direction,
+            )
+            self.assertTupleAlmostEquals(s.val().Center().toTuple(), expected_center, 3)
+
     def testWedgeDefaults(self):
         s = Workplane("XY").wedge(10, 10, 10, 5, 5, 5, 5)
         self.saveModel(s)
@@ -2909,7 +2948,11 @@ def testClean(self):
             .faces("<Z")
             .workplane()
             .cylinder(
-                2, 0.2, centered=(True, True, False), direct=(0, 0, -1), clean=True
+                2,
+                0.2,
+                centered=(True, True, False),
+                direct=Vector(0, 0, -1),
+                clean=True,
             )
         )
         assert len(s.edges().vals()) == 15
@@ -2976,7 +3019,11 @@ def testNoClean(self):
             .faces("<Z")
             .workplane()
             .cylinder(
-                2, 0.2, centered=(True, True, False), direct=(0, 0, -1), clean=False
+                2,
+                0.2,
+                centered=(True, True, False),
+                direct=Vector(0, 0, -1),
+                clean=False,
             )
         )
         assert len(s.edges().vals()) == 16