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

internal assertion failing when inserting contrained edge with Spade 2.11 #111

Closed
mockersf opened this issue Aug 4, 2024 · 2 comments · Fixed by #112
Closed

internal assertion failing when inserting contrained edge with Spade 2.11 #111

mockersf opened this issue Aug 4, 2024 · 2 comments · Fixed by #112

Comments

@mockersf
Copy link

mockersf commented Aug 4, 2024

After the 2.11 release, I have this assertion panicking

spade/src/cdt.rs

Lines 880 to 882 in 442fd2e

// try_add_constraint should always succeed as any conflicting edge should have been
// removed temporarily
assert_ne!(new_edges, Vec::new());

If I remove it, everything seems to work as expected

This time it's with f64 🙂

use spade::{ConstrainedDelaunayTriangulation, Point2, Triangulation};

fn main() {
    let edges = [
        (
            Point2 {
                x: 18.69314193725586,
                y: 19.589109420776367,
            },
            Point2 {
                x: 18.69314193725586,
                y: 20.40362548828125,
            },
        ),
        (
            Point2 {
                x: 19.507659912109375,
                y: 20.40362548828125,
            },
            Point2 {
                x: 17.878625869750977,
                y: 18.774595260620117,
            },
        ),
        (
            Point2 {
                x: 20.322175979614258,
                y: 21.218143463134766,
            },
            Point2 {
                x: 15.435077667236328,
                y: 16.331045150756836,
            },
        ),
    ];
    let mut cdt: ConstrainedDelaunayTriangulation<Point2<f64>> =
        ConstrainedDelaunayTriangulation::new();
    for edge in edges {
        let point_a = cdt.insert(edge.0).unwrap();
        let point_b = cdt.insert(edge.1).unwrap();
        cdt.add_constraint_and_split(point_a, point_b, |v| v);
    }
}
thread 'main' panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/spade-2.11.0/src/cdt.rs:882:17:
assertion `left != right` failed
  left: []
 right: []
stack backtrace:
   0: rust_begin_unwind
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panicking.rs:72:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panicking.rs:363:5
   4: spade::cdt::ConstrainedDelaunayTriangulation<V,DE,UE,F,L>::add_splitting_constraint_edge_fallback
             at .cargo/registry/src/index.crates.io-6f17d22bba15001f/spade-2.11.0/src/cdt.rs:882:17
   5: spade::cdt::ConstrainedDelaunayTriangulation<V,DE,UE,F,L>::try_add_constraint_inner
             at .cargo/registry/src/index.crates.io-6f17d22bba15001f/spade-2.11.0/src/cdt.rs:834:13
   6: spade::cdt::ConstrainedDelaunayTriangulation<V,DE,UE,F,L>::add_constraint_and_split
             at .cargo/registry/src/index.crates.io-6f17d22bba15001f/spade-2.11.0/src/cdt.rs:1140:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@Stoeoef
Copy link
Owner

Stoeoef commented Aug 7, 2024

Thanks for sharing! I've got the feeling that this is going to take a few more iterations until it's completely crash free.

Please keep in mind removing the assertion will remove the crash but may result in an incorrect triangulation (more specifically: The edges may not be connected with constraints in a way that you would expect). Remove it on your own risk 😁

I'll look into this!

@Stoeoef
Copy link
Owner

Stoeoef commented Aug 13, 2024

Fixed with v2.12.0 (just released).

Thanks again for the report. Keep them coming 😁

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 a pull request may close this issue.

2 participants