fix bug with flipEdgeNetwork created by piecewise Dijkstra#237
Open
Joel-Hecht wants to merge 2 commits intonmwsharp:masterfrom
Open
fix bug with flipEdgeNetwork created by piecewise Dijkstra#237Joel-Hecht wants to merge 2 commits intonmwsharp:masterfrom
Joel-Hecht wants to merge 2 commits intonmwsharp:masterfrom
Conversation
Joel-Hecht
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.Suggestion cannot be applied right now. Please check back later.
Fixes #209
For some vertices A,B,C using shortestEdgePath() to connect vertex A to vertex B, and vertex B to vertex C would result in paths AB and BC that possibly contain one or more duplicate edges (i.e. one or more edges in AB has its twin in BC). The flipout algorithm is not able to flip these 'back-and-forth' loops (or the edges immediately before/after them), leading to incorrect shortening of the input path.
PR adds an option (enabled as default) in constructFromPiecewiseDijkstraPath() to remove any subset of the resulting curve that starts and ends at the same vertex, which, for all halfedges in the subset, also contains their twins.
Only fixes the 'artifacts' problem in the linked issue. Other 'problems' mentioned in the linked issue are fundamental misunderstandings of the algorithm.
Also adds a few contrived tests illustrating cases that would fail before, but now have the expected behavior. These tests are not very interesting and can probably get deleted.
Possibly also fixes this issue noted in comment in wedgeIsClear() (?):
TODO handle checks in case where the path lollipops out and back along a single edge