Loading moderate sized gerber file is too slow.

[phdussud]

Even moderate size gerber files containing just a few thousand polygons are too slow to load(more than 90 seconds on a Core I7 processor). The culprit is the call to cascaded_union in Gerber.parslines (line 1967). I am not sure why this call is necessary. Why would creating a MultiPolygon out of the parsed polygons not good enough? If it isn't good enough there is a very close approximation of the cascaded union that, in my opinion, should be good enough: Create the MultiPolygon out of the parsed polygons, Then applying buffer (delta) then buffer (-delta) where delta is very small like 0.00000001 It turns out that buffer is way way faster than the union and it does the same thing to a close approximation. delta needs to be small enough so we don't lose any feature but on the other hand the features have practical lower limits in order to be rendered chemically or mechanically. Making this change result in about 90 sec improvement on a moderate size gerber file.

[Juan Pablo Caram]

If you don't create a union, then you put a bigger load on the rendering (or any processing happening after load). Having a time penalty on the beginning, a single time, is preferable over multiple times during the user interaction.

I didn't know a buffer operation over a MultiPolygon would join the polygons. Sounds like a hack to rely on such behavior, unless in computational geometry that behavior is agreed upon.

Nonetheless, this sounds really attractive from the performance point of view. Let's go for it, but I would make it an option. It can be the default, but allowing the user to revert to the old behavior in case it has some unintended effects.

[phdussud]

do you want to make it a visible default or one of them to set with the TCL command line like excellon_zeros ?

[Juan Pablo Caram]

Your choice. The invisible option is less work.

How likely do you think this way of joining polygons could create undesired behavior? I it's like 1% then I'd say make it invisible. If it's going to be like 10%, then we definitely want to make it really easy/visible to change.

[phdussud]

I believe that the definition of buffer implies that the polygons are joined. However the MultiPolygon that I form can be non valid since the contained polygons could share an infinite number of points. That being said, I looked at their algorithm and they don't seem to care about this property. I tested specially constructed cases separately from flatcam to verify the behavior and I am fairly certain that it is correct. I wish they didn't hack the implementation to special case buffer(0) which does something totally different than buffer(delta). That's why I have to use a very small delta. I think the invisible option is the way to go. Mostly because this is a non obvious choice to make and it would clutter the UI.

[Juan Pablo Caram]

I agree. Go for it!

[phdussud]

By the way if we find that it has some problems, I would propose to just turn it off completely as you would never be sure if you can trust it blindly. One of the workflows I care a lot about is a completely automated way to go from gerber to cnc without interaction (like pcb-gcode for eagle).

[Juan Pablo Caram]

What about writing some tests: Give different gerbers to both methods and compare the difference in area or rms-value of the error between vertices?

[phdussud]

That's a good idea. I think it is easy to compare the areas. Not no simple to compare vertices. I believe that the buffer call produces some very sharp rounded corners corresponding to a real corner in the case of the union.

[Juan Pablo Caram]

Buffering should add points, so it would be hard to know which ones to compare. I agree. Area should be much easier.

At this point any test would be good. This project has no tests at all, which makes development difficult. So If you could write a simple gerber import integrity test, just to get us started, it would be really useful to make sure that we don't mess up things every time we make a change to the parser.

[phdussud]

I will add a test. Probably early next week. I haven't looked at your test infrastructure yet but will ramp up.

[Juan Pablo Caram]

There is not much of an infrastructure. You are welcome to start one.

If you look in the tests/ folder, test_excellon.py and test_pathconnect.py are proper unit tests.

[phdussud]

Ok. I have a test. Please review it in the pull request I just submitted. Thanks, Patrick.

[Juan Pablo Caram]

Resolve issue 144

Added a test for the new mechanism Added a gerber test file

→ <<cset 0bf21458c620>>

[Juan Pablo Caram]

I'm getting

[WARNING][MainThread] Tried to set an option or field that does not exist: use_buffer_for_union

and

[WARNING][MainThread] Failed to read option from field: use_buffer_for_union

when opening a Gerber. I suspect this is normal but I'm not sure at this time.