kicad-developers team mailing list archive

[Thomas Eriksson <scrizt@xxxxxxxxx>]

I've been looking into the DRC-correctness (or lack thereof) when routing
on the edge, so to say.

First of all I noted some places where (I think) the greater-than and
lesser-than tests where wrong in inclusion/exclusion (greater-than versus
greater-than-or-equal.)

One such place was the first test in DRC::checkMarginToCircle(). A trivial
case, according to the comments, yet still wrong.

Then along came the beast. DRC::checkLine(). I found 8 tests which I
believe was wrong. It would be nice with some code reviewing here!

These changes are attached as a patch for
pcbnew/drc_clearance_test_functions.cpp.

I've also started looking into some rounding errors (with so far good
results,) but that's a story for another day (much bigger patch.)

Please look at my changes and tell me what you think.

Best,
Thomas

=== modified file 'pcbnew/drc_clearance_test_functions.cpp'
--- pcbnew/drc_clearance_test_functions.cpp	2013-09-11 15:30:21 +0000
+++ pcbnew/drc_clearance_test_functions.cpp	2013-11-26 08:25:14 +0000
@@ -806,7 +806,7 @@
 }
 
 
-/* test if distance between a segment is > aMinDist
+/* test if distance between a segment is >= aMinDist
  * segment start point is assumed in (0,0) and  segment start point in m_segmEnd
  * and its orientation is m_segmAngle (m_segmAngle must be already initialized)
  * and have aSegmentWidth.
@@ -1011,7 +1011,7 @@
  */
 bool DRC::checkMarginToCircle( wxPoint aCentre, int aRadius, int aLength )
 {
-    if( abs( aCentre.y ) > aRadius )     // trivial case
+    if( abs( aCentre.y ) >= aRadius )     // trivial case
         return true;
 
     // Here, distance between aCentre and X axis is < aRadius
@@ -1056,14 +1056,14 @@
     if( aSegStart.x > aSegEnd.x )
         EXCHG( aSegStart, aSegEnd );
 
-    if( (aSegEnd.x < m_xcliplo) || (aSegStart.x > m_xcliphi) )
+    if( (aSegEnd.x <= m_xcliplo) || (aSegStart.x >= m_xcliphi) )
     {
         WHEN_OUTSIDE;
     }
 
     if( aSegStart.y < aSegEnd.y )
     {
-        if( (aSegEnd.y < m_ycliplo) || (aSegStart.y > m_ycliphi) )
+        if( (aSegEnd.y <= m_ycliplo) || (aSegStart.y >= m_ycliphi) )
         {
             WHEN_OUTSIDE;
         }
@@ -1073,7 +1073,7 @@
             temp = USCALE( (aSegEnd.x - aSegStart.x), (m_ycliplo - aSegStart.y),
                            (aSegEnd.y - aSegStart.y) );
 
-            if( (aSegStart.x += temp) > m_xcliphi )
+            if( (aSegStart.x += temp) >= m_xcliphi )
             {
                 WHEN_OUTSIDE;
             }
@@ -1087,7 +1087,7 @@
             temp = USCALE( (aSegEnd.x - aSegStart.x), (aSegEnd.y - m_ycliphi),
                            (aSegEnd.y - aSegStart.y) );
 
-            if( (aSegEnd.x -= temp) < m_xcliplo )
+            if( (aSegEnd.x -= temp) <= m_xcliplo )
             {
                 WHEN_OUTSIDE;
             }
@@ -1116,7 +1116,7 @@
     }
     else
     {
-        if( (aSegStart.y < m_ycliplo) || (aSegEnd.y > m_ycliphi) )
+        if( (aSegStart.y <= m_ycliplo) || (aSegEnd.y >= m_ycliphi) )
         {
             WHEN_OUTSIDE;
         }
@@ -1126,7 +1126,7 @@
             temp = USCALE( (aSegEnd.x - aSegStart.x), (aSegStart.y - m_ycliphi),
                            (aSegStart.y - aSegEnd.y) );
 
-            if( (aSegStart.x += temp) > m_xcliphi )
+            if( (aSegStart.x += temp) >= m_xcliphi )
             {
                 WHEN_OUTSIDE;
             }
@@ -1140,7 +1140,7 @@
             temp = USCALE( (aSegEnd.x - aSegStart.x), (m_ycliplo - aSegEnd.y),
                            (aSegStart.y - aSegEnd.y) );
 
-            if( (aSegEnd.x -= temp) < m_xcliplo )
+            if( (aSegEnd.x -= temp) <= m_xcliplo )
             {
                 WHEN_OUTSIDE;
             }
@@ -1168,10 +1168,10 @@
         }
     }
 
-    if( ( (aSegEnd.x + aSegStart.x) / 2 <= m_xcliphi )
-       && ( (aSegEnd.x + aSegStart.x) / 2 >= m_xcliplo ) \
-       && ( (aSegEnd.y + aSegStart.y) / 2 <= m_ycliphi )
-       && ( (aSegEnd.y + aSegStart.y) / 2 >= m_ycliplo ) )
+    if( ( (aSegEnd.x + aSegStart.x) / 2 < m_xcliphi )
+       && ( (aSegEnd.x + aSegStart.x) / 2 > m_xcliplo )
+       && ( (aSegEnd.y + aSegStart.y) / 2 < m_ycliphi )
+       && ( (aSegEnd.y + aSegStart.y) / 2 > m_ycliplo ) )
     {
         return false;
     }