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.

Sign up for GitHub

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

Jump to bottom

Remove Edge::reverse #151

Closed
hannobraun opened this issue Feb 9, 2022 · 0 comments · Fixed by #244
Closed

Remove Edge::reverse #151

hannobraun opened this issue Feb 9, 2022 · 0 comments · Fixed by #244
Labels
topic: core Issues relating to core geometry, operations, algorithms type: development Work to ease development or maintenance, without direct effect on features or bugs

Comments

@hannobraun
Copy link
Owner

Edge has a field (and method) that can be used to reverse the direction of the edge. This shouldn't be necessary. Triangulation doesn't care about edge direction (any more), and any code relying on it is probably affected by loads of edge cases. The field (and method) should just be removed.

Unfortunately, the sweep operation relies on it for building the side walls. Not quite coincidentally, this is also the last piece of code that generates a triangle mesh as an intermediate representation (see #97).

This issue is a reminder to remove this field, once possible. It is blocked on #97.
@hannobraun hannobraun added status: blocked Issue or pull request is blocked by another issue or pull request, or some outside circumstance type: development Work to ease development or maintenance, without direct effect on features or bugs topic: core Issues relating to core geometry, operations, algorithms labels Feb 9, 2022
@hannobraun hannobraun mentioned this issue Feb 11, 2022
Closed
hannobraun added a commit that referenced this issue Feb 23, 2022
@hannobraun
Remove Edge::reverse
892e6ac 
This simplifies some code, without any loss in functionality, as far as
I can tell.

Close #151

The issue this commit addresses is nominally blocked. However, given
that #173 exists, this mechanism doesn't have its desired effect anyway.

A proper fix will likely involve a deeper look at surface orientation,
and I don't think this hack will be part of that. Given that, and that
this removal simplifies some ongoing work, I feel good about doing this
now.
@hannobraun hannobraun mentioned this issue Feb 23, 2022
Merged
@hannobraun hannobraun removed the status: blocked Issue or pull request is blocked by another issue or pull request, or some outside circumstance label Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: core Issues relating to core geometry, operations, algorithms type: development Work to ease development or maintenance, without direct effect on features or bugs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove Edge::reverse hannobraun/fornjot
1 participant
@hannobraun