kicad-developers team mailing list archive

[Cirilo Bernardo <cirilo.bernardo@xxxxxxxxx>]

The IDF code has a bug in the calculation of the outlines winding direction
and there is a possibility that strange shaped board outlines such as
a crescent (2 arcs), pac-man (1arc, 2 lines), or a modified pac-man with
1 arc and 3 lines can result in the wrong winding being calculated. I could
not trigger the bug with my hand-crafted outlines but I thought I may as
well fix the bug before it is triggered by a user who requires a strange
shaped board.

The VRML exporter also requires the winding calculations but it is immune
to the bug since all arcs are approximated by line segments; the IDF
code is vulnerable since arcs are treated as line segments between the
start and end of the arc, which is obviously wrong.

- Cirilo

=== modified file 'utils/idftools/idf_common.cpp'
--- utils/idftools/idf_common.cpp	2015-02-16 17:45:37 +0000
+++ utils/idftools/idf_common.cpp	2015-05-25 06:25:24 +0000
@@ -1258,8 +1258,12 @@
             if( ( a1 < -MIN_ANG || a1 > MIN_ANG )
                 && ( a2 < -MIN_ANG || a2 > MIN_ANG ) )
             {
-                // we have 2 arcs
-                if( abs( a1 ) >= abs( a2 ) )
+                // we have 2 arcs; the winding is determined by
+                // the longer cord. although the angles are in
+                // degrees, there is no need to convert to radians
+                // to determine the longer cord.
+                if( abs( a1 * outline.front()->radius ) >=
+                    abs( a2 * outline.back()->radius ) )
                 {
                     // winding depends on a1
                     if( a1 < 0.0 )
@@ -1299,7 +1303,7 @@
     }
 
     double winding = dir + ( outline.front()->startPoint.x - outline.back()->endPoint.x )
-    * ( outline.front()->startPoint.y + outline.back()->endPoint.y );
+                      * ( outline.front()->startPoint.y + outline.back()->endPoint.y );
 
     if( winding > 0.0 )
         return false;
@@ -1351,8 +1355,31 @@
     }
 
     outline.push_back( item );
-    dir += ( outline.back()->endPoint.x - outline.back()->startPoint.x )
-    * ( outline.back()->endPoint.y + outline.back()->startPoint.y );
+
+    double ang = outline.back()->angle;
+    double oang = outline.back()->offsetAngle;
+    double radius = outline.back()->radius;
+
+    if( ang < -MIN_ANG || ang > MIN_ANG )
+    {
+        // arcs require special consideration since the winding depends on
+        // the arc length; the arc length is adequately represented by
+        // taking 2 cords from the endpoints to the midpoint of the arc.
+        oang = (oang + ang / 2.0) * M_PI / 180.0;
+        double midx = outline.back()->center.x + radius * cos( oang );
+        double midy = outline.back()->center.y + radius * sin( oang );
+
+        dir += ( outline.back()->endPoint.x - midx )
+        * ( outline.back()->endPoint.y + midy );
+
+        dir += ( midx - outline.back()->startPoint.x )
+        * ( midy + outline.back()->startPoint.y );
+    }
+    else
+    {
+        dir += ( outline.back()->endPoint.x - outline.back()->startPoint.x )
+        * ( outline.back()->endPoint.y + outline.back()->startPoint.y );
+    }
 
     return true;
 }