[PyGerber]: Macro elements come out upside down in SVG render

[Trackmaniadude]

PyGerber Bug Report

Mandatory checks

Before opening this issue:

• I have reviewed the README for guidelines and haven't found a solution there.
• I have reviewed the PyGerber documentation and haven't found a solution there.
• I have reviewed the existing open issues and verified that this is not a
  duplicate.
• I have reviewed the existing closed issues and verified that this was already
  resolved or marked as won't fix.
• I have reviewed the existing pull requests and verified that this is not a already
  known issue.
• I have reviewed the existing discussions and verified that this is not a already
  known issue.

#242

To Reproduce

Run below code with errtest-F_Paste.txt (note, wouldn't let me upload .gbr so I changed it to a .txt, just change it back)

from pygerber.gerberx3.api.v2 import GerberFile
GerberFile.from_file('errtest-F_Paste.gbr').parse().render_svg("output.svg")

Should result in

(Note: rounded rectangle macros generated by KiCad, not normal rectangles)

Expected behavior

Image should be this (from KiCad gerber viewer)

Also, output from raster renderer

Additional context

Initially thought the issue was inverted rotation, like with the rectangle issue, however it became clear its y axis inversion when looking at asymmetric elements (unfortunately I couldn't figure out how to make an original test file with such elements). I poked around trying to fix it myself for a bit, and I think the issue is somewhere around render_flash_macro in renderer2/svg.py. Changing flip_y=False (668 svg.py) to True sort of fixed the issue, but affected elements were misplaced vertically.

Environment:

Please complete the following information:

• Operating system: Windows 10
• Python version: 3.11.0
• PyGerber version: 2.4.2

Optional checks

• I want to contribute example source files attached to this issue in the test suite
  of PyGerber for regression testing purposes.
• I want to include separate LICENSE file for resource files attached as a result of
  an agreement described in first checkbox in this section.
• I want to include separate README file for resource files attached as a result of
  an agreement described in first checkbox in this section.

[Argmaster]

Hi

Thanks for reporting the issue.
Could you please chceck if you can reproduce the issue on 3.0.0a4?
Due to limitations in version 2.4.x it may not be possible to fix the issue there.

[Trackmaniadude]

The example gerber renders correctly with 3.0.0a4.

[Argmaster]

Thanks for checking.
Currently I would prefer to focus on getting 3.0.0 ready for official release instead of attempting to fix 2.4 which is fundamentally flawed, especially since that particular macro rotation case is covered in detail in test suite for 3.0.0.
If you want, you can try finding a way to fix it for 2.4. I think the cause might be that rotation direction for SVG elements is inverted (clockwise vs counterclockwise?) in some combination of parameters (maybe due to Y axis inversion?), maybe there is a way to determine when rotation angle should be inverted (abs(360 - angle)?). Let me know if you are interested in poking around with that issue.

[Trackmaniadude]

Found that setting the flip_y argument in SVGRenderer2Hooks init() to False causes it to render correctly, albeit the entire image is upside-down.
Removing the flip_y handling in the svg renderer's _convert_y() function also does the same.
Couldn't figure out how to undo said flip internally though (or if there are extra consequences to the change), but works for my purposes.

[Argmaster]

The image flip is caused by Y axis in SVG having opposite direction than in Gerber format, flip_y was introduced to counteract this problem.

[Trackmaniadude]

Ok so I think what's going on is that flipping isn't inherited for children of macro elements. That's why setting flip to true when the macro function makes a new frame partially fixes the issue. As for the offset issue afterwards I found it was always the height of the element's bounding box lower than it should be, so I changed it to not do that offset in macro frames, and that made it work.

[Argmaster]

Ok so I think what's going on is that flipping isn't inherited for children of macro elements. That's why setting flip to true when the macro function makes a new frame partially fixes the issue. As for the offset issue afterwards I found it was always the height of the element's bounding box lower than it should be, so I changed it to not do that offset in macro frames, and that made it work.

I appreciate your findings, could you submit a pull request so we can verify this fix against test suite?