horizon-eda / horizon

Horizon is a free EDA package
https://horizon-eda.org/
GNU General Public License v3.0
1.11k stars 82 forks source link

Fix board pdf export drawing tiny bezier arcs #690

Closed frmdstryr closed 2 years ago

frmdstryr commented 2 years ago

I'm working on automating some stencil workflows and the current pdf export contains bezier commands with all equal points that appear to be due to float rounding/tolerances. Edit: This occurs with rounded rect pads.

pdf command: c [-9.523, 7.078, -9.523, 7.078, -9.523, 7.078] is invalid
pdf command: c [-4.512, 12.089, -4.512, 12.089, -4.512, 12.089] is invalid

With this change I don't see any more warnings emitted.

frmdstryr commented 2 years ago

Well it's better than it was.... but something still seems off. I'm not sure why the last point of the last arc is so far off the start point.

// Before
pdf command: m [-9.522, 7.075]
pdf command: c [-9.446, 7.0, -9.446, 6.875, -9.523, 6.797]
pdf command: c [-9.601, 6.72, -9.726, 6.72, -9.804, 6.797]
pdf command: c [-9.881, 6.875, -9.881, 7.0, -9.804, 7.078]
pdf command: c [-9.726, 7.155, -9.601, 7.155, -9.523, 7.078]
pdf command: c [-9.523, 7.078, -9.523, 7.078, -9.523, 7.078]
pdf command: l [-9.522, 7.075]
pdf command: h []

// After
pdf command: m [-9.522, 7.075]
pdf command: c [-9.446, 7.0, -9.446, 6.875, -9.523, 6.797]
pdf command: c [-9.601, 6.72, -9.726, 6.72, -9.804, 6.797]
pdf command: c [-9.881, 6.875, -9.881, 7.0, -9.804, 7.078]
pdf command: c [-9.726, 7.155, -9.601, 7.155, -9.523, 7.078]
pdf command: l [-9.522, 7.075]
pdf command: h []
carrotIndustries commented 2 years ago

With this change I don't see any more warnings emitted.

Where do these warnings come from? Is there an observable visual difference in the PDF itself?

I'm working on automating some stencil workflows

Maybe the better long-term solution is adding DXF export. DXF supports actual arcs.

frmdstryr commented 2 years ago

There is no visual difference that I can tell. The warnings are from a script I made that loads it using pdf4py (see tutorial) and checks for a bezier with all equal points. If you import the pdf into inkscape and use the node tool you can see the circles have more than 4 nodes.

I'm trying to boolean union the circles and rectangles and cut an offset path but the small line is causing it to do weird things. Inkscape also has trouble if you do a boolean union with that.

solder-paste

Supporting DXF and/or SVG would probably be the best solution. SVG would be nice for the schematic so it can be shown as a zoomable image on a web page.

frmdstryr commented 2 years ago

So I added some prints and the values in horizon are correct.. Edit: Sorry the pdf curves look correct, but the move to and line to end points to don't match the arc start & end points?

rebuild took 0.008714
render took 59.2593
render took 390.778
render took 688.705
move: -9.80364,6.79705
s:-9.80364,6.79705 e:-9.80364,6.79705 c:-9.94395,6.65674 cw: 1 a0: 7.06858 a1: 0.785398 e: -6.28319 r: 70000
  cur:-9.80364,6.51643 d:-1.5708 a:5.49779
  cur:-10.0843,6.51643 d:-1.5708 a:3.92699
  cur:-10.0843,6.79705 d:-1.5708 a:2.35619
  cur:-9.80364,6.79705 d:-1.5708 a:0.785398
line: -9.80364,6.79705
move: -11.4272,8.42061
s:-11.4272,8.42061 e:-11.4272,8.42061 c:-11.5675,8.2803 cw: 1 a0: 7.06858 a1: 0.785398 e: -6.28319 r: 70000
  cur:-11.4272,8.13999 d:-1.5708 a:5.49779
  cur:-11.7078,8.13999 d:-1.5708 a:3.92699
  cur:-11.7078,8.42061 d:-1.5708 a:2.35619
  cur:-11.4272,8.42061 d:-1.5708 a:0.785398
line: -11.4272,8.42061

but the pdf is not..

from pdf4py.parser import Parser, SequentialParser
f = open('top-paste.pdf', 'rb')
doc = Parser(f)
root = doc.parse_reference(doc.trailer["Root"])
pages = doc.parse_reference(root["Pages"])
page = doc.parse_reference(pages["Kids"][0])
contents = doc.parse_reference(page["Contents"])
data = contents.stream()
len(data)
Out[9]: 306000
data.split(b"\n")[0:100]
Out[10]: 
[b'1 J',
 b'0.000000000 0.000000000 0.000000000 rg',
 b'1.000000000000000 0.000000000000000 0.000000000000000 1.000000000000000 59.000000000000000 59.000000000000000 cm',
 b'q',
 b'0.000000000 0.000000000 0.000000000 rg',
 b'0.000000000 0.000000000 0.000000000 RG',
 b'0.000000000 w',
 b'-9.802204724 6.794645669 m',
 b'-9.726154167 6.719558891 -9.726154167 6.593923005 -9.803644026 6.516433146 c',
 b'-9.881133885 6.438943286 -10.006769771 6.438943286 -10.084259631 6.516433146 c',
 b'-10.161749490 6.593923005 -10.161749490 6.719558891 -10.084259631 6.797048750 c',
 b'-10.006769771 6.874538610 -9.881133885 6.874538610 -9.803644026 6.797048750 c',
 b'-9.802204724 6.794645669 l',
 b'h',
 b'S',
 b'Q',
 b'q',
 b'0.000000000 0.000000000 0.000000000 rg',
 b'0.000000000 0.000000000 0.000000000 RG',
 b'0.000000000 w',
 b'-11.426456693 8.418897638 m',
 b'-11.349715878 8.343120603 -11.349715878 8.217484717 -11.427205738 8.139994858 c',
 b'-11.504695597 8.062504998 -11.630331483 8.062504998 -11.707821342 8.139994858 c',
 b'-11.785311202 8.217484717 -11.785311202 8.343120603 -11.707821342 8.420610462 c',
 b'-11.630331483 8.498100322 -11.504695597 8.498100322 -11.427205738 8.420610462 c',
 b'-11.426456693 8.418897638 l',
 b'h',
 b'S',
 b'Q',
frmdstryr commented 2 years ago

It appears to be due to C++'s implicit casts...

inline void PdfPainterMM::LineToMM( long lX, long lY )
{
    this->LineTo( static_cast<double>(lX) * CONVERSION_CONSTANT,
                  static_cast<double>(lY) * CONVERSION_CONSTANT );
}

// -----------------------------------------------------
// 
// -----------------------------------------------------
inline void PdfPainterMM::MoveToMM( long lX, long lY )
{
    this->MoveTo( static_cast<double>(lX) * CONVERSION_CONSTANT,
                  static_cast<double>(lY) * CONVERSION_CONSTANT );
}
frmdstryr commented 2 years ago

Works perfect now :shipit:

top-paste-fixed

And Inkscape only shows 4 nodes as expected

image

carrotIndustries commented 2 years ago

So the main problem was using the metric integer API of podofo with a maximum precision of 1µm rather than the pt-based API that supports doubles? If I didn't overlook anything you can remove the to_um conversion function.

It'd be good if you could provide a brief description of the problem this PR solves that I can use in the squashed commit.

frmdstryr commented 2 years ago

I believe it has to do with The MM functions casting a double to long, then back to double. Even with an integer value it is different https://godbolt.org/z/KrhM1dvGs

A summary would be something along the lines of "Fix numerical errors with exported pdf arcs"

carrotIndustries commented 2 years ago

Thanks for the bugfix, one less bug in the next release :) This issue also reminded me to investigate why some arcs were drawn incorrectly in the pdf export. The fix landed in 49d41516948c4e72adc6244fcfedeeb370e7052f and the subsequent commit.

frmdstryr commented 2 years ago

Nice, thanks!