Make bounding box tests pass

5 files changed, 112 insertions, 28 deletions

diff --git a/gerbonara/gerber/graphic_objects.py b/gerbonara/gerber/graphic_objects.py
index 278401c..82aac67 100644
--- a/gerbonara/gerber/graphic_objects.py
+++ b/gerbonara/gerber/graphic_objects.py
@@ -43,7 +43,7 @@ class GerberObject:
         self._rotate(rotation, cx, cy)
 
     def bounding_box(self, unit=None):
-        bboxes = [ p.bounding_box for p in self.to_primitives(unit) ]
+        bboxes = [ p.bounding_box() for p in self.to_primitives(unit) ]
         min_x = min(min_x for (min_x, _min_y), _ in bboxes)
         min_y = min(min_y for (_min_x, min_y), _ in bboxes)
         max_x = max(max_x for _, (max_x, _max_y) in bboxes)
@@ -237,6 +237,7 @@ class Arc(GerberObject):
     y1 : Length(float)
     x2 : Length(float)
     y2 : Length(float)
+    # relative to (x1, x2)
     cx : Length(float)
     cy : Length(float)
     clockwise : bool
@@ -268,7 +269,7 @@ class Arc(GerberObject):
         conv = self.converted(unit)
         yield gp.Arc(x1=conv.x1, y1=conv.y1,
                 x2=conv.x2, y2=conv.y2,
-                cx=conv.cx, cy=conv.cy,
+                cx=conv.cx+conv.x1, cy=conv.cy+conv.y1,
                 clockwise=self.clockwise,
                 width=self.aperture.equivalent_width(unit),
                 polarity_dark=self.polarity_dark)
diff --git a/gerbonara/gerber/graphic_primitives.py b/gerbonara/gerber/graphic_primitives.py
index 98b8aa1..4e176b2 100644
--- a/gerbonara/gerber/graphic_primitives.py
+++ b/gerbonara/gerber/graphic_primitives.py
@@ -40,6 +40,9 @@ def add_bounds(b1, b2):
     max_x, max_y = max_none(max_x_1, max_x_2), max_none(max_y_1, max_y_2)
     return ((min_x, min_y), (max_x, max_y))
 
+def rad_to_deg(x):
+    return x/math.pi * 180
+
 @dataclass
 class Circle(GraphicPrimitive):
     x : float
@@ -174,11 +177,14 @@ def point_line_distance(l1, l2, p):
     return abs((x2-x1)*(y1-y0) - (x1-x0)*(y2-y1)) / length
 
 def svg_arc(old, new, center, clockwise):
+    print(f'{old=} {new=} {center=}')
     r = point_distance(old, new)
     d = point_line_distance(old, new, center)
-    sweep_flag = int(clockwise)
+    # invert sweep flag since the svg y axis is mirrored
+    sweep_flag = int(not clockwise)
     large_arc = int((d > 0) == clockwise) # FIXME check signs
-    return f'A {r:.6} {r:.6} {large_arc} {sweep_flag} {new[0]:.6} {new[1]:.6}'
+    print(f'{r=:.3} {d=:.3} {sweep_flag=} {large_arc=} {clockwise=}')
+    return f'A {r:.6} {r:.6} 0 {large_arc} {sweep_flag} {new[0]:.6} {new[1]:.6}'
 
 @dataclass
 class ArcPoly(GraphicPrimitive):
@@ -206,6 +212,7 @@ class ArcPoly(GraphicPrimitive):
             else:
                 line_bounds = (min(x1, x2), min(y1, y2)), (max(x1, x2), max(y1, y2))
                 bbox = add_bounds(bbox, line_bounds)
+        return bbox
 
     def __len__(self):
         return len(self.outline)
@@ -251,13 +258,14 @@ class Arc(GraphicPrimitive):
     y1 : float
     x2 : float
     y2 : float
+    # absolute coordinates
     cx : float
     cy : float
     clockwise : bool
     width : float
 
     def bounding_box(self):
-        r = self.w/2
+        r = self.width/2
         endpoints = add_bounds(Circle(self.x1, self.y1, r).bounding_box(), Circle(self.x2, self.y2, r).bounding_box())
 
         arc_r = point_distance((self.cx, self.cy), (self.x1, self.y1))
@@ -272,16 +280,16 @@ class Arc(GraphicPrimitive):
         x2 = self.x2 + dx/arc_r * r
         y2 = self.y2 + dy/arc_r * r
 
-        arc = arc_bounds(x1, y1, x2, y2, cx, cy, self.clockwise)
+        arc = arc_bounds(x1, y1, x2, y2, self.cx, self.cy, self.clockwise)
         return add_bounds(endpoints, arc) # FIXME add "include_center" switch
 
     def to_svg(self, tag, color='black'):
         arc = svg_arc((self.x1, self.y1), (self.x2, self.y2), (self.cx, self.cy), self.clockwise)
         return tag('path', d=f'M {self.x1:.6} {self.y1:.6} {arc}',
-                style=f'stroke: {color}, stroke-width: {self.width:.6}; stroke-linecap: round')
+                style=f'stroke: {color}, stroke-width: {self.width:.6}; stroke-linecap: round; fill: none')
 
-def svg_rotation(angle_rad):
-    return f'rotation({angle_rad/math.pi*180:.4})'
+def svg_rotation(angle_rad, cx=0, cy=0):
+    return f'rotate({float(rad_to_deg(angle_rad)):.4} {float(cx):.6} {float(cy):.6})'
 
 @dataclass
 class Rectangle(GraphicPrimitive):
@@ -313,7 +321,8 @@ class Rectangle(GraphicPrimitive):
 
     def to_svg(self, tag, color='black'):
         x, y = self.x - self.w/2, self.y - self.h/2
-        return tag('rect', x=x, y=y, w=self.w, h=self.h, transform=svg_rotation(self.rotation), style=f'fill: {color}')
+        return tag('rect', x=x, y=y, width=self.w, height=self.h,
+                transform=svg_rotation(self.rotation, self.x, self.y), style=f'fill: {color}')
 
 @dataclass
 class RegularPolygon(GraphicPrimitive):
diff --git a/gerbonara/gerber/rs274x.py b/gerbonara/gerber/rs274x.py
index 3ee8c4d..607b8bc 100644
--- a/gerbonara/gerber/rs274x.py
+++ b/gerbonara/gerber/rs274x.py
@@ -90,7 +90,8 @@ class GerberFile(CamFile):
     def to_svg(self, tag=Tag, margin=0, arg_unit='mm', svg_unit='mm', force_bounds=None, color='black'):
 
         if force_bounds is None:
-            (min_x, min_y), (max_x, max_y) = self.bounding_box(svg_unit)
+            (min_x, min_y), (max_x, max_y) = self.bounding_box(svg_unit, default=((0, 0), (0, 0)))
+            print('bounding box:', (min_x, min_y), (max_x, max_y))
         else:
             (min_x, min_y), (max_x, max_y) = force_bounds
             min_x = convert(min_x, arg_unit, svg_unit)
@@ -106,17 +107,19 @@ class GerberFile(CamFile):
             max_y += margin
 
         w, h = max_x - min_x, max_y - min_y
+        w = 1.0 if math.isclose(w, 0.0) else w
+        h = 1.0 if math.isclose(h, 0.0) else h
 
         primitives = [ prim.to_svg(tag, color) for obj in self.objects for prim in obj.to_primitives(unit=svg_unit) ]
 
         # setup viewport transform flipping y axis
-        xform = f'scale(0 -1) translate(0 {h})'
+        xform = f'translate({min_x} {min_y+h}) scale(1 -1) translate({-min_x} {-min_y})'
 
         svg_unit = 'in' if svg_unit == 'inch' else 'mm'
         # TODO export apertures as <uses> where reasonable.
-        return tag('svg', [*primitives],
+        return tag('svg', [tag('g', primitives, transform=xform)],
                 width=f'{w}{svg_unit}', height=f'{h}{svg_unit}',
-                viewBox=f'{min_x} {min_y} {w} {h}', transform=xform,
+                viewBox=f'{min_x} {min_y} {w} {h}',
                 xmlns="http://www.w3.org/2000/svg", xmlns__xlink="http://www.w3.org/1999/xlink", root=True)
 
     def merge(self, other):
@@ -200,21 +203,24 @@ class GerberFile(CamFile):
         GerberParser(obj, include_dir=enable_include_dir).parse(data)
         return obj
 
-    @property
-    def size(self):
-        (x0, y0), (x1, y1) = self.bounding_box
+    def size(self, unit='mm'):
+        (x0, y0), (x1, y1) = self.bounding_box(unit, default=((0, 0), (0, 0)))
         return (x1 - x0, y1 - y0)
 
-    @property
-    def bounding_box(self, unit='mm'):
+    def bounding_box(self, unit='mm', default=None):
+        """ Calculate bounding box of file. Returns value given by 'default' argument when there are no graphical
+        objects (default: None)
+        """
         bounds = [ p.bounding_box(unit) for p in self.objects ]
+        if not bounds:
+            return default
 
         min_x = min(x0 for (x0, y0), (x1, y1) in bounds)
         min_y = min(y0 for (x0, y0), (x1, y1) in bounds)
         max_x = max(x1 for (x0, y0), (x1, y1) in bounds)
         max_y = max(y1 for (x0, y0), (x1, y1) in bounds)
 
-        return ((min_x, max_x), (min_y, max_y))
+        return ((min_x, min_y), (max_x, max_y))
 
     def generate_statements(self, drop_comments=True):
         yield UnitStmt()
@@ -421,6 +427,11 @@ class GraphicsState:
             self.point = (0, 0)
         old_point = self.map_coord(*self.update_point(x, y))
 
+        if aperture and math.isclose(self.aperture.equivalent_width(), 0):
+            warnings.warn('D01 interpolation with a zero-size aperture. This is invalid according to spec, however, we '
+                    'pass through the created objects here. Note that these will not show up in e.g. SVG output since '
+                    'their line width is zero.', SyntaxWarning)
+
         if self.interpolation_mode == LinearModeStmt:
             if i is not None or j is not None:
                 raise SyntaxError("i/j coordinates given for linear D01 operation (which doesn't take i/j)")
@@ -448,7 +459,11 @@ class GraphicsState:
 
     def _create_arc(self, old_point, new_point, control_point, aperture=True):
         clockwise = self.interpolation_mode == CircularCWModeStmt
-        return go.Arc(*old_point, *new_point,* self.map_coord(*control_point, relative=True),
+        print('creating arc')
+        print('  old point', old_point)
+        print('  new point', new_point)
+        print('  control point', self.map_coord(*control_point, relative=True))
+        return go.Arc(*old_point, *new_point, *self.map_coord(*control_point, relative=True),
                 clockwise=clockwise, aperture=(self.aperture if aperture else None),
                 polarity_dark=self.polarity_dark, unit=self.file_settings.unit)
 
@@ -643,7 +658,7 @@ class GerberParser:
             if self.current_region is None:
                 self.target.objects.append(self.graphics_state.interpolate(x, y, i, j))
             else:
-                self.current_region.append(self.graphics_state.interpolate(x, y, i, j))
+                self.current_region.append(self.graphics_state.interpolate(x, y, i, j, aperture=False))
 
         else:
             if i is not None or j is not None:
@@ -687,6 +702,12 @@ class GerberParser:
             }
 
         if (kls := aperture_classes.get(match['shape'])):
+            if match['shape'] == 'P' and math.isclose(modifiers[0], 0):
+                warnings.warn('Definition of zero-size polygon aperture. This is invalid according to spec.' , SyntaxWarning)
+
+            if match['shape'] in 'RO' and (math.isclose(modifiers[0], 0) or math.isclose(modifiers[1], 0)):
+                warnings.warn('Definition of zero-width and/or zero-height rectangle or obround aperture. This is invalid according to spec.' , SyntaxWarning)
+
             new_aperture = kls(*modifiers, unit=self.file_settings.unit)
 
         elif (macro := self.aperture_macros.get(match['shape'])):
diff --git a/gerbonara/gerber/tests/image_support.py b/gerbonara/gerber/tests/image_support.py
index dc2cbdb..ee84203 100644
--- a/gerbonara/gerber/tests/image_support.py
+++ b/gerbonara/gerber/tests/image_support.py
@@ -60,8 +60,8 @@ def run_cargo_cmd(cmd, args, **kwargs):
     except FileNotFoundError:
         return subprocess.run([str(Path.home() / '.cargo' / 'bin' / cmd), *args], **kwargs)
 
-def svg_to_png(in_svg, out_png, dpi=100):
-    run_cargo_cmd('resvg', ['--dpi', str(dpi), in_svg, out_png], check=True, stdout=subprocess.DEVNULL)
+def svg_to_png(in_svg, out_png, dpi=100, bg='black'):
+    run_cargo_cmd('resvg', ['--background', bg, '--dpi', str(dpi), in_svg, out_png], check=True, stdout=subprocess.DEVNULL)
 
 def gerbv_export(in_gbr, out_svg, format='svg', origin=(0, 0), size=(6, 6), fg='#ffffff'):
     x, y = origin
diff --git a/gerbonara/gerber/tests/test_rs274x.py b/gerbonara/gerber/tests/test_rs274x.py
index 5d3e355..5be0af5 100644
--- a/gerbonara/gerber/tests/test_rs274x.py
+++ b/gerbonara/gerber/tests/test_rs274x.py
@@ -12,6 +12,7 @@ from argparse import Namespace
 from itertools import chain
 from pathlib import Path
 from contextlib import contextmanager
+from PIL import Image
 
 import pytest
 
@@ -54,7 +55,7 @@ def tmpfile(request):
     def register_tempfile(name, suffix):
         nonlocal registered
         f = tempfile.NamedTemporaryFile(suffix=suffix)
-        registered.append((name, f))
+        registered.append((name, suffix, f))
         return Path(f.name)
 
     yield register_tempfile
@@ -62,13 +63,13 @@ def tmpfile(request):
     if request.node.rep_call.failed:
         fail_dir.mkdir(exist_ok=True)
         test_name = path_test_name(request)
-        for name, tmp in registered:
+        for name, suffix, tmp in registered:
             slug = re.sub(r'[^\w\d]+', '_', name.lower())
             perm_path = fail_dir / f'failure_{test_name}_{slug}{suffix}'
             shutil.copy(tmp.name, perm_path)
             print(f'{name} saved to {perm_path}')
 
-    for _name, tmp in registered:
+    for _name, _suffix, tmp in registered:
         tmp.close()
 
 @pytest.fixture
@@ -143,6 +144,15 @@ MIN_REFERENCE_FILES = [
     'eagle_files/copper_bottom_l4.gbr'
     ]
 
+HAS_ZERO_SIZE_APERTURES = [
+    'bottom_copper.GBL',
+    'bottom_silk.GBO',
+    'top_copper.GTL',
+    'top_silk.GTO',
+    'board_outline.GKO',
+    'eagle_files/silkscreen_top.gbr',
+    ]
+
 
 @filter_syntax_warnings
 @pytest.mark.parametrize('reference', REFERENCE_FILES, indirect=True)
@@ -310,7 +320,7 @@ def test_svg_export(reference, tmpfile):
 
     out_svg = tmpfile('Output', '.svg')
     with open(out_svg, 'w') as f:
-        f.write(str(grb.to_svg(force_bounds=bounds, arg_unit='inch')))
+        f.write(str(grb.to_svg(force_bounds=bounds, arg_unit='inch', color='white')))
 
     ref_png = tmpfile('Reference render', '.png')
     gerbv_export(reference, ref_png, origin=bounds[0], size=bounds[1], format='png', fg='#000000')
@@ -323,3 +333,46 @@ def test_svg_export(reference, tmpfile):
     assert hist[3:].sum() < 1e-3*hist.size
 
 # FIXME test svg margin, bounding box computation
+
+@filter_syntax_warnings
+@pytest.mark.parametrize('reference', REFERENCE_FILES, indirect=True)
+def test_bounding_box(reference, tmpfile):
+    # skip this check on files that contain lines with a zero-size aperture at the board edge
+    if any(reference.match(f'*/{f}') for f in HAS_ZERO_SIZE_APERTURES):
+        pytest.skip()
+
+    # skip this file because it does not contain any graphical objects
+    if reference.match('*/multiline_read.ger'):
+        pytest.skip()
+
+    margin = 1.0 # inch
+    dpi = 200
+    margin_px = int(dpi*margin) # intentionally round down to avoid aliasing artifacts
+
+    grb = GerberFile.open(reference)
+    out_svg = tmpfile('Output', '.svg')
+    with open(out_svg, 'w') as f:
+        f.write(str(grb.to_svg(margin=margin, arg_unit='inch', color='white')))
+
+    out_png = tmpfile('Render', '.png')
+    svg_to_png(out_svg, out_png, dpi=dpi)
+
+    img = np.array(Image.open(out_png))
+    img = img[:, :, :3].mean(axis=2) # drop alpha and convert to grayscale
+    img = np.round(img).astype(int) # convert to int
+    assert (img > 0).any() # there must be some content, none of the test gerbers are completely empty.
+    cols = img.sum(axis=1)
+    rows = img.sum(axis=0)
+    print('shape:', img.shape)
+    col_prefix, col_suffix = np.argmax(cols > 0), np.argmax(cols[::-1] > 0)
+    row_prefix, row_suffix = np.argmax(rows > 0), np.argmax(rows[::-1] > 0)
+    print('cols', 'prefix:', row_prefix, 'suffix:', row_suffix)
+    print('rows', 'prefix:', row_prefix, 'suffix:', row_suffix)
+
+    # Check that all margins are completely black and that the content touches the margins. Allow for some tolerance to
+    # allow for antialiasing artifacts.
+    assert margin_px-1 <= col_prefix <= margin_px+1
+    assert margin_px-1 <= col_suffix <= margin_px+1
+    assert margin_px-1 <= row_prefix <= margin_px+1
+    assert margin_px-1 <= row_suffix <= margin_px+1
+