-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Missing spaces: Implemented traverse algorithm that is based on edges rather than nodes #995
Missing spaces: Implemented traverse algorithm that is based on edges rather than nodes #995
Conversation
Added a test to check if FindAllClosedRegions basic functionality works correctly.
Added a test - a room with two inner leaves. FindAllClosedRegions will find only a single clockwise region. Such regions are filtered out during the rooms search, so such room will be missed.
- Switched to traversing through edges during closed regions search. - Fixed TwoLeavesClosedRegions test. - Fixed several other tests that extracted an additional clockwise cycle. - EShapeNetworkClosedRegions test is broken, but it will be fixed later.
No logic changes
- Filtered out "closed regions" that were created from path by edges backtracking. - EShapeNetworkClosedRegions test is fixed by these changes.
Removed codirectional edges from adjacency matrix. Sometimes several edges with the same start have the same direction. In such cases the shortest edge stays, and other edges are replaced with smaller edges between consecutive points. For example: A ----- B ----- C ----- D There are edges AB, AC, AD. They will be replaced with AB, BC and CD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! thanks for taking on this work, sorry it took me a while to review!
Reviewable status: complete! 1 of 1 approvals obtained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 2 of 1 approvals obtained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this changes on Drywall function. Everything works great
Reviewable status: complete! 2 of 1 approvals obtained
BACKGROUND:
DESCRIPTION:
TESTING:
FUTURE WORK:
This change is