Calculate out all aperture macros by default.

There are just too many severely buggy implementations around. Today I
ran into problems with both gerbv and with whatever JLC uses. You can
still export macros with raw expressions by setting a flag in the export
FileSettings.

6 files changed, 65 insertions, 75 deletions

diff --git a/gerbonara/aperture_macros/expression.py b/gerbonara/aperture_macros/expression.py
index c9c0470..7c291cb 100644
--- a/gerbonara/aperture_macros/expression.py
+++ b/gerbonara/aperture_macros/expression.py
@@ -31,9 +31,6 @@ class Expression:
     def converted(self, unit):
         return self
 
-    def replace_mixed_subexpressions(self, unit):
-        return self
-
     def calculate(self, variable_binding={}, unit=None):
         expr = self.converted(unit).optimized(variable_binding)
         if not isinstance(expr, ConstantExpression):
@@ -104,9 +101,6 @@ class UnitExpression(Expression):
     def __repr__(self):
         return f'<UE {self.expr.to_gerber()} {self.unit}>'
 
-    def replace_mixed_subexpressions(self, unit):
-        return self.converted(unit).replace_mixed_subexpressions(unit)
-
     def converted(self, unit):
         if self.unit is None or unit is None or self.unit == unit:
             return self.expr
@@ -198,9 +192,6 @@ class VariableExpression(Expression):
     def __eq__(self, other):
         return type(self) == type(other) and self.expr == other.expr
 
-    def replace_mixed_subexpressions(self, unit):
-        return VariableExpression(self.expr.replace_mixed_subexpressions(unit))
-
     def to_gerber(self, register_variable=None, unit=None):
         if register_variable is None:
             return self.expr.to_gerber(None, unit)
@@ -356,17 +347,6 @@ class OperatorExpression(Expression):
 
         return expr(rv).optimized(variable_binding)
 
-    def replace_mixed_subexpressions(self, unit):
-        l = self.l.replace_mixed_subexpressions(unit)
-        if l._operator not in (None, self.op):
-            l = VariableExpression(self.l)
-
-        r = self.r.replace_mixed_subexpressions(unit)
-        if r._operator not in (None, self.op):
-            r = VariableExpression(self.r)
-
-        return OperatorExpression(self.op, l, r)
-        
     def to_gerber(self, register_variable=None, unit=None):
         lval = self.l.to_gerber(register_variable, unit)
         rval = self.r.to_gerber(register_variable, unit)
diff --git a/gerbonara/aperture_macros/parse.py b/gerbonara/aperture_macros/parse.py
index fb4f0fe..51c1f27 100644
--- a/gerbonara/aperture_macros/parse.py
+++ b/gerbonara/aperture_macros/parse.py
@@ -118,41 +118,32 @@ class ApertureMacro:
                 pass
         return replace(self, primitives=tuple(new_primitives))
 
+    def substitute_params(self, params, unit=None, macro_name=None):
+        params = dict(enumerate(params, start=1))
+        return replace(self,
+                       num_parameters=0,
+                       name=macro_name,
+                       primitives=tuple(p.substitute_params(params, unit) for p in self.primitives),
+                       comments=(f'Fully substituted instance of {self.name} macro',
+                                 f'Original parameters {"X".join(map(str, params.values()))}'))
+
     def to_gerber(self, settings):
         """ Serialize this macro's content (without the name) into Gerber using the given file unit """
         comments = [ f'0 {c.replace("*", "_").replace("%", "_")}' for c in self.comments ]
 
         subexpression_variables = {}
         def register_variable(expr):
-            if not settings.allow_mixed_operators_in_aperture_macros:
-                expr = expr.replace_mixed_subexpressions(unit=settings.unit)
-
             expr_str = expr.to_gerber(register_variable, settings.unit)
             if expr_str not in subexpression_variables:
                 subexpression_variables[expr_str] = self.num_parameters + 1 + len(subexpression_variables)
-
             return subexpression_variables[expr_str]
 
-        primitive_defs = []
-        for prim in self.primitives:
-            if not settings.allow_mixed_operators_in_aperture_macros:
-                prim = prim.replace_mixed_subexpressions(unit=settings.unit)
-
-            primitive_defs.append(prim.to_gerber(register_variable, settings))
-
-        variable_defs = []
-        for expr_str, num in subexpression_variables.items():
-            variable_defs.append(f'${num}={expr_str}')
-
+        primitive_defs = [prim.to_gerber(register_variable, settings) for prim in self.primitives]
+        variable_defs = [f'${num}={expr_str}' for expr_str, num in subexpression_variables.items()]
         return '*\n'.join(comments + variable_defs + primitive_defs)
 
     def to_graphic_primitives(self, offset, rotation, parameters : [float], unit=None, polarity_dark=True):
-        variables = {i: v for i, v in enumerate(self.variables, start=1) if v is not None}
-        for number, value in enumerate(parameters, start=1):
-            if number in variables:
-                raise SyntaxError(f'Re-definition of aperture macro variable {number} through parameter {value}')
-            variables[number] = value
-
+        parameters = dict(enumerate(parameters, start=1))
         for primitive in self.primitives:
             yield from primitive.to_graphic_primitives(offset, rotation, variables, unit, polarity_dark)
 
diff --git a/gerbonara/aperture_macros/primitive.py b/gerbonara/aperture_macros/primitive.py
index 1738ff7..dce5677 100644
--- a/gerbonara/aperture_macros/primitive.py
+++ b/gerbonara/aperture_macros/primitive.py
@@ -51,14 +51,10 @@ class Primitive:
                 getattr(self, field.name).optimized().to_gerber(register_variable, settings.unit)
                 for field in fields(self) if issubclass(field.type, Expression))
     
-    def replace_mixed_subexpressions(self, unit):
-        print('prim rms')
-        import pprint
-        out = replace(self, **{
-                       field.name: getattr(self, field.name).optimized().replace_mixed_subexpressions(unit)
+    def substitute_params(self, binding, unit):
+        out = replace(self, unit=unit, **{
+                       field.name: getattr(self, field.name).calculate(binding, unit)
                        for field in fields(self) if issubclass(field.type, Expression)})
-        pprint.pprint(self)
-        pprint.pprint(out)
         return out
 
     def __str__(self):
@@ -111,6 +107,12 @@ class Circle(Primitive):
             x, y = x+offset[0], y+offset[1]
             return [ gp.Circle(x, y, calc.diameter/2, polarity_dark=(bool(calc.exposure) == polarity_dark)) ]
 
+    def substitute_params(self, binding, unit):
+        with self.Calculator(self, binding, unit) as calc:
+            x, y = rotate_point(calc.x, calc.y, -deg_to_rad(calc.rotation), 0, 0)
+            new =  Circle(unit, self.exposure, calc.diameter, x, y)
+            return new
+
     def dilated(self, offset, unit):
         return replace(self, diameter=self.diameter + UnitExpression(offset, unit))
 
@@ -144,6 +146,12 @@ class VectorLine(Primitive):
             return [ gp.Rectangle(center_x, center_y, length, calc.width, rotation=rotation,
                         polarity_dark=(bool(calc.exposure) == polarity_dark)) ]
 
+    def substitute_params(self, binding, unit):
+        with self.Calculator(self, binding, unit) as calc:
+            x1, y1 = rotate_point(calc.start_x, calc.start_y, -deg_to_rad(calc.rotation), 0, 0)
+            x2, y2 = rotate_point(calc.end_x, calc.end_y, -deg_to_rad(calc.rotation), 0, 0)
+            return VectorLine(unit, calc.exposure, calc.width, x1, y1, x2, y2)
+
     def dilated(self, offset, unit):
         return replace(self, width=self.width + UnitExpression(2*offset, unit))
 
@@ -174,6 +182,12 @@ class CenterLine(Primitive):
 
             return [ gp.Rectangle(x, y, w, h, rotation, polarity_dark=(bool(calc.exposure) == polarity_dark)) ]
 
+    def substitute_params(self, binding, unit):
+        with self.Calculator(self, binding, unit) as calc:
+            x1, y1 = rotate_point(calc.x, calc.y-calc.height/2, -deg_to_rad(calc.rotation), 0, 0)
+            x2, y2 = rotate_point(calc.x, calc.y+calc.height/2, -deg_to_rad(calc.rotation), 0, 0)
+            return VectorLine(unit, calc.exposure, calc.width, x1, y1, x2, y2)
+
     def dilated(self, offset, unit):
         return replace(self, width=self.width + UnitExpression(2*offset, unit))
 
@@ -293,26 +307,17 @@ class Outline(Primitive):
         return f'<Outline {len(self.coords)} points>'
 
     def to_gerber(self, register_variable=None, settings=None):
-        # Calculate out rotation since at least gerbv mis-renders Outlines with rotation other than zero.
         rotation = self.rotation.optimized()
-        coords = self.coords
-        if isinstance(rotation, ConstantExpression) and rotation != 0:
-            rotation = math.radians(rotation.value)
-            # This will work even with variables in x and y, we just need to pass in cx and cy as UnitExpressions
-            unit_zero = UnitExpression(expr(0), MM)
-            coords = [ rotate_point(x, y, -rotation, cx=unit_zero, cy=unit_zero) for x, y in self.points ]
-            coords = [ e for point in coords for e in point ]
-            if not settings.allow_mixed_operators_in_aperture_macros:
-                coords = [e.replace_mixed_subexpressions(unit=settings.unit) for e in coords]
-
-            rotation = ConstantExpression(0)
-
-        coords = ','.join(coord.optimized().to_gerber(register_variable, settings.unit) for coord in coords)
+        coords = ','.join(coord.optimized().to_gerber(register_variable, settings.unit) for coord in self.coords)
         return f'{self.code},{self.exposure.optimized().to_gerber(register_variable)},{len(self.coords)//2-1},{coords},{rotation.to_gerber(register_variable)}'
 
-    def replace_mixed_subexpressions(self, unit):
-        return replace(Primitive.replace_mixed_subexpressions(self, unit),
-                       coords=[e.replace_mixed_subexpressions(unit) for e in self.coords])
+    def substitute_params(self, binding, unit):
+        with self.Calculator(self, binding, unit) as calc:
+            rotation = calc.rotation
+            coords = [ rotate_point(x.calculate(binding, unit), y.calculate(binding, unit), -deg_to_rad(rotation), 0, 0)
+                      for x, y in self.points ]
+            coords = [ e for point in coords for e in point ]
+            return Outline(unit, calc.exposure, calc.length, coords)
 
     def parameters(self):
         yield from Primitive.parameters(self)
diff --git a/gerbonara/apertures.py b/gerbonara/apertures.py
index 09b15d2..f057052 100644
--- a/gerbonara/apertures.py
+++ b/gerbonara/apertures.py
@@ -446,6 +446,11 @@ class ApertureMacroInstance(Aperture):
     def scaled(self, scale):
         return replace(self, macro=self.macro.scaled(scale))
 
+    def calculate_out(self, unit=None, macro_name=None):
+        return replace(self,
+                       parameters=tuple(),
+                       macro=self.macro.substitute_params(self._params(unit), unit, macro_name))
+
     def _params(self, unit=None):
         # We ignore "unit" here as we convert the actual macro, not this instantiation.
         # We do this because here we do not have information about which parameter has which physical units.
@@ -455,4 +460,3 @@ class ApertureMacroInstance(Aperture):
             parameters = parameters[:self.macro.num_parameters]
         return tuple(parameters)
 
-
diff --git a/gerbonara/cam.py b/gerbonara/cam.py
index 8389425..e6bb651 100644
--- a/gerbonara/cam.py
+++ b/gerbonara/cam.py
@@ -56,11 +56,10 @@ class FileSettings:
     number_format : tuple = (None, None)
     #: At least the aperture macro implementations of gerbv and whatever JLCPCB uses are severely broken and simply
     #: ignore parentheses in numeric expressions without throwing an error or a warning, leading to broken rendering.
-    #: To avoid trouble with severely broken software like this, we split out any non-trivial numeric sub-expressions
-    #: into separate internal macro variables by default.
+    #: To avoid trouble with severely broken software like this, we just calculate out all macros by default.
     #: If you want to export the macros with their original formulaic expressions (which is completely fine by the
-    #: Gerber standard, btw), set this parameter to ``True`` before exporting.
-    allow_mixed_operators_in_aperture_macros: bool = False
+    #: Gerber standard, btw), set this parameter to ``False`` before exporting.
+    calculate_out_all_aperture_macros: bool = True
 
     # input validation
     def __setattr__(self, name, value):
diff --git a/gerbonara/rs274x.py b/gerbonara/rs274x.py
index ad78ceb..d5cbb34 100644
--- a/gerbonara/rs274x.py
+++ b/gerbonara/rs274x.py
@@ -284,12 +284,23 @@ class GerberFile(CamFile):
         self.dedup_apertures()
 
         am_stmt = lambda macro: f'%AM{macro.name}*\n{macro.to_gerber(settings)}*\n%'
-        for macro in self.aperture_macros():
-            yield am_stmt(macro)
-
         aperture_map = {ap: num for num, ap in enumerate(self.apertures(), start=10)}
-        for aperture, number in aperture_map.items():
-            yield f'%ADD{number}{aperture.to_gerber(settings)}*%'
+
+        if settings.calculate_out_all_aperture_macros:
+            adds = []
+            for aperture, number in aperture_map.items():
+                if isinstance(aperture, apertures.ApertureMacroInstance):
+                    aperture = aperture.calculate_out(settings.unit, macro_name=f'CALCM{number}')
+                    yield am_stmt(aperture.macro)
+                adds.append(f'%ADD{number}{aperture.to_gerber(settings)}*%')
+            yield from adds
+
+        else:
+            for macro in self.aperture_macros():
+                yield am_stmt(macro)
+
+            for aperture, number in aperture_map.items():
+                yield f'%ADD{number}{aperture.to_gerber(settings)}*%'
 
         def warn(msg, kls=SyntaxWarning):
             warnings.warn(msg, kls)