`CubicBezier` should not allow constructing discontinuous curves

[mweatherley]

What's wrong?

CubicBezier says that it produces continuous output, but it actually just constructs a bunch of cubic Bézier segments which a priori have nothing to do with each other. The problem is that its input is essentially a Vec<[P; 4]> — an array of quadruples of control points which it takes ownership of:

/// Create a new cubic Bezier curve from sets of control points.
pub fn new(control_points: impl Into<Vec<[P; 4]>>) -> Self {
    Self {
        control_points: control_points.into(),
    }
}

Each quadruple of control points is then used to construct a single CubicSegment. The problem here is that the guarantees from the Bézier construction do not relate these sets of control points to each other at all. For instance, the first segment of the curve will end at the last control point of the first quadruple and start at the first control point of the next.

Generally, the expectation for this kind of construction is that the last control point of one quadruple is the same as the first control point of the next. Thus, the correct input data to such a construction is actually 4 + 3k control points, and the output is actually guaranteed to be continuous from the inputs alone.

In case users actually want to construct arbitrary unions of Bézier segments for some reason, that should be done through CubicCurve or CubicSegment. (I am not sure this is very ergonomic right now, but it probably does not need to be.)

[Wuketuke]

I have made a quick draft of what i would have done:

/// Create a new cubic Bezier curve from sets of control points.
    pub fn new(control_points: impl Into<Vec<[P; 4]>>) -> Option<Self> {
        let control_points = control_points.into();

        // check the vertecies are connected
        let mut index = 1;
        while index < control_points.len() {
            let prev: &P = &(&control_points[index - 1])[4];
            let next: &P = &(&control_points[index])[0];
            if prev != next {
                return None;
            }
            index += 1;
        }

        Some(Self { control_points })
    }

however, the trait VectorSpace doesnt implement PartialEq???? the documentation says "Note that, because implementing types use floating point arithmetic, they are not required to actually implement PartialEq or Eq." Vec3A and Vec4 really dont implement PartialEq, However, when i just add PartialEq to VectorSpace, the code compiles and all unit tests just pass? I dont understand this

[mweatherley]

however, the trait VectorSpace doesnt implement PartialEq???? the documentation says "Note that, because implementing types use floating point arithmetic, they are not required to actually implement PartialEq or Eq."

That's at least partially in regard to the equalities that are supposed to be "satisfied" by the vector space axioms, which probably don't hold precisely, but should morally hold with some degree of imprecision (so still cannot be interpreted using PartialEq. I think my preferred solution for this would be to have the user actually input 4 + 3k control points, but maybe that is ergonomically infeasible.

[Wuketuke]

i agree that theoretically, 4 + 3k control points is better, because invalid states will not be representable. maybe there could be a bezier_segment struct/enum that holds 3 VectorSpace points, and then a reference to either the next bezier_segment or the endpoint. Then, one segment could have a function that returns an array of 4 points with its 3 points and the start of the next point (or the end). i think that solution would just postpone this issue down the road to the constructor of that new struct, at the cost of making everything a bit more complicated. so i think this specific iteration would not work. if we just make a constructor that takes a Vec of points, it becomes problematic when the user gives the incorrect amount. maybe we could allow the construction of discontinuous bezier curves, and have a function like fn is_continuous(&self) -> bool these are at least all the solutions that i can come up with

[mweatherley]

Yeah I don't really see any perfect solution here; maybe this is a case where we could benefit from having multiple constructors. e.g.:

1. One constructor that works as you laid out before, with a PartialEq requirement and checking that the segments actually match
2. A second that takes an arbitrary-length vector input and checks that it's the right size and uses 4 + 3k points.

I'm generally pretty against checking the condition when we can enforce it at the level of the type.

[Wuketuke]

@mweatherley do u have a code example where u used that function? maybe that could help us find the most ergonomic solution