Skip to content

try_length() panics on an unclamped knot vector instead of returning Err #104

Description

@khellang

Hi! 👋🏻 Thanks for a great library — curvo is doing the NURBS evaluation in a pure-Rust IFC reader I am building, and it has been a pleasure to work with. One thing I ran into:

NurbsCurve::try_length returns anyhow::Result, but on a curve with an unclamped (periodic) knot vector it panics with a slice-index error rather than returning an error.

Periodic curves are written this way routinely. Every IfcBSplineCurveWithKnots in the buildingSMART basin-advanced-brep sample is degree 3 with seven control points over eleven knots at unit multiplicity, and try_new accepts those unmodified: knots_domain() correctly reports [-4, 0], is_clamped() correctly reports false, and point_at/rational_derivatives are correct across the whole domain. Only try_length fails.

Reproducer

use curvo::prelude::*;
use nalgebra::Point4;

fn main() {
    // Degree 3, seven control points, eleven knots -7..=3, every multiplicity 1.
    // This is an unclamped uniform (periodic) knot vector: `try_new` accepts it and
    // `knots_domain()` correctly reports [-4, 0].
    let square = [(1.0, 0.0), (0.0, 1.0), (-1.0, 0.0), (0.0, -1.0)];
    let control_points: Vec<Point4<f64>> = square
        .iter()
        .chain(square[..3].iter())
        .map(|(x, y)| Point4::new(*x, *y, 0.0, 1.0))
        .collect();
    let knots: Vec<f64> = (0..11).map(|i| i as f64 - 7.0).collect();

    let curve = NurbsCurve3D::try_new(3, control_points, knots).unwrap();
    assert_eq!(curve.knots_domain(), (-4.0, 0.0));
    assert!(!curve.is_clamped());

    let _ = curve.try_length(); // panics
}

Output:

thread 'main' panicked at curvo-0.1.91/src/curve/nurbs_curve.rs:710:33:
slice index starts at 3 but ends at 1

Cause

src/curve/nurbs_curve.rs:700-710. The function trims the decomposed Bézier segments by how far the end knot multiplicities fall short of degree + 1:

let required_multiplicity = self.degree + 1;
let i = required_multiplicity.saturating_sub(start);
let j = if end < required_multiplicity {
    segments.len() - (required_multiplicity - end)
} else {
    segments.len()
};
let segments = &segments[i..j];

For a uniform unclamped vector both multiplicities are 1, so with degree == 3 and four Bézier segments this is i = 3, j = 4 - 3 = 1, and the slice &segments[3..1] panics. i is guarded with saturating_sub but j is not, and nothing checks i <= j.

The two trims also over-subtract independently, so the arithmetic looks questionable beyond the panic: the correct count of interior Bézier segments for this curve is four, and any positive trim discards real geometry.

Suggested fix

At minimum, return the error the signature promises rather than panicking, for example by clamping j to at least i or by bailing when i > j. A try_ function returning anyhow::Result is the one place a panic is unambiguously unintended.

Beyond the crash, note that try_clamp() is not a workaround: it inserts knots at knots.first() and knots.last(), which for this curve are -7.0 and 3.0, both outside the domain [-4, 0]. That extends the curve rather than clamping it, and reports 1592.67 against a true length of 902.41. try_trim_range((lo, hi)) followed by try_length() on the first returned curve does give the correct value, so that is the workaround we used for cross-checking.

Environment

curvo 0.1.91, nalgebra 0.35, default features. macOS aarch64, Rust 1.95. Not platform-dependent: it is an index computation.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions