thibauts / b-spline

B-spline interpolation
MIT License
299 stars 48 forks source link

Closed loop curves end point don't match start point #15

Closed ghsoares closed 3 years ago

ghsoares commented 3 years ago

Hello! I've found this repository a couple of days ago when searching for nurbs curve implementations for C#, and I really liked how simple to understand the source code is and how easy is to use.

I've found a strange behaviour when using closed loop curves (repeating the first degree + 1 points at the end of the points array, as in the example), that the end point (t = 1) doesn't match start point (t = 0), actually it overshoots a bit in my tests (random generated 2D points with degree = 3, auto generated knots and weights).

To better illustrate, there's a video of drawing the spline in a canvas: https://user-images.githubusercontent.com/43936806/125488852-15926514-f731-4173-929e-60493b18691d.mp4

Here's the files used for the test: BSpline Test.zip

ghsoares commented 3 years ago

After some tests, somehow if you remap t from [0, 1] to [0, 1 - 1 / (numPoints + 1)] the spline loops perfectly, without overshooting the first points, here's a video demonstrating: https://user-images.githubusercontent.com/43936806/125511764-17b566d8-1ba1-4451-aa33-e467a77ab5d9.mp4

So, in the code that draws in the canvas it is something like this:

var maxT = 1.0 - 1.0 / (points.length + 1); // "points" is the original points array, without repeating the first degree + 1 points

var p = BSpline(t * maxT, degree, newPoints); // "newPoints" is the original points array which is added the first degree + 1 points at the end of the array
thibauts commented 3 years ago

Hi ! That’s very interesting. I had completely missed this fact. Looks like it will be difficult to integrate it in the interpolation code. As I understand it happens only on closed curves that repeat start points. Does your remapping depend on the degree of the curve ? How would you fix it in the lib ? As code ? Documentation ? Interested in your input.

ghsoares commented 3 years ago

In my tests, it only happens in closed curves with unclamped knots vector, without depending on the degree of the spline, the example with a circle in readme.md don't have the same issue, as t = 0 will always be the first point and t = 1 will always be the last point because of the clamped knots vector. I propose the creation of a wrapper class, that would handle automatically closed curves and remap t when needed, I can create a PR if needed.

thibauts commented 3 years ago

Design-wise I'd like to keep this repo very low-level and very simple. Yet, it would be nice to have a wrapping class that deals with this transparently as you suggest. A simple solution would be to layer the object-oriented interface on top of this module in a second repo. This way, this module continues to operate at a single abstraction level and you're free to build the best interface for higher-level needs. On this repo's side I think an update to the docs is needed so others can benefit from your findings. I'm deep into other subjects currently and it would be really awesome if you could patch the unclamped closed examples in the README. What do you think ?

ghsoares commented 3 years ago

A simple solution would be to layer the object-oriented interface on top of this module in a second repo. This way, this module continues to operate at a single abstraction level and you're free to build the best interface for higher-level needs.

I can work on a module for NURBS curves, that can be used for a more general purpose like on Blender.

On this repo's side I think an update to the docs is needed so others can benefit from your findings. I'm deep into other subjects currently and it would be really awesome if you could patch the unclamped closed examples in the README. What do you think ?

Of course, I can update the docs. Should I add a reference link to this second repository as well?

thibauts commented 3 years ago

That's merged and pushed to npm. Thanks for your PR @ghsoares !