Skip to content

Reference Catmull-Rom in the docs for curvePoint #5493

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

Merged
merged 3 commits into from
Mar 4, 2022

Conversation

davepagurek
Copy link
Contributor

This is a small documentation change to make it clearer what curvePoint is calculating. Right now, bezierPoint's documented return value is a Bezier value, but the docs for curvePoint also call its return value a Bezier value. Hopefully this will be clearer if the curvePoint docs mention the type of spline it is calculating a point alone.

Changes:

  • Mentions that the return value of curvePoint is a value on a Catmull-Rom spline (which is what curve draws) as opposed to a Bezier value (which is what bezierPoint draws)

PR Checklist

  • [Inline documentation] is included / updated

@lmccart
Copy link
Member

lmccart commented Mar 2, 2022

@davepagurek I wonder if many p5.js users will know what a Catmull-Rom is. Could we simply say "curve value" and presumably it's the value along the curve specified? If it's important to mention it's using Catmull-Rom to calculate, maybe that could be added as a line in the description.

@davepagurek
Copy link
Contributor Author

I agree that it's probably not something most people will recognize or need to know. I'm guessing what'll be important to people is that it does a similar calculation to curveVertex, so saying "curve" (and maybe even linking it to the curveVertex docs?) would get the job done. The curveVertex page also mentions Catmull-Rom splines already, so it's there for people to look into if they're interested.

@lmccart
Copy link
Member

lmccart commented Mar 3, 2022

@davepagurek that sounds like a good plan

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 this pull request may close these issues.

2 participants