Skip to content
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

Rewrite edge sweeping algorithm #1033

Merged
merged 6 commits into from
Sep 2, 2022
Merged

Rewrite edge sweeping algorithm #1033

merged 6 commits into from
Sep 2, 2022

Conversation

hannobraun
Copy link
Owner

The old edge sweeping code did some dodgy things that could easily lead to bugs (case in point: #1031 (comment)). It also made assumptions that were incompatible with #1020. This rewritten code fixes all that, at the cost of some verbosity (and a disabled test).

This isn't exactly the end of the journey, but I think it leaves us in a better position in regards to future changes.

This doesn't make a difference right now, but makes a following change
smaller.
This is another small change that serves no purpose, except as to make a
following change smaller.
This test is going to be difficult to keep working with the rewritten
edge sweep operation that I'm working on, just like the other test that
is already disabled became diffult before.

I hope that I can re-enable both, once curves are made irreversible.
@hannobraun hannobraun enabled auto-merge September 2, 2022 14:36
@hannobraun hannobraun merged commit 01bbb95 into main Sep 2, 2022
@hannobraun hannobraun deleted the sweep branch September 2, 2022 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant