-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat(Superformula): add superformula #274
Conversation
Run & review this pull request in StackBlitz Codeflow. |
✅ Deploy Preview for cientos-tresjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
src/core/shapes/Superformula.vue
Outdated
/** | ||
* B's first exponent | ||
*/ | ||
expB1?: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andretchen0 looks amazing, I think I haven't seen this since university haha. When animated looks mesmerizing.
Wonder if this is the best way we can over the props:
The exponents are limited to 3 or to "N"? Could the user pass them as arrays?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andretchen0 looks amazing, I think I haven't seen this since university haha. When animated looks mesmerizing.
Wonder if this is the best way we can over the props:
The exponents are limited to 3 or to "N"?
They are limited to 3. (The 3d plot is the product of 2 2d plots. Each 2d plot has 3 exponents.)
Could the user pass them as arrays?
Just to be clear: Do you want to offer both :args
and :exp-a1
, etc? Or just :args
?
If we want to offer :args
, I guess I'd prefer to allow the user to pass an :args
array, in addition to keywords.
Reasoning
All the args are optional and there are currently 11 of them. In the case of positional args, that leads to calls like:
<Superformula :args="[128, 128, 4, undefined, undefined, undefined, 3, undefined, undefined, 1]" />
The editor's auto-complete helps out, but it still seems kind of hairy. (I was also thinking of offering phiStart
, phiLength
, thetaStart
, thetaLength
, as these make it possible to create some shapes that are impossible with the superformula alone. So that's 4 extra args, or 15 in total.)
I see that <Sphere />
works with :args
but not keyword arguments. But <Sphere />
only really needs 3 or fewer of those - the last 4 args are for drawing partial spheres or offsetting.
In contrast, <Superformula />
really needs 3+ arguments, often more, in order to specify the shapes the user wants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there @andretchen0 , normally :args
is reserved for the class constructor params. I was thinking more of:
<Superformula :exp-b="[b1, b2, b3]" />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Superformula :exp-b="[b1, b2, b3]" />
Ah, I see. Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing job @andretchen0 Thanks for grouping the props, lets merge this
Closes #270
pnpm run docs:dev
and see/guide/shapes/superformula.html
.pnpm run playground
and see/shapes/superformula
.NOTE: Uses the unmerged
.gitignore
from this PR.