yaqwsx
commented
4 years ago I am aware of the issue with the resulting image size, however, I am quite skeptical about accepting the PR in the current form.
In general, I like the idea of -e switch and I would like to accept it as it makes sense for many cases (e.g., drawing only bare PCB without components). I dislike the implementation via a global variable. I know PcbDraw is a rather simple script, but this is the first step to make it a spaghetti mess. I consider the proposed implementation rather hacky. Instead, get_hole_mask and get_layers should take another parameter which will be the desired area of plotting (which is currently the board bounding box) instead of computing it inside the function. get_board_substrate can also receive the parameter. This implementation allows us to easily implement e.g., --viewbox switch for rendering only a portion of the board.
I dislike the naming of the --remove-hidden switch. There is no concept of "hidden" layers in PcbDraw and also it is confusing "Why to remove something hidden?". Also, the current implementation is incompatible with the --vcuts switch. Also, if we properly implement #41, there is no need for such a weird switch. This switch also has a problem with components sticking out of the board - they are usually drawn on these layers, thus, the components will be cropped. Event letting the user which layers to plot make no sense in the context of #41.
I propose to remove --remove-hidden from this PR and refactor the implementation of -e. Then I will be happy to accept this PR.
The second is perfect for the board without components, but coul crop components that extend outside the PCB border.
They should help with #41 BTW, I think -r should be enabled by default.