-
Notifications
You must be signed in to change notification settings - Fork 73
Conversation
Should close #136 |
@@ -110,6 +110,10 @@ Shape Constructors | |||
Right hand palm on p1, fingertips on p2, thumb is the normal vector | |||
(points away from the half-plane). | |||
|
|||
``curve.quadratic p0 p1 p2`` |
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.
This doesn't belong here, since curve.quadratic
doesn't work unless you have included lib.curves
, which isn't mentioned. Since this is actually documentation for lib.curves
, it belongs in docs/lib/Curves.rst
.
@@ -0,0 +1,66 @@ | |||
{ |
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.
If you include lib.curves
, then you import definitions of dot2, cross2 and sdQuadraticBezier, which pollutes your namespace.
A better design is to hide the internal code like this:
let
dot2 = ...;
cross2 = ...;
sdQuadraticBezier = ...;
in {
quadratic = ...;
}
); | ||
) | ||
in | ||
sqrt res * sign sgn; |
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.
This looks like the old code with the bad SDF, not the new code that you got after emailing IQ.
This PR also includes PR #139 in its entirety, which should be removed. A spline library should ideally include both "stroke" and "area" primitives, in 2D and in 3D, for various spline types. I'm not saying you should implement all of this right away. It's more that I'm thinking about a general naming convention for the different spline primitives we may want to support. There are multiple types of splines used in computer graphics, not just Bezier, so the name "quadratic" is too vague. There are also quadratic B-splines, etc. Also, the list of control points that define the spline ought to be specified as a single argument value, a list of points. I think this is more idiomatic in Curv, especially given the rich set of operations on lists of points. So I'm thinking the namespace and API should look more like this:
|
No description provided.