Fix possibly disappearing airwires #687
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dbrgn
reviewed
Apr 19, 2020
ubruhin
added a commit
that referenced
this issue
Apr 22, 2020
Fix possibly disappearing airwires (cherry picked from commit 65a1e73)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
It's actually not a fix, but a workaround for issue #588. The Delaunay triangulation library is not able to handle colinear points in the input data (our net points) and silently ignores some of the input points, so there are edges (our airwires) missing in the output.
I spend quite some time on this issue, and I think it's not possible to actually fix this problem on our side, so the fix needs to be done in delaunay-triangulation. But actually I'm not sure if it's worth investigating and fixing the issue in that library - in my opinion the overall quality of that library is not acceptable for an EDA tool anyway. For long-term, we might switch to a different library, for example delaunator-cpp which seems to be developed more seriously. But they currently also have a similar issue (abellgithub/delaunator-cpp#8), so we should wait until this issue is resolved.
However, to get LibrePCB 0.1.4 released as soon as possible with disappearing airwires fixed, I simply implemented some kind of "fallback". Even if delaunay-triangulation failed to add some edges, we now still get all edges connected, but connections might not be "optimal", i.e. not the shortest distance is shown. But at least no airwires disappear now, so the risk of producing PCBs with missing connections is gone.
I also refactored the airwires builder to make it easier testable, and added some unittests which are able to detect the current bug.