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

Object constructor broken with quadraticBezier #45

Open
Izhaki opened this issue Oct 22, 2019 · 4 comments
Open

Object constructor broken with quadraticBezier #45

Izhaki opened this issue Oct 22, 2019 · 4 comments

Comments

@Izhaki
Copy link

Izhaki commented Oct 22, 2019

From https://github.com/thelonious/kld-intersections/blob/development/docs/ShapeInfo.md#quadratic-bezier:

ShapeInfo.quadraticBezier({p1: {x: 10, y: 20}, p2: {x: 5, y: 10}, p3: {x: 15, y: 30}})

But running this:

  const quadShape = ShapeInfo.quadraticBezier({
    p1: { x: 160, y: 50 },
    p2: { x: 110, y: 90 },
    p3: { x: 60, y: 50 },
  });
  console.log(quadShape);

Logs this:

ShapeInfo {name: "Bezier2", args: Array(3)}
  args: Array(3)
    0: Point2D {x: 160, y: 50}
    1: Point2D {x: 160, y: 50}
    2: Point2D {x: 160, y: 50}
@thelonious
Copy link
Owner

Thanks for reporting this. I’ll take a look shortly and report back.

@thelonious
Copy link
Owner

I see the issue. I'll try to get a fix out this weekend. This will effect cubic béziers using objects as well.

@Izhaki
Copy link
Author

Izhaki commented Oct 26, 2019

I suspect the obvious - you are very busy.

But this repo is a life-line for me. I cannot believe it only got 143 stars. The last commit was on the 27th of May.

The issue I reported is not an issue for me. I just use a different API variant.

But this repo is hugely valuable, so it would be nice to see a bit of maintenance... Just to know someone is still there.

Thanks!

@thelonious
Copy link
Owner

I've pushed a fix to development. Although I've added unit tests for quadratic and cubic béziers being constructed via objects, I'd like to add similar tests for other shapes before pushing another release. However, you can give development a try, if you'd like. Also, this updates dependencies to remove security risks as reported by npm.

Regarding the last commit, here are a few comments that I hope will allay your fears somewhat 🙂.

Firstly, this code was written in 2004, so a commit in the last 5 months seems pretty good to me 😀. Most of the biggest changes have happened recently and mostly consist of updating to the latest version of Javascript, bundling, and some API changes. Really, one of the goals of the API was to keep things low-level and simple partly so it wouldn't need frequent updating. I suspect things built on top of this would be updated much more frequently.

Secondly, I should mention that over the years I have been testing new approaches and algorithms, mainly to improve accuracy and precision. For example, I currently have an implementation in Julia that allows me to run this module's algorithms using different numeric types: rationals, big rationals, big floats, unums, etc.. If those prove promising, I hope to back port to JS at some point.

I say some of this tongue-in-cheek, but it is truly great to hear that you find this code beneficial and I hope that you continue to do so. I have lots of things I'd like to use it for, but sadly, that has yet to come to fruition. If anything, it sounds like you're using it much more than I am!

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

No branches or pull requests

2 participants