Skip to content

Add option to apply mirroring to the Y axis#572

Merged
eyal0 merged 6 commits intopcb2gcode:masterfrom
vicencb:master
Apr 9, 2021
Merged

Add option to apply mirroring to the Y axis#572
eyal0 merged 6 commits intopcb2gcode:masterfrom
vicencb:master

Conversation

@vicencb
Copy link
Copy Markdown
Contributor

@vicencb vicencb commented Apr 8, 2021

Hi,
this change adds the option to appy the mirroring to the Y axis instead of the X axis.

Regards,
Vicente.

@eyal0
Copy link
Copy Markdown
Contributor

eyal0 commented Apr 8, 2021

Excellent work. I'll add some integration tests so that we can look visually to confirm that everything is working as expected.

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Apr 8, 2021

Coverage Status

Coverage increased (+0.02%) to 69.244% when pulling 54fba8a on vicencb:master into e7c1175 on pcb2gcode:master.

@eyal0
Copy link
Copy Markdown
Contributor

eyal0 commented Apr 9, 2021

I see a little bit of confusion around positive and negative. In KiCad, positive x is to the right and positive y is downward. But in gerbv, positive x is to the right and positive y is upward.

Since we're using gerbv as a library, I think that we'll do it the gerbv way. Also it makes the code make more sense so that the handling of x and the handling of y look similar.

Here's my test image:

image

If I set the axis to be y and the mirror-axis to be -0.5inches, those little marks on the right side line up properly, so that seems great!

I also checked that it works with the tiling features enabled. Great!

The tests should improve coverage.

@eyal0 eyal0 merged commit c2890c8 into pcb2gcode:master Apr 9, 2021
@vicencb
Copy link
Copy Markdown
Contributor Author

vicencb commented Apr 9, 2021

Hi Eyal,
thank you for adding tests, improving coverage and merging it.
Are you able to merge this one too ?

Regards,
Vicente.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants