Fixed G2/3 command generation for flipped arcs.

src/Mod/Path/PathScripts/PathDressupHoldingTags.py

@@ -44,8 +44,8 @@ def translate(text, context = "PathDressup_HoldingTags", disambig=None):
     return QtCore.QCoreApplication.translate(context, text, disambig)
 
 
-LOG_MODULE = 'PathDressupHoldingTags'
-#PathLog.setLevel(PathLog.Level.DEBUG, LOG_MODULE)
+LOG_MODULE = PathLog.thisModule()
+PathLog.setLevel(PathLog.Level.INFO, LOG_MODULE)
 
 if FreeCAD.GuiUp:
     from pivy import coin

src/Mod/Path/PathScripts/PathGeom.py

@@ -32,7 +32,7 @@
 
 PathGeomTolerance = 0.000001
 
-#PathLog.setLevel(PathLog.Level.INFO, PathLog.thisModule())
+#PathLog.setLevel(PathLog.Level.DEBUG, PathLog.thisModule())
 
 class Side:
     """Class to determine and define the side a Path is on, or Vectors are in relation to each other."""
@@ -162,31 +162,35 @@ def cmdsForEdge(cls, edge, flip = False, useHelixForBSpline = True, segm = 50):
         if type(edge.Curve) == Part.Line or type(edge.Curve) == Part.LineSegment:
             commands =  [Path.Command('G1', params)]
         else:
-            if not flip:
-                p1 = edge.valueAt(edge.FirstParameter)
-                p3 = pt
-            else:
-                p1 = pt
-                p3 = edge.valueAt(edge.LastParameter)
+            p1 = edge.valueAt(edge.FirstParameter) if not flip else edge.valueAt(edge.LastParameter)
             p2 = edge.valueAt((edge.FirstParameter + edge.LastParameter)/2)
+            p3 = pt
+
             if (type(edge.Curve) == Part.Circle and cls.isRoughly(edge.Curve.Axis.x, 0) and cls.isRoughly(edge.Curve.Axis.y, 0)) or (useHelixForBSpline and type(edge.Curve) == Part.BSplineCurve):
-                if Side.Left == Side.of(p2 - p1, p3 - p2):
-                    cmd = 'G3'
+                # This is an arc or a helix and it should be represented by a simple G2/G3 command
+                if edge.Curve.Axis.z < 0:
+                    cmd = 'G2' if not flip else 'G3'
                 else:
-                    cmd = 'G2'
-                #print("**** (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f)" % (p1.x, p1.y, p1.z, p2.x, p2.y, p2.z, p3.x, p3.y, p3.z))
+                    cmd = 'G3' if not flip else 'G2'
                 pd = Part.Circle(PathGeom.xy(p1), PathGeom.xy(p2), PathGeom.xy(p3)).Center
+                PathLog.info("**** %s.%d: (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f) -> center=(%.2f, %.2f)" % (cmd, flip, p1.x, p1.y, p1.z, p2.x, p2.y, p2.z, p3.x, p3.y, p3.z, pd.x, pd.y))
 
+                # Have to calculate the center in the XY plane, using pd leads to an error if this is a helix
                 pa = PathGeom.xy(p1)
                 pb = PathGeom.xy(p2)
                 pc = PathGeom.xy(p3)
-                #print("**** (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f)" % (pa.x, pa.y, pa.z, pc.x, pc.y, pc.z))
-                #print("**** (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f)" % (pb.x, pb.y, pb.z, pd.x, pd.y, pd.z))
-                offset = Part.Circle(PathGeom.xy(p1), PathGeom.xy(p2), PathGeom.xy(p3)).Center - p1
-                #print("**** (%.2f, %.2f, %.2f)" % (offset.x, offset.y, offset.z))
+                offset = Part.Circle(pa, pb, pc).Center - pa
+
+                PathLog.debug("**** (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f)" % (pa.x, pa.y, pa.z, pc.x, pc.y, pc.z))
+                PathLog.debug("**** (%.2f, %.2f, %.2f) - (%.2f, %.2f, %.2f)" % (pb.x, pb.y, pb.z, pd.x, pd.y, pd.z))
+                PathLog.debug("**** (%.2f, %.2f, %.2f)" % (offset.x, offset.y, offset.z))
+
                 params.update({'I': offset.x, 'J': offset.y, 'K': (p3.z - p1.z)/2})
                 commands = [ Path.Command(cmd, params) ]
+
             else:
+                # We're dealing with a helix or a more complex shape and it has to get approximated
+                # by a number of straight segments
                 eStraight = Part.Edge(Part.LineSegment(p1, p3))
                 esP2 = eStraight.valueAt((eStraight.FirstParameter + eStraight.LastParameter)/2)
                 deviation = (p2 - esP2).Length

src/Mod/Path/PathTests/PathTestUtils.py

@@ -110,3 +110,14 @@ def assertConeAt(self, solid, pt, r1, r2, h):
         self.assertLine(hull, Vector(pt.x+r1, pt.y, pt.z), Vector(pt.x+r2, pt.y, pt.z+h))
         self.assertCircle(base, Vector(pt.x, pt.y, pt.z), r1)
 
+    def assertCommandEqual(self, c1, c2):
+        """Verify that the 2 commands are equivalent."""
+        self.assertEqual(c1.Name, c2.Name)
+
+        self.assertRoughly(c1.Parameters.get('X', 0), c2.Parameters.get('X', 0))
+        self.assertRoughly(c1.Parameters.get('Y', 0), c2.Parameters.get('Y', 0))
+        self.assertRoughly(c1.Parameters.get('Z', 0), c2.Parameters.get('Z', 0))
+
+        self.assertRoughly(c1.Parameters.get('I', 0), c2.Parameters.get('I', 0))
+        self.assertRoughly(c1.Parameters.get('J', 0), c2.Parameters.get('J', 0))
+        self.assertRoughly(c1.Parameters.get('K', 0), c2.Parameters.get('K', 0))

src/Mod/Path/PathTests/TestPathGeom.py

@@ -122,6 +122,25 @@ def test30(self):
                     Path.Command('G3', {'X': p2.x, 'Y': p2.y, 'Z': p2.z, 'I': 1, 'J': 0, 'K': -1}), p1),
                 p1, Vector(-1/math.sqrt(2), -1/math.sqrt(2), 1), p2)
 
+    def test40(self):
+        """Verify arc results in proper G2/3 command."""
+        p1 = Vector(  0, -10, 0)
+        p2 = Vector(-10,   0, 0)
+        p3 = Vector(  0, +10, 0)
+        p4 = Vector(+10,   0, 0)
+
+        def cmds(pa, pb, pc, flip):
+            return PathGeom.cmdsForEdge(Part.Edge(Part.Arc(pa, pb, pc)), flip)[0]
+        def cmd(c, end, off):
+            return Path.Command(c, {'X': end.x, 'Y': end.y, 'Z': end.z, 'I': off.x, 'J': off.y, 'K': off.z})
+
+        self.assertCommandEqual(cmds(p1, p2, p3, False), cmd('G2', p3, Vector(0,  10, 0)))
+        self.assertCommandEqual(cmds(p1, p4, p3, False), cmd('G3', p3, Vector(0,  10, 0)))
+
+        self.assertCommandEqual(cmds(p1, p2, p3,  True), cmd('G3', p1, Vector(0, -10, 0)))
+        self.assertCommandEqual(cmds(p1, p4, p3,  True), cmd('G2', p1, Vector(0, -10, 0)))
+
+
     def test50(self):
         """Verify proper wire(s) aggregation from a Path."""
         commands = []