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

Face edges not always present in triangulation #430

Closed
hannobraun opened this issue Apr 5, 2022 · 6 comments · Fixed by #1369
Closed

Face edges not always present in triangulation #430

hannobraun opened this issue Apr 5, 2022 · 6 comments · Fixed by #1369
Labels
topic: core Issues relating to core geometry, operations, algorithms type: bug Something isn't working

Comments

@hannobraun
Copy link
Owner

It's possible that edges of a face don't show up in the triangulation, which is obviously an invalid result. It's relatively easy to come up with an example, once you know what to look for. Check out this quick sketch I did:

triangulation

There's a face on the left, and a possible Delaunay triangulation of that face on the right. The long vertical edge that I specially marked is not present in the triangulation.

Thanks to @alexoro412, who reported this issue in #105!

This should be pretty easy to fix, hopefully. Spade can do constrained Delaunay triangulations, which should take care of this issue.

Blocked on #105, since I believe we should have a test suite in place before fixing more triangulation issues.

@hannobraun hannobraun added type: bug Something isn't working topic: core Issues relating to core geometry, operations, algorithms labels Apr 5, 2022
@hannobraun
Copy link
Owner Author

#105 has been addressed. This issue is no longer blocked.

I forgot to label this as https://github.com/hannobraun/Fornjot/labels/status%3A%20blocked in the first place.

@hannobraun
Copy link
Owner Author

I suspect that the missing triangles here are an instance of this bug:
non-symmetric-sweep-new

This is basically the normal star model, except that the let radius = ... line has been replaced with this:

let mut radius = if i % 2 == 0 { r1 } else { r2 };
radius *= (i + 1) as f64 / num_vertices as f64;

@willhansen
Copy link
Contributor

Here's a more minimal example:

#[fj::model]
pub fn model(
) -> fj::Shape {
    let good_x = 0.5;
    let bad_x = 0.4;
    let x = bad_x;
    let mut other = Vec::new();
        other.push([0., 0.]);
        other.push([x, 0.]);
        other.push([0.4, 1.0]);
        other.push([0.1, 0.1]);
        other.push([0., 0.8]);

    let other = fj::Sketch::from_points(other);


    other.into()
}

Selection_043
Selection_044

@hannobraun
Copy link
Owner Author

Thank you, @willhansen!

@willhansen
Copy link
Contributor

Converted example to failing unit test:
main...willhansen:Fornjot:bugfix/#430-triangulation-missing-edges

@hannobraun
Copy link
Owner Author

That's great, @willhansen, thank you!

Would you mind slapping an #[ignore] on the test and submitting that in a pull request? That way, the test is available to be used right where it will be needed, without making the CI build fail.

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.

2 participants