Re: gEDA-user: geda-user Digest, Vol 46, Issue 27

[Harry Eaton]

  I submitted my first PCB bug report to SF last month (#2946254),
 and
  shortly after added a patch that fixed the problem.  I must admit
 that
  the lack of response was discouraging - but I fully appreciate
 that
  the developers are time poor (I am also!).
 
  BTW thank you to Rikster, for taking the time to try the patch 
  confirm that it fixes the bug, and posting the result back to SF
 :-)
 ...
 The patch looks good to me (although I've only skimmed it). It might
 warrant a definition of what a freckle is, if that term isn't use
 elsewhere.
 The optimisation is probably fine to add. A complete fix would
 address
 the issue in the auto-router as well.
 I'm happy to apply the patch, but I'm heading home now, as its
 getting
 late. Someone bug me to apply the patch!

   It looks to me like the SQ() macro risks integer overflow when squaring
   the lengths. I too only glanced at the patch so maybe I'm wrong.
   This is one of the difficulties in getting patches on a fast track. The
   internals of pcb are ugly and hard to understand in a lot of ways, and
   even many of the developers don't fully understand them (myself
   included these days!!). pcb has a huge amount of cruft from its 20 year
   life. It is extremely easy to create a patch that on its face looks
   good, appears to fix the problem at hand and passes certain tests, BUT,
   introduces ugly lurking bugs that can be a nightmare to find and
   resolve.
   harry


___
geda-user mailing list
geda-user@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-user

Re: gEDA-user: geda-user Digest, Vol 46, Issue 27

[Stephen Ecob]

On Mon, Mar 8, 2010 at 10:13 PM, Harry Eaton bump...@gmail.com wrote:

[...]

   It looks to me like the SQ() macro risks integer overflow when squaring
   the lengths.

Thank you, good point.

Here's an updated patch that uses Manhattan distance instead - for the
tiny lengths involved it should be fine.

Peer review saves the day :)

diff --git a/src/djopt.c b/src/djopt.c
index 68b3641..230b3c7 100644
--- a/src/djopt.c
+++ b/src/djopt.c
@@ -75,6 +75,9 @@ RCSID ($Id$);
 #define ORIENT(x) ((x)  0xf0)
 #define DIRECT(x) ((x)  0x0f)

+/* Manhattan length of the longest freckle */
+#define LONGEST_FRECKLE2
+
 struct line_s;

 typedef struct corner_s
@@ -1230,6 +1233,33 @@ simple_optimize_corner (corner_s * c)
}
 }
   check (c, 0);
+  if (c-n_lines == 1  !c-via)
+{
+  corner_s *c0 = other_corner (c-lines[0], c);
+  if (abs(c-x - c0-x) + abs(c-y - c0-y) = LONGEST_FRECKLE)
+   {
+  /*
+   * Remove this line, as it is a freckle.  A freckle is an extremely
+   * short line (around 0.01 thou) that is unconnected at one end.
+   * Freckles are almost insignificantly small, but are annoying as
+   * they prevent the mitering optimiser from working.
+   * Freckles sometimes arise because of a bug in the autorouter that
+   * causes it to create small overshoots (typically 0.01 thou) at the
+   * intersections of vertical and horizontal lines. These overshoots
+   * are converted to freckles as a side effect of canonicalize_line().
+   * Note that canonicalize_line() is not at fault, the bug is in the
+   * autorouter creating overshoots.
+   * The autorouter bug arose some time between the 20080202
and 20091103
+   * releases.
+   * This code is probably worth keeping even when the
autorouter bug is
+   * fixed, as freckles could conceivably arise in other ways.
+   */
+ dprintf (freckle %d,%d to %d,%d\n,
+  c-x, c-y, c0-x, c0-y);
+ move_corner (c, c0-x, c0-y);
+   }
+}
+  check (c, 0);
   return rv;
 }


___
geda-user mailing list
geda-user@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-user