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

Sweep algorithm assumes it's sweeping objects in x-y plane #990

Closed
hannobraun opened this issue Aug 24, 2022 · 0 comments · Fixed by #1063
Closed

Sweep algorithm assumes it's sweeping objects in x-y plane #990

hannobraun opened this issue Aug 24, 2022 · 0 comments · Fixed by #1063
Labels
topic: core Issues relating to core geometry, operations, algorithms type: bug Something isn't working

Comments

@hannobraun
Copy link
Owner

The sweep algorithm makes an assumptions about the objects being swept that can't generally be relied upon, namely that objects being swept are located in the x-y plane.

This works out from an end user perspective, because the end user can only sweep sketches, and those are always defined in the x-y-plane. However, this is subject to change going forward, which would then break the sweep operation. Within the kernel, it is already possible to freely define the orientation of any objects being swept, so the algorithm is already broken for users of the kernel API.

The assumption is encoded in this method:
https://github.com/hannobraun/Fornjot/blob/fa0e5d73448d6c9fb2981cf1c68de934b052f101/crates/fj-kernel/src/algorithms/sweep/mod.rs#L36-L39

It only looks at the z-coordinate of the sweep path, but needs to look at the sweep path in relation to what is being swept.

For now, this works for end users. For kernel API users, the workaround is to always use the x-y plane as the surface for anything that is being swept.

@hannobraun hannobraun added type: bug Something isn't working topic: core Issues relating to core geometry, operations, algorithms labels Aug 24, 2022
hannobraun added a commit that referenced this issue Sep 8, 2022
From the perspective of the edge sweep operation, there is no such thing
as a negative direction. This is only relevant to the face sweep, so it
makes more sense to do it there.

And in fact, the edge sweep operation can't even decide whether it
*should* reverse. The way the decision is made right now is incorrect:
#990
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Issues relating to core geometry, operations, algorithms type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant