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

[p5.js 2.0 RFC Proposal]: New API for vertex functions #6766

Open
5 of 21 tasks
GregStanton opened this issue Jan 23, 2024 · 47 comments
Open
5 of 21 tasks

[p5.js 2.0 RFC Proposal]: New API for vertex functions #6766

GregStanton opened this issue Jan 23, 2024 · 47 comments

Comments

@GregStanton
Copy link
Collaborator

GregStanton commented Jan 23, 2024

Increasing access

Implementing this proposal would

  • decrease complexity
  • increase consistency
  • enable new features

As a result, p5.js would be more accessible to beginners, while offering others access to a wider feature set.

Which types of changes would be made?

  • Breaking change (Add-on libraries or sketches will work differently even if their code stays the same.)
  • Systemic change (Many features or contributor workflows will be affected.)
  • Overdue change (Modifications will be made that have been desirable for a long time.)
  • Unsure (The community can help to determine the type of change.)

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

What's the problem?

Short version

The current API for vertex functions contains complexities, inconsistencies, and hard-to-extend features. It's included below for easier comparison with the proposed API.

// vertex functions (brackets indicate optional parameters)
// copied from the syntax section of the p5.js reference
// note that arcVertex() is planned but not currently included

vertex(x, y, [z], [u], [v])
// arcVertex(x2, y2, x3, y3, x4, y4, x5, y5)
// arcVertex(x2, y2, z2, x3, y3, z3, x4, y4, z4, x5, y5, z5) 
quadraticVertex(cx, cy, x3, y3)
quadraticVertex(cx, cy, cz, x3, y3, z3)
bezierVertex(x2, y2, x3, y3, x4, y4)
bezierVertex(x2, y2, z2, x3, y3, z3, x4, y4, z4)
curveVertex(x, y, [z])

Long version

Eleven distinct problems arise from the current API for vertex functions. Below, each problem is listed and tagged with a problem type. (In case we want to modify the proposed solution, we can check the new version against this list.)

  1. Texture coordinates: Texture coordinates are currently supported inconsistently, and it's infeasible to fix this without a change to the API. Specifically, the user can specify texture coordinates to vertex() but not to quadraticVertex(), curveVertex(), or bezierVertex(), as noted in #5722. Under the current API, specifying texture coordinates would require up to fifteen parameters in a single function call, with the following syntax: bezierVertex(x2, y2, z2, u2, v2, x3, y3, z3, u3, v3, x4, y4, z4, u4, v4). [Inconsistency/Inflexibility]
  2. Parameter grouping: The last example may look like it contains a typo, but it doesn't. The current parameter lists for quadraticVertex() and bezierVertex() really do start with x2, y2/x2, y2, z2. That's because the first set of parameters x1, y1/x1, y1, z1 must be specified separately, and we need a way to distinguish the second, third, and fourth points since they're grouped together in the same function call. [Complexity]
  3. Mixed commands: If we want to make a quadratic or cubic Bézier curve, that first point x1, y1/x1, y1, z1 gets passed to vertex(). But that's just for Bézier curves. If we want to make a Catmull-Rom spline, the first point and all successive points get specified directly to curveVertex(). In other words, the current API allows the user to call vertex() to start and continue a polyline, and to call curveVertex() to start and continue a Catmull-Rom spline; however, to start a shape with a quadratic or cubic Bézier curve, the user needs to mix commands. [Inconsistency]
  4. Singularity of "vertex": A function like bezierVertex() is named after a single vertex, but it accepts coordinates for three points. We may try to reconcile this by identifying the first two of those points as "control points" and the last point as a "vertex," but this is an uncommon distinction, and it doesn't hold up: we cannot reconcile it with the meaning of "vertex" in curveVertex(). [Inconsistency]
  5. Multiple signatures: With the current API, it's necessary to specify multiple signatures for the same function, which increases complexity. For example, we have both bezierVertex(x2, y2, x3, y3, x4, y4) and bezierVertex(x2, y2, z2, x3, y3, z3, x4, y4, z4). Each of these signatures looks rather complicated by itself. [Complexity]
  6. Missing primitives: Right now, bezierVertex() and bezier() are supported, and curveVertex() and curve() are supported. We also have quadraticVertex(), but quadratic() is not supported. (I've proposed separately that we should introduce a new function arcVertex() to address a similar inconsistency.) [Inconsistency]
  7. "Quadratic" confusion: The name quadraticVertex() is confusing for multiple reasons. Both quadraticVertex() and bezierVertex() produce Bézier curves, but only one of them contains the name "Bézier"; in other words, one function is named after the type of curve while the other is named after the order of the curve. Also, many p5 users are students who will have recently learned in algebra that "the vertex of a quadratic" (a parabola) corresponds to its lowest or highest point; that's not generally true of a vertex specified with quadraticVertex(). [Inconsistency]
  8. Higher-order Bézier curves: With the current API, supporting general higher-order Bézier curves is not possible. (Even if p5.js may not want to implement higher-order Bézier curves, add-on libraries may want to.) [Inflexibility]
  9. Bézier surfaces: Right now, supporting Bézier surfaces (in p5.js or in an add-on library) would introduce more complications to the API. For example, a quadratic Bézier triangle is defined by six control points. If we specify the first of these with vertex() and specify the next four with quadraticVertex(), we only have five vertices. We could potentially use vertex() to specify the sixth vertex at the end, but this is inconsistent with how vertex() is used everywhere else, and it still requires us to mix command types to create a single primitive. [Inflexibility]
  10. Bézier syntax vs. other syntax: The syntax for the vertex functions is inconsistent. The current API bundles multiple points together into a single function call for quadratic and cubic Bézier curves, but it uses one function call per point for polylines and Catmull-Rom splines. [Inconsistency]
  11. Two meanings of "curve": The name curveVertex() uses a general term for a special shape, which leads to inconsistencies. For example, in the p5 reference, “Curves” is used as a general section heading that includes Béziers, while curve() specifically creates Catmull-Rom splines only. This may lead to confusion. For instance, if we introduce a function such as curveType() for different spline implementations, it will be hard to guess whether it applies to all curves or only curves made with curveVertex(). [Inconsistency]

What's the solution?

The main idea is to pass only one vertex into each vertex function.

Proposed API

The proposed API is indicated below.1

// mode function (could default to n = 3, with native support for n = 2 and 3, at least)
bezierOrder(n)

//mode function (defaults to `INCLUDE` for interpolated ends, while `EXCLUDE` indicates default 1.x behavior)
splineEnds(mode)

// vertex functions (brackets indicate optional parameters)
vertex(x, y, [z], [u], [v])
arcVertex(x, y, [z], [u], [v])
bezierVertex(x, y, [z], [u], [v])
splineVertex(x, y, [z], [u], [v])

Amazingly, this simple API solves all eleven problems listed above 🤯The basic design was inspired by this comment from @davepagurek.

Notes:

  • bezierOrder() eliminates the need for quadraticVertex()
  • arcVertex() is based on the latest iteration of #6459
  • splineVertex() replaces curveVertex()2

Code examples

Example 1: Consistency across all types of curves
Note: Conceptually, polylines can be viewed as linear splines or as chained first-order Béziers.

Polylines (no change proposed) Splines (proposed) Bézier curves (proposed)
beginShape();

// polyline 
// (with three segments)

vertex(x0, y0, z0);
vertex(x1, y1, z1);
vertex(x2, y2, z2);
vertex(x3, y3, z3);

endShape();
beginShape();

// Catmull-Rom spline
// (a type of cubic spline)

splineVertex(x0, y0, z0);
splineVertex(x1, y1, z1);
splineVertex(x2, y2, z2);
splineVertex(x3, y3, z3);

endShape();
beginShape();

// cubic Bezier curve
bezierOrder(3);

bezierVertex(x0, y0, z0);
bezierVertex(x1, y1, z1);
bezierVertex(x2, y2, z2);
bezierVertex(x3, y3, z3);

endShape();

Example 2: Clear function names, readable parameter lists, and flexible design
Note: Chaining quadratic and cubic Bézier curves does not currently work in p5, but issue #6560 aims to address this.

Chained Bézier curves (current) Chained Bézier curves (proposed)
beginShape();

// two quadratic Bezier curves
// explicit starting vertex required

vertex(x0, y0, z0);

quadraticVertex(x1, y1, z1, x2, y2, z2);
quadraticVertex(x3, y3, z3, x4, y4, z4);

// one cubic Bezier curve
// implicit starting vertex at x4, y4, z4

bezierVertex(x5, y5, z5, x6, y6, z6, x7, y7, z7);

endShape();







beginShape();

// two quadratic Bezier curves
// explicit starting vertex required

bezierOrder(2);
bezierVertex(x0, y0, z0);

bezierVertex(x1, y1, z1);
bezierVertex(x2, y2, z2);

bezierVertex(x3, y3, z3);
bezierVertex(x4, y4, z4);

// one cubic Bezier curve
// implicit starting vertex at x4, y4, z4

bezierOrder(3);
bezierVertex(x5, y5, z5);
bezierVertex(x6, y6, z6);
bezierVertex(x7, y7, z7);

endShape();

Example 3: Consistency across all chained shapes (curves, triangles, and quads)
Note: Bézier control points are rendered below as a visual aid, but the code for that is not shown.

Quad strip (no change proposed) Bézier curves (proposed)
A quad strip consisting of three adjacent quadrilaterals. A chain of three quadratic Bézier curves, with their control points.
beginShape(QUAD_STRIP);

// first quad
// four vertices required
vertex(30, 20);
vertex(30, 75);
vertex(50, 20);
vertex(50, 75);

// one more quad
// two vertices required (two reused)
vertex(65, 20);
vertex(65, 75);

// one more quad
// two vertices required (two reused)
vertex(85, 20);
vertex(85, 75);

endShape();
beginShape();

// first quadratic Bezier curve
// three vertices required
bezierOrder(2);
bezierVertex(5, 12);
bezierVertex(41, 12);
bezierVertex(23, 30);

// one more quadratic Bezier curve
// two vertices required (one reused)
bezierVertex(5, 48);
bezierVertex(41, 48);

// one more quadratic Bezier curve
// two vertices required (one reused)
bezierVertex(182, 54);
bezierVertex(41, 57);

endShape();

Pros (updated based on community comments)

  • Simplicity: All the complexities noted in the problem statement are eliminated.
  • Consistency: All inconsistencies noted in the problem statement are eliminated.
  • Flexibility: Multiple new features are possible, with a more intuitive API.
  • Readability: Code is easier to read since long lists of positional parameters are eliminated.
  • Predictability: An arc starts with arcVertex(), rather than vertex(). Same for other curve types.

Cons (updated based on community comments)

  • Breaking changes: Breaking changes will always cause difficulties when they are first introduced, for some people. For example, YouTubers may need to update their video tutorials, and add-on library authors may need to update their code if they want to use the most recent version of p5.js. However, the proposal can be implemented without breaking changes, at a small cost.3
  • Differences with other APIs: Changing the p5.js API would mean a bigger departure from the Processing API, the native canvas API, and the SVG API. The latter two API's are similar to p5's, but they sidestep some of the problems by referring to their commands as "curve" commands rather than "vertex" commands. In the context of p5, beginners won't tend to know those other APIs, and more experienced users may have less trouble adapting, so this seems like a smaller concern.

Proposal status

Under review

Updates

This proposal has been updated to reflect some small adjustments, based on the discussion in the comments section:

  1. bezierOrder() was added to the API so that chained segments of different orders can be distinguished
  2. arcVertex() was added based on the current consensus in #6459
  3. curveVertex() was replaced with splineVertex() based on Problem 11, which was added to the problem list
  4. splineEnds() was added to the proposed API to allow interpolated and non-interpolated endpoints

Additional code examples have also been included.

Footnotes

  1. We may also want to support texture coordinates when a z-coordinate is not passed. This is currently supported by vertex() (see the last example for texture() in the reference); however, it requires a separate signature for vertex() that is not documented on its reference page (i.e. vertex(x, y, [u], [v])). We'd need to discuss whether the benefits justify the extra complexity (namely, documenting and supporting an extra signature for every type of vertex function).

  2. For consistency, we could also rename curve() as spline().

  3. We can distinguish new and old usage of bezierVertex() by detecting the number of arguments, and we can deprecate the old usage. We could deprecate quadraticVertex() and curveVertex() entirely with the @deprecated tag in the inline documentation.

@davepagurek
Copy link
Contributor

Thanks for the writeup @GregStanton! I'm in favor of the consistency we'd get from an API like this, as well as the readability (any time a function has a bunch of positional arguments, I find myself having to write comments between arguments to remind myself which is which.)

For curveVertex texture coordinates, I think the same pattern we're suggesting for bezier vertices will also apply. When getting a position along a Catmull-Rom curve in between two control points, the interpolated position is a weighted sum of the curve's control points. I think we can do the same weighting for texture coordinates if we have one texture coordinate per control point, similar to position.

@limzykenneth
Copy link
Member

I do really like the overall simplicity and flexibility this can potentially give. One thing I would like to illustrate this proposal a bit more would be some more complete code examples, maybe even a before and after versions to make the use case clearer.

I'll probably share this with my colleagues teaching with p5.js to get their thoughts as well.

@GregStanton
Copy link
Collaborator Author

GregStanton commented Jan 24, 2024

@limzykenneth Thanks! Code examples are a great idea. Here's a list of examples that illustrates some key differences:

Example 1

Current syntax:

beginShape();
vertex(x0, y0, z0); 
bezierVertex(x1, y1, z1, x2, y2, z2, x3, y3, z3);
endShape();

beginShape()
curveVertex(x0, y0, z0); 
curveVertex(x1, y1, z1); 
curveVertex(x2, y2, z2); 
curveVertex(x3, y3, z3);
endShape();

Proposed Syntax:

beginShape();
bezierVertex(x0, y0, z0); 
bezierVertex(x1, y1, z1); 
bezierVertex(x2, y2, z2);
bezierVertex(x3, y3, z3);
endShape();

beginShape();
curveVertex(x0, y0, z0); 
curveVertex(x1, y1, z1); 
curveVertex(x2, y2, z2); 
curveVertex(x3, y3, z3);
endShape();

Example 2

Current syntax (the Bézier case isn't actually supported, but a user might expect this to work):

beginShape();
vertex(x0, y0, z0, u0, v0);
vertex(x1, y1, z1, u1, v1);
vertex(x2, y2, z2, u2, v2);
vertex(x3, y3, z3, u3, v3);
endShape();

beginShape();
vertex(x0, y0, z0, u0, v0);
bezierVertex(x1, y1, z1, u1, v1, x2, y2, z2, u2, v2, x3, y3, z3, u3, v3);
quadraticVertex(x4, y4, z4, u4, v4, x1, y1, z1, u1, v1);
endShape();

Proposed syntax (contours are used to specify separate pieces of different orders):

beginShape();
vertex(x0, y0, z0, u0, v0);
vertex(x1, y1, z1, u1, v1);
vertex(x2, y2, z2, u2, v2);
vertex(x3, y3, z3, u3, v3);
endShape();

beginShape();
beginContour();
bezierVertex(x0, y0, z0, u0, v0);
bezierVertex(x1, y1, z1, u1, v1);
bezierVertex(x2, y2, z2, u2, v2);
bezierVertex(x3, y3, z3, u3, v3);
endContour();
beginContour();
bezierVertex(x4, y4, z4, u4, v4);
bezierVertex( x1, y1, z1, u1, v1);
endContour();
endShape()

Example 3

Current syntax (the first one will cause an error because it's not supported, due to an inconsistency):

quadratic(x0, y0, x1, y1, x2, y2); 
bezier(x0, y0, x1, y1, x2, y2, x3, y3);

Proposed syntax:

bezier(x0, y0, x1, y1, x2, y2);
bezier(x0, y0, x1, y1, x2, y2, x3, y3);

Example 4

Current syntax (this will produce an error because higher-order Bézier curves aren't currently supported):

beginShape();
vertex(x0, y0); 
bezierVertex(x1, y1, x2, y2, x3, y3, x4, y4);
endShape();

beginShape();
curveVertex(x0, y0); 
curveVertex(x1, y1); 
curveVertex(x2, y2); 
curveVertex(x3, y3); 
curveVertex(x4, y4)
endShape();

Proposed syntax (this assumes we want to support higher-order Bézier curves):

beginShape();
bezierVertex(x0, y0); 
bezierVertex(x1, y1); 
bezierVertex(x2, y2); 
bezierVertex(x3, y3); 
bezierVertex(x4, y4);
endShape();

beginShape();
curveVertex(x0, y0); 
curveVertex(x1, y1); 
curveVertex(x2, y2); 
curveVertex(x3, y3); 
curveVertex(x4, y4);
endShape();

Example 5

Current syntax (this is how a user might try to create a Bézier triangle, but that's not currently supported):

beginShape(TRIANGLES); 
vertex(x0, y0, z0); 
quadraticVertex(x1, y1, z1, x2, y2, z2); 
quadraticVertex(x3, y3, z3, x4, y4, z4); 
vertex(x5, y5, z5); 
endShape();

Proposed syntax:

beginShape(TRIANGLES); 
bezierVertex(x0, y0, z0); 
bezierVertex(x1, y1, z1); 
bezierVertex(x2, y2, z2); 
bezierVertex(x3, y3, z3); 
bezierVertex(x4, y4, z4); 
bezierVertex(x5, y5, z5); 
endShape();

Special cases

Shape built from multiple béziers

One issue with having a single bezierVertex() function (instead of separate functions for quadratic and cubic cases), is that we need some other way to infer the order when a single shape is made out of multiple Bézier curves or surfaces. See Example 2. (A similar situation arises with Bézier surfaces. For example, the case of three cubic Bézier triangles and the case of five quadratic Bézier triangles each require a total of thirty vertices; so, we can no longer determine the order of the surface based on the number of function calls.) One possible solution, based on contours, is indicated in Example 2.

Composite shapes

Currently, composite shapes aren't supported, but #6560 would address this, and it provides more examples of that type in the current syntax.

TODO / Invitation for contributions

  1. Fully consider the contour approach to handling multiple pieces of possibly different orders, to make sure it will work.
  2. Consider if there are any other cases (e.g. cases of composite shapes) that would be good to explore here.
  3. When describing which examples would produce errors in the current version of p5.js, I was using my understanding of the current feature set; I haven't tested those examples yet. If anyone wants to choose reasonable values for the parameters and run the code examples for the current API, that would be a nice, quick contribution.
  4. @capGoblin: If I understand correctly, you're already working on a trial implementation for some of this, right? How would you feel about implementing some of the examples of the proposed API above, in the p5.js Web Editor? (You could pass in your own hard-coded values.) We could link this issue to any examples you make; also, if this proposal is accepted, we could maybe turn them into examples for the p5.js reference.
  5. @davepagurek Do you have any examples to add? Or would you like to make modifications to the examples I gave?

@limzykenneth
Copy link
Member

I want to also invite @peilingjiang who created the p5.bezier addon library to have a look at this. Seeing that your library also deals with Bezier curves and also support higher order Bezier curves, it would be great to hear your thoughts, we can even look at the implementation you have in p5.bezier as a starting point for this too.

@peilingjiang
Copy link
Contributor

Thanks for the invite! Looking at the example code comparisons, I think this would be a very nice update, syntax-wise (I don't know how differently the updated functions will be implemented, so I'll only share some thoughts on the proposed syntax)! The vertex-related syntax would be unified, and having consistent, separate functions with a manageable number of arguments, instead of a crazy list of 9 or 15 arguments, would greatly improve the code readability.

For the implementation, I hope my implementation for the higher-order Bézier curves would be helpful to serve as a starting point for the new native support.

Moreover, I wonder if you would consider having an array of arrays of vertices, e.g.,

beginShape();
bezierVertices([
  [x0, y0, z0, u0, v0],
  [x1, y1, z1, u1, v1],
  [x2, y2, z2, u2, v2],
  [x3, y3, z3, u3, v3]
])
endShape();

I did something similar for the addon for the usability of the API, as managing items in an array (e.g., when adding or removing vertices) would be easier than modifying separate function calls. Moreover, the users may already maintain the positions somewhere else using nested arrays. This would be a further departure from the native Canvas API, but that might not be a bad thing - we could keep the old APIs for a few more minor version updates, mark them as deprecated, and introduce new ones like vertices and bezierVertices.

@GregStanton
Copy link
Collaborator Author

Thanks for your feedback @peilingjiang! Your library looks really cool, by the way. I think it'd be great to flesh out the pros and cons of your API idea, for the sake of discussion. I've already written up a partial list, but that gave me a new idea that I want to think about a bit more. I'll share what I have as soon as I'm able to think it over a bit more.

@GregStanton
Copy link
Collaborator Author

GregStanton commented Jan 31, 2024

Okay, @peilingjiang! I have some initial feedback to share regarding the API you proposed.

Pros

The bezierVertices() API (as opposed to the bezierVertex() API) has some nice benefits. Here are a few of them.

  1. This might make the implementation a bit simpler, since all the data for a path segment is provided in one command.
    1. Right now, we do have a plan for handling an API like the one I've proposed, since that same API is already implemented for curveVertex(). Roughly, the way it works is that each vertex function adds its arguments to a primitive shape object. Since we're going to support composite shapes made of multiple types of primitives, curveVertex() will check to see if the current primitive consists of curve vertices. If so, it will add its data to that segment, and if not, it will start a new primitive shape and add its data to that. With your API, we shouldn't need to check the type of the current segment.
    2. Another issue is that, in order to support different shape kinds in a more extensible way, we end up creating a new curve primitive before checking the type of the current primitive. If the current primitive is already made of curve vertices, the new curve primitive's data is added to that, and then the containing object will be discarded, i.e. the memory should be reclaimed. So we sometimes have a very temporary object, which should be easier to avoid with your API.
  2. The API would match the internal representation more closely, since internally, we end up gathering up all the vertices into a primitive shape object. I already noted above that this may simplify the implementation, but it might have other benefits.
  3. In the proposed API, it's not quite as easy to create composite paths made up of Bézier curves of different orders; however, this is probably a fairly uncommon use case, and surrounding each piece with beginContour()/endContour() is a pretty simple solution (for a piece with control points A, B, C, and D, the user would also have to make sure the next piece starts with D, since using contours means it's no longer implied that pieces are glued together; but that seems like a minor issue).

Cons

The bezierVertices() API also has some potential cons.

  1. With the WEBGL renderer, it's possible to specify color (stroke or fill) for each vertex. Here's an example sketch. Similarly, the user can specify a normal vector for each vertex with normal(). It's not immediately obvious how to achieve these kinds of features in a simple way with bezierVertices(). (Thanks to @davepagurek for raising this point in conversation.)
  2. Nested array arguments are not found anywhere else in the p5.js API, as far as I know, so this would be less consistent with the usual interface.
  3. For beginners, the extra complexity of a nested array argument might be significant.
  4. This API could lead to an inconsistency when we introduce arcVertex(), which I proposed previously in #6459; arcVertex() is a crucial feature, for reasons cited in that issue. Fortunately, this discussion gave me an idea for a new arcVertex() API that would eliminate this inconsistency; we're in the process of exploring its viability.

Questions for the community

  1. Does anyone have any additional pros or cons to add?
  2. Does anyone have any other variations of the API for us to consider?

Edit: Added first point under cons, about features of the WEBGL renderer.

@davepagurek
Copy link
Contributor

davepagurek commented Feb 1, 2024

Just noticed a small thing that might need some clarification, and that's how to distinguish multiple segments of bezier curves.

In current p5, let's say I want to draw a square with two rounded corners, plus a cutout contour:

beginShape()
vertex(20, 20)
vertex(100, 20)
quadraticVertex(180, 20, 180, 100)
quadraticVertex(180, 180, 100, 180)
vertex(20, 180)
  
beginContour()
vertex(40, 40)
vertex(40, 100)
vertex(100, 100)
vertex(100, 40)
endContour()
  
endShape(CLOSE)
image

There are a few things going on here:

  • each quadratic segment starts from where the last segment or vertex left off
  • there are two quadratic semgents side-by-side
  • there's also a contour in the middle that is a separate path

I think based on the proposal currently, the code would look like this:

beginShape()
vertex(20, 20)
vertex(100, 20)

// top right corner
beginContour()
bezierVertex(100, 20)
bezierVertex(180, 20)
bezierVertex(180, 100)
endContour()

// bottom right corner
beginContour()
bezierVertex(180, 100)
bezierVertex(180, 180)
bezierVertex(100, 180)
endContour()

vertex(20, 180)

// The inner contour
beginContour()
vertex(40, 40)
vertex(40, 100)
vertex(100, 100)
vertex(100, 40)
endContour(CLOSE)

endShape(CLOSE)
  • Right now it's using begin/endContour to disambiguate different sections. Currently, contours start separate shapes, similar to a moveTo in native canvas. I think these two uses are different, as fill winding order needs composite paths to still be part of one contour in order to calculate the winding order of the whole path correctly.
    • Alternatively, can we call the segment boundaries begin/endSegment and leave begin/endContour as is?
  • If I'm understanding correctly, I think it also requires values to be duplicated when paths connect, and it's only when they're disconnected that it moves to a new shape.
    • I think it's more common to have paths with multiple segments than to have higher order curves, so I think this it's better to prioritize those two cases in the opposite order.

A suggestion

I think it might solve some of those problems if you still have separate methods for separate orders as before, e.g. bezierVertex and quadraticVertex (and adding new ones for higher orders), but split each control point into multiple function calls.

So this: ...becomes this:
beginShape()
vertex(20, 20)
vertex(100, 20)
quadraticVertex(180, 20, 180, 100)
quadraticVertex(180, 180, 100, 180)
vertex(20, 180)

beginContour()
vertex(40, 40)
vertex(40, 100)
vertex(100, 100)
vertex(100, 40)
endContour()

endShape(CLOSE)
beginShape()
vertex(20, 20)
vertex(100, 20)
quadraticVertex(180, 20)
quadraticVertex(180, 100)

quadraticVertex(180, 180)
quadraticVertex(100, 180)
vertex(20, 180)

beginContour()
vertex(40, 40)
vertex(40, 100)
vertex(100, 100)
vertex(100, 40)
endContour(CLOSE)

endShape(CLOSE)

Now:

  • you don't need contours to disambiguate segments: quadratic segments will always need two quadraticVertex calls and cubic segments will always need three bezierVertex calls, so you can just continue to add more and p5 will know where each one ends
  • contours still always represent whole paths this way
  • there's less code involved when you need to make longer, multi-segment paths, which I think is quite common
  • every segment always picks up from where the last segment left off without the need for duplication

Like in your initial proposal:

  • you still have a reasonable amount of params per function, with the ability to add z and/or uv coordinates to each
  • you can still place fill() or normal() between each control point if you want to change those per control point instead of per segment

curveVertex

In current p5, curveVertex is the odd one out when it comes to picking up where the last segment left off, but that's mostly because it interprets the whole shape as curve vertices whenever you use a single curve vertex. Here are some thoughts about how to handle them now.

I think it makes sense for curve vertices to connect to the last point if it exists to add API consistency. I imagine Catmull-Rom splines like being a set of points, where we determine the tangents by looking at line between the previous and text points:

I'm imagining that if you put a Catmull-Rom curve in the middle of some other curves, it would connect thesecond curveVertex(which is the first that gets drawn) to the previous segment, and connect the second-last curveVertex to the next segment. e.g.:

image

@GregStanton
Copy link
Collaborator Author

Thanks so much @davepagurek! This is a great discussion.

Disambiguating primitives

I'm glad you raised the issue about using beginContour()/endContour() for two different purposes. This leads us to a possible improvement: how about having beginPrimitive()/endPrimitive() (instead of beginSegment()/endSegment()) and beginContour()/endContour() for the two purposes? The "Primitive" in the name would coincide with our current refactoring plan, which handles not just segments but other types of shape kinds as well.

Having beginPrimitive()/endPrimitive() in addition to beginContour()/endContour() would have several benefits, including improvements not directly related to the problem you identified. I think that's a sign of a good design. Here's a list of advantages:

  1. It seems to solve the problem of disambiguating segments (or other primitives) internally.
  2. It improves readability of the user's code. For example, let's say you're using the TRIANGLES shape kind. Right now, we may have fifteen vertex() calls in a row, and it's up to the person reading the code to mentally separate vertex() calls into groups of three, and then determine that the code should produce five triangles based on that. If users wrap every three vertices in beginPrimitive()/endPrimitive(), it's easier to determine that the code should produce five triangles. (The code would be a bit longer, but the added length seems worthwhile.)
  3. This allows us to support implicit start vertices for segments in composite paths, since beginPrimitive() won't entail a moveTo() operation for segments like beginContour() does.
  4. Having beginPrimitive()/endPrimitive() seems to provide most of the benefits of bezierVertices(), without the downsides we identified.
    1. The API might allow us to simplify the implementation. (This requires more thought, but I guess beginPrimitive() may just set a flag to indicate that a new primitive is to be started, and it'd no longer be necessary to check the type of the previous primitive.)
    2. The API would match the internal representation more closely.
    3. It's still possible to specify color and normal data per vertex.
  5. We'd still be able to solve problems 6, 7, 8, and 9 from the original proposal; using quadraticVertex() for disambiguation would reintroduce those problems or replace them with new problems.

Implicit start vertices

Yep! The current refactoring plan handles curveVertex() in the way that you described. Each type of segment will be represented internally by its own class (LineSegment, BezierSegment, etc.) These classes implement the Segment interface, which has the methods getStartVertex() and getEndVertex(). These can work differently depending on the type of segment. For example, the getEndVertex() method of a CurveSegment object will get its second-to-last vertex. So, "end vertex" doesn't refer to the last vertex specified, since that may be a control point that's not on the segment itself; instead, it refers to the vertex that's on the actual end of the segment. If we can come up with a clearer name, that might clear up any confusion. Maybe getStartPoint() and getEndPoint()?

Note: These are initial thoughts. It will be good to confirm the details when time permits.

@davepagurek
Copy link
Contributor

davepagurek commented Feb 1, 2024

I think beginPrimitive()/endPrimitive() addresses the conflicts with beginContour, so that makes sense, thanks! Engineering-wise, I think this all would be safe to implement.

I think as a matter of opinion, I'm still unsure that the extra code required to surround each segment by begin/endPrimitive is worth it? It seems like it has the following pros:

  • It can support higher-order Beziers more easily
  • It addresses the confusing naming of quadraticVertex and bezierVertex
  • It makes segment boundaries more explicit in code
  • It maps more closely to the internal structure

The con for me is really just:

  • It makes it more verbose to write composite paths, involving duplicated vertices for continuity

At least in my own experience, I've rarely felt the need to go higher than a cubic Bezier, and SVG/CanvasRenderingContext2D APIs don't natively support them either. I'd still love to support them, but I'm personally OK going with a more complex API for those since I see them as a very special case. Meanwhile, most shapes I'd want to draw (imagine drawing text glyphs as a motivating example) are composed of connected lines and curves, often with many connected cubic segments. Since a single text glyph can have upwards of 20 segments in it, and each segment would now need a duplicate vertex to remain connected, this adds a lot of overhead, which gives me pause.

As an alternative, I think if we break each control point into their own call but don't try to support multi-order bezier and don't try to surround segments with other functions, we'd still check the other boxes we wanted.

That's just an opinion based on my own experience though! I want to hear if other people's experience is similar to that too.

As a final aside, I think if we keep separate methods for the different bezier orders, maybe it's worth aliasing bezierVertex to cubicVertex and referring to it as the latter in our docs.

@GregStanton
Copy link
Collaborator Author

GregStanton commented Feb 1, 2024

Thanks @davepagurek! I'll reply to your points about using beginPrimitve()/endPrimitive() to distinguish primitive shapes, as well as your ideas about naming.

Considerations for beginPrimitive()/endPrimitive()

Duplicate vertices

With beginPrimitve()/endPrimitive(), I don't think we need to duplicate vertices for continuity. That's what I meant by the following:

This allows us to support implicit start vertices for segments in composite paths, since beginPrimitive() won't entail a moveTo() operation for segments like beginContour() does.

If I'm thinking about that incorrectly, please let me know!

Consistency

For me, part of the problem right now is that Bézier curves are already a special case that needs to be thought about differently. If we support higher-order Bézier curves and distinguish individual segments with beginPrimitive()/endPrimitive(), then all segment primitives will work in a consistent way (I'll paraphrase a comment I made on #6459):

With the proposed API, none of the vertex functions has an upper bound on the number of vertices that may be specified:

  1. vertex() works with any number of vertices greater than one
  2. bezierVertex() works with any number of vertices greater than two
  3. curveVertex() works with any number of vertices greater than three
  4. arcVertex() works with any number of vertices greater than four

General notes on naming

I'm glad you raised the issue of naming.

quadraticVertex()

I think quadraticVertex() is a problematic name by itself (see Problem 7 in the body of this issue). Regarding the name bezierVertex(), cubicVertex() is better in some ways, but it introduces other problems; for example, Catmull-Rom splines are also cubic, and there'd be no obvious indication in the API whether we're using Bézier curves or something else. This discussion also raises an issue with curveVertex() that's been in the back of my mind...

curveVertex()

The name "curve" kind of pollutes the namespace since there are a lot of possible curves, and now we can't use "curve" to refer to anything but Catmull-Rom splines, unless we want to be inconsistent. We can already see this inconsistency in the p5.js reference, where "Curve" is used as a generic section heading for functions that include "curve()" and also "bezier()". Using "curve" to refer to Catmull-Rom splines is also inconsistent with the native canvas API's use of the term "curve" (it uses that term in bezierCurveTo(), for example).

In other words, "curve" is ambiguous since it doesn't specify the type of curve. The name catmullRomVertex() is a bit long and maybe hard to spell, so maybe hermiteVertex()? This would be pretty specific, it'd be consistent with the name bezierVertex() in that both are named after people, and it'd allow some flexibility (e.g. Cardinal splines, Catmull-Rom splines, and Kochanek–Bartels splines are Hermite splines with different choices of tangents). I feel like it might take some getting used to, and naming ideas after people can lead to other problems (but for better or worse, this usage is pretty well established and won't be easy to change).

Another option is something like splineVertex(), which is not ideal since Béziers are also sometimes referred to as splines, but it's at least more precise than curveVertex().

Feedback welcome! / Mock reference page

Just to be clear, I haven't fully thought through beginPrimitive()/endPrimitive() yet, and I'm making these points for the sake of discussion. So it'd be great if anyone wants to add to their feedback! Fleshing out a mock documentation page could be helpful too; that would force us to think through the different uses and how we'd explain them.

@davepagurek
Copy link
Contributor

This allows us to support implicit start vertices for segments in composite paths, since beginPrimitive() won't entail a moveTo() operation for segments like beginContour() does.

I think the issue I was thinking of is that, if there isn't an implicit moveTo, then a cubic vertex segment only needs 3 control points (the fourth is borrowed from the previous curve.) In example 1 above though, we have this:

Current API Proposed API
beginShape();
vertex(x0, y0, z0); 
bezierVertex(x1, y1, z1, x2, y2, z2, x3, y3, z3);
endShape();
beginShape();
bezierVertex(x0, y0, z0); 
bezierVertex(x1, y1, z1); 
bezierVertex(x2, y2, z2);
bezierVertex(x3, y3, z3);
endShape();

Here we've got all four in the segment. I guess the point I'm unclear on is whether or not that's a special case for just the first segment? e.g. would the following also be valid?

beginShape();

vertex(x0, y0, z0); 

beginPrimitive();
bezierVertex(x1, y1, z1); 
bezierVertex(x2, y2, z2);
bezierVertex(x3, y3, z3);
endPrimitive();

endShape();

If it is, then does that mean that within the first primitive, bezier segments will always have one fewer point than in the later ones?

For me, part of the problem right now is that Bézier curves are already a special case that needs to be thought about differently.

For context, what is the special case you're referring to here?

@GregStanton
Copy link
Collaborator Author

GregStanton commented Feb 2, 2024

Thanks for bringing up these points @davepagurek! This is all helpful for filling in the details.

Bézier curves are a special case?

Yeah, I probably could've worded that more clearly. What I meant was that vertex() can be called an unbounded number of times to create a polyline, and curveVertex() can be called an unbounded number of times to create a Catmull-Rom spline. With the latest idea in #6459, we can even call arcVertex() an unbounded number of times to create an elliptical arc that fits the data. If we continue to use quadraticVertex() for quadratic Béziers and bezierVertex() for cubic Béziers, then unlike all the other vertex functions, these may only be called a fixed number of times.

So, I really meant that quadraticVertex()/bezierVertex() represent a distinct case. If we remove quadraticVertex() and use bezierVertex() to specify Bézier curves of any order, then Bézier curves will no longer be a distinct case. They'll work the same as all the other kinds of curves. That's a simplification regardless of whether people end up using higher-order Bézier curves, since we won't have to remind them to never use quadraticVertex() more than three times and to never use bezierVertex() more than four times.

Implied beginPrimitive()/endPrimitive()?

For the very first primitive, we might want to make beginPrimitive()/endPrimitive() optional, so that the following two code snippets do the same thing:

With beginPrimitive()/endPrimitive():

beginShape();
beginPrimitive();
bezierVertex(x0, y0, z0); 
bezierVertex(x1, y1, z1); 
bezierVertex(x2, y2, z2);
bezierVertex(x3, y3, z3);
endPrimitive();
endShape();

Without beginPrimitive()/endPrimitive():

beginShape();
bezierVertex(x0, y0, z0); 
bezierVertex(x1, y1, z1); 
bezierVertex(x2, y2, z2);
bezierVertex(x3, y3, z3);
endShape();

There are pros and cons to making these commands optional for the first primitive. If we always use beginPrimitive()/endPrimitive(), we'll increase consistency, but we'll also add an extra pair of commands that won't be entirely necessary in all cases. I guess I'd probably prefer to err on the side of consistency in the reference examples, but we could make it so that the code still works if the first primitive is specified without beginPrimitive()/endPrimitive() (then users who forget these commands won't have to do any debugging).

Handling the first vertex

I think it makes sense for all of a primitive's vertices to always go inside beginPrimitive()/ endPrimitive(), including the first vertex. For example, we could make it so that a cubic Bézier segment can always be made with four vertices, each of which would be passed to bezierVertex():

beginPrimitive();
bezierVertex(x0, y0, z0); 
bezierVertex(x1, y1, z1); 
bezierVertex(x2, y2, z2);
bezierVertex(x3, y3, z3);
endPrimitive();

However, if the first vertex duplicates the last vertex of a previous primitive, then we could allow the first vertex to be omitted.

I don't think we ever need to use vertex() to specify a vertex in any kind of segment except a line segment (I guess using vertex() to start off other kinds of segments is only necessary in the current API because quadraticVertex() and bezierVertex() aren't currently designed to accept a single vertex).

Implementation notes

I haven't thought through the implementation yet, but I wanted to reply quickly to keep the conversation going. As a next step, one thing we might consider is replacing the segment primitives in our refactoring plan with classes for polylines, Béziers, Catmull-Rom splines, and elliptical arcs. Each of these could hold an unbounded number of vertices. For example, the polyline class would replace the line segment class. That way, we could specify a whole polyline between beginPrimitive()/endPrimitive(), and that would match the internal representation.

@davepagurek
Copy link
Contributor

davepagurek commented Feb 2, 2024

Thanks for explaining! I think my confusion about the special casing is because I saw a different pattern in vertex/quadraticVertex/bezierVertex: all can be used to chain multiple segments, but each segment has an increasing number of control points (1, 2, then 3) from the last point. In fact, vertex() can be seen as a linear bezier, which is just a line segment, needing no control point in the middle. In that sense, adding multiple vertices isn't different from cubic beziers, since each straight line is a segment, not the whole polyline.

(That's also why curveVertex felt like the odd one out to me, where all the points make up one segment. For what it's worth, the curveVertex workflow feels pretty separate from the rest, which are shared in other vector software. A possibly simplifying mental model would just be to treat curve vertices as a shortcut to bezier segments rather than a "real" primitive?)

One of the things that still feels inconsistent in the variable-order proposal is that multiple vertices adds more segments, but multiple of arc or bezier vertices increases the order of the same segment. To me it feels that to be consistent, more vertices in a row would do a line of beat fit through them rather than a polyline. I think the heart of what I'm suggesting is to always add more segments as the standard behavior.

The shift I was initially imagining was from grouping all control points of a segment extension into a single function call, to putting each control point into its own call., and the difference between segment extension types would be the interval at which a whole segment is complete. So vertex() calls, needing only one control point, add a new segment each time, quadratic points add a new segment every other point, cubic every three, and one could add quartic and quintic in the future that add a segment every four and five. (One could maybe add a generic version where you pass in the order into the first parameter.)

Of course, this is all from a frame of mind where chaining is key. I still think this in my own work this is the case that is most common, where paths are composed of many segments. I ideally want the freedom that vertex() has in your proposal, where you can keep chaining more of them without boilerplate, for bezier curves too, since I think in your proposal right now every cubic segment would need to be bounded by begin/endPrimitive. Do you think there's anything we can do in the API proposal to make that use case a bit easier?

@davepagurek
Copy link
Contributor

Another thought: if we do go the route of having chains of vertices be all one segment like you're suggesting, maybe curve vertices don't have to be a separate thing, but just an interpolation strategy for those vertices?

  • The default value is linear
  • The behaviour of Catmull-Rom splines where the first and last points don't render seems to be one of the most confusing aspects of it. maybe rather than keeping that in the exact form it's currently in, we can do something that goes through all the vertices?
    • e.g. for midpoint vertices, get their tangents from adjacent points like normal, but for endpoints, take the tangents from the tangents going in/out
  • Maybe treating the whole set as control points of a higher order bezier would be another form of interpolation?
  • Same with arc vertices jointly approximating a single arc?

So for an API, that would mean quadratic and cubic vertices continue to chain the same way normal vertices do, but you could surround vertices in begin/endSegment(interpolationStrategy) where instead of chaining, we use something akin to basing the order off of the number of points like before?

Sorry if I'm derailing a bit haha, just wanted to share some thoughts that might combine both models!

@GregStanton
Copy link
Collaborator Author

GregStanton commented Feb 2, 2024

This is getting very interesting @davepagurek! I'm going to think about this some more. As if we didn't already have enough to think about, here are some papers that your recent comments inspired me to discover:

I've only barely glanced through them, but the first of these seems to provide an interesting framework for a whole class of splines, which includes Bézier splines and elliptical splines (splines made from elliptical arcs) as special cases. They're distinguished by choosing different interpolation functions. The second article seems to develop an approach that's specific to splines made up of elliptical arcs.

I need to think through everything again in light of all this discussion, so I'm not sure if these will be relevant yet, but I figured I'd share them in case they inspire ideas from others.

@davepagurek
Copy link
Contributor

davepagurek commented Feb 2, 2024

Good morning again, just adding a summary comment about some of the discussion so far so it's easier to follow!

  • Regardless of what we do, we want to split up large function calls into multiple smaller calls for readability, e.g. splitting each point of bezierVertex() into its own call.
  • It feels like there are two "modes" one might want to create curves in:
    1. Aggregating: The more points you add, a single segment will interpret those points, e.g. by interpolating a curve through all of them, or approximating them all with a curve. (The p5.bezier library works like this, where more points make a higher-order single bezier segment. Another idea mentioned for arcs is to draw a single arc that best fits all the points provided. curveVertex also arguably works like this.)
    2. Chaining: The more points you add, the more segments get added. (This is how quadraticVertex, bezierVertex, and arguably vertex currently work.)
  • Originally Greg was proposing an aggregating-first API, and I was proposing keeping a chaining-first API.
  • Potentially we can include both: the approximating/interpolating mode proposal would surround segments with begin/endPrimitive(). This could potentially coexist with chaining API functions, which aren't surrounded, e.g.:
    beginShape()
    // Chaining
    vertex(0, 0)
    quadraticVertex(50, 0)
    quadraticVertex(50, 50) // completes one quadratic segment
    quadraticVertex(50, 100)
    quadraticVertex(0, 100) // completes a second quadratic segment
    
    // Aggregating
    beginSegment(BEZIER)
    vertex(-20, 80)
    vertex(20, 50)
    vertex(-20, 20)
    vertex(0, 0)
    endSegment() // completes a single quintic bezier segment
    endShape()

@GregStanton
Copy link
Collaborator Author

Thanks so much for the summary @davepagurek! I'll add a few thoughts about the existing options and propose a new one.

A chaining-only API

One option is to keep the original proposal, with a few changes.

  • Rename bezier() to cubicBezier() and create a new quadraticBezier() function. (This solves Problem 6 of the proposal.)
  • Rename quadraticVertex() and bezierVertex() as quadraticBezierVertex() and cubicBezierVertex(), respectively. These names are longer, but the parameter lists are shorter, so it should be okay. (This solves Problem 7.)

In this case, we would sacrifice extensibility (e.g. higher-order Bézier curves); in other words, we wouldn't try to solve Problem 8 of the original proposal.1 We’d also lose the simplicity of being able to call any vertex function an unlimited amount of times.

Separate APIs for chaining and aggregating

Here I'll add some issues to consider in relation to the idea you recently proposed.

  1. Ideally, we want an API that's easy to guess. Since segments are just one shape kind, a user may guess that in addition to beginSegment(), we'll have beginTriangleFan(), etc. Rather than expanding the size of the API significantly, one way to possibly address this is to replace beginSegment(interpolationStrategy) with beginPrimitive(kind). Then BEZIER_SEGMENT would be just one example of a shape kind. This isn't perfect, since it'd be nice if the primitive kinds would be the same as the shape kinds, but that'd require some extra complexity, e.g. beginPrimitive(vertexKind, shapeKind).
  2. It may be confusing to have two APIs, mainly because they don't seem entirely consistent. In the chaining API, the segment type is part of the function name, whereas in the other API, it's a parameter. Another way to think of this is that vertex() is just used for linear shapes in one API 2, but it's used for other kinds of shapes in the other API.

Combined API for chaining and aggregating

Below, I'll describe one approach that seems to do everything we want in a simple way. It's a variation of the API proposed by @peilingjiang. I'm not sure about it yet, but it seems really nice.

The idea is to have four vertex functions:

  1. linearVertices() for polylines
  2. bezierVertices() for Bézier curves
  3. catmullRomVertices() for Catmull-Rom splines 3
  4. arcVertices() for elliptical arc splines 4

As in the current version of p5.js, each function call creates a new segment (when the default shape kind is being used), and multiple calls will chain segments. There are two main differences from the current API:

  1. The vertex functions all accept a variable number of arguments, so that a single function call can create a polyline, a Bézier curve of arbitrary order, a Catmull-Rom spline through any number of points, or an elliptical spline.
  2. The arguments would be instances of a new p5.Vertex class. Vertex objects would have properties for position, texture coordinates, color, and a normal vector. 5 This would make it possible to support features like per-vertex coloring while retaining the benefits of @peilingjiang’s API. In particular, it makes code easier to read by explicitly grouping vertices into primitives, and it may allow for a simpler implementation that directly reflects the API.

As an initial example, here's how we could create a cubic Bézier segment:

let v1 = createVertex(...);
let v2 = createVertex(...);
let v3 = createVertex(...);
let v4 = createVertex(...);

beginShape();
bezierVertices(v1, v2, v3, v4) //vertices could also be created inline, with one argument per line
endShape();

//ALTERNATIVE
let vertices = [
  createVertex(...), 
  createVertex(...), 
  createVertex(...), 
  createVertex(...)
];

beginShape();
bezierVertices(...vertices); //could support array arguments if we want to avoid spread syntax
endShape();

This would be a slight departure from typical p5.js usage, which has users passing coordinates directly into functions, but it's similar to how we already allow users to pass a p5.Vector object directly into point().

If we don't immediately see any major problems with this overall approach, I can work out more of the details (e.g. I can redo this list of examples in the new API).

Footnotes

  1. With this approach, it might still be possible to mostly solve Problem 9, e.g. by calling quadraticBezierVertex() six times for a quadratic Bézier triangle and calling cubicBezierVertex(x, y, z) ten times for a cubic Bézier triangle. Similarly, we could call quadraticBezierVertex() nine times for a biquadratic patch and cubicBezierVertex() sixteen times for a bicubic patch.

  2. Here, I'm ignoring the possibility of manually specifying normal vectors for vertices, since that's a less common use case.

  3. Although curveVertex() is a friendly name, in that "curve" is a common word, it's overly generic and inconsistent with the other names, which are more specific. This can lead to confusion. For example, right now, there's a section of the reference called “Curves,” and underneath, it has both bezier() and curve() commands; so according to the reference, a bezier() is a curve, but it's not a curve(). So we may consider renaming curveVertex() (or curveVertices()) to something less generic, like hermiteVertex(), catmullRomVertex(), or even splineVertex(). Although splineVertex() is still too general (e.g. one can also make Bézier splines), it’s more precise than the current name. This note applies to all three APIs considered in this comment.

  4. The idea is that we can interpolate points with elliptical arcs instead of Bézier curves or something else. I cited a couple papers on this previously: here's one interesting paper that shows how to create both Bézier and elliptical splines in a common framework, and another paper that focuses specifically on "blended elliptic arc" splines. If we were to adopt an approach like this, we'd still need to consider how easy it'd be to create a single ellipse or elliptical arc. Also, this would probably mean that we would not try to support curve fitting (e.g. for fitting a single arc or line segment to a large number of vertices), as discussed in Add arcVertex() for creating arcs with beginShape()/endShape() #6459, unless we provide a separate API for that.

  5. If this idea seems promising, we'll want to vet the idea of a p5.Vertex class more carefully. One issue we've come across before is that we may end up with some null properties, which may be a sign that we need to split the class up. However, a single p5.Vertex class would be consistent with p5's usual approach of providing a small interface to a large number of features (e.g. p5.Vector combines 2D and 3D vectors).

@davepagurek
Copy link
Contributor

Thanks @GregStanton! Here are a few thoughts on the different ideas you mentioned.

  • A chaining-only API
    • To me this is probably the smallest departure from the current API that still accomplishes many of the goals we've set out, so could be a good option to keep in our back pocket if other departures seem too large.
  • Separate APIs for chaining and aggregating
    • Using beginPrimitive to handle all those cases seems good to me.
    • I think I'm also ok with the tradeoff of having a slightly different pattern for curve vertices and higher-order beziers if the idea is that they're more advanced, less commonly used, and less clear how they would work in conjunction with the chaining API functions, given that this more advanced set of features has a pattern to it too. (I think the interplay between curveVertex and other vertex functions is pretty confusing to people currently, based on some Discord questions in the past.)
  • Combined API for chaining and aggregating
    • This does a nice job of supporting the things we want. I think we need to dig into createVertex though: if it takes positional arguments (e.g. createVector(x, y, z, u, v)) it becomes hard to supply just a color and not a texture coordinate, or something like that. We might need it to be createVector(x, y, z, options) where options can be { u: 0, v: 0 } and only fill in the keys that are needed.
    • I wonder if people who teach with p5 have any opinions on the teachability of this sort of API, since it uses a few more JS concepts compared to the current one. @nickmcintyre do you have any thoughts?

@nickmcintyre
Copy link
Member

@davepagurek I have some initial impressions but haven't had the opportunity to teach this part of API yet. I'll share some thoughts soon.

@tabreturn you know much more about teaching curves than I do. What do you think about the proposal?

@tabreturn
Copy link
Contributor

I'll need to make some time to study this properly -- but it looks interesting!

@davepagurek davepagurek self-assigned this Jun 18, 2024
@davepagurek
Copy link
Contributor

The 2.0 advisory committee was looking at all the proposals, and the feedback on this one is that the most important goal here is to increase simplicity and predictability, and the larger departures from the existing API end up complicating things too much to be worth it. So I think to move forward with this one, the best approach would be to narrow the focus and pick out the bits that offer incremental improvements to just the current p5 shape functions.

In this case, I think that means going back to the chaining-only API described above. To summarize, from the current API:

  • Bezier and quadratic vertex commands are split up into individual control points to improve readability, allow the addition of uv coordinates more easily, and allow fill(), stroke(), normal(), etc to be changed per vertex by interleaving function calls
    • If we want to keep the existing API as an option, we can use a different suffix for the control points, e.g. bezierControl (as in control point) or bezierHandle (as in handle, borrowed from the handles in vector editors)
  • Although it's still not required, it isn't an error to have beginContour()/endContour() around the first path in a shape -- it just denotes a distinct, disconnected part of the shape. This just makes it align more with how native shape drawing works, and makes easier to handle sub-paths (e.g. when drawing a font from data) without special casing the first contour.

This compromises the original proposal's side goal of allowing higher order Beziers and different interpolation schemes. I think if we want, we can extend the above to cover some of those other points by adding a curveMode() call, which (1) can be used to specify the start of a new set of curveVertex points if one wants to split a long chain into two, and (2) lets us add other interpolation modes one might use, e.g. BEZIER to interpret the vertices as a single higher order Bezier.

@GregStanton
Copy link
Collaborator Author

GregStanton commented Aug 23, 2024

@davepagurek Thanks so much for sharing the feedback!

After reading it, I had a new idea that I'm excited about. I think it's consistent with the feedback, in spirit: it preserves the simplicity and predictability of the original API proposal, with few changes to the current API. It also happens to be very extensible. Actually, it solves all ten of the original problems, and it addresses the main flaw in the original proposal.

I’d like to make more code examples to vet the idea further, but I’m hoping to get some initial feedback first. The API is indicated below, along with a code example, and some additional context.

API

// mode function (could default to n = 3, with native support for n = 2 and 3, at least)
bezierOrder(n)

// vertex functions (brackets indicate optional parameters)
vertex(x, y, [z], [u], [v])
arcVertex(x, y, [z], [u], [v])
bezierVertex(x, y, [z], [u], [v])
splineVertex(x, y, [z], [u], [v])

Example

beginShape();

// two quadratic Bezier curves
// explicit starting vertex is required for first segment in a shape

bezierOrder(2);
bezierVertex(x0, y0);

bezierVertex(x1, y1);
bezierVertex(x2, y2);

bezierVertex(x3, y3);
bezierVertex(x4, y4);

// one cubic Bezier curve
// implicit starting vertex is at x4, y4

bezierOrder(3);
bezierVertex(x5, y5);
bezierVertex(x6, y6);
bezierVertex(x7, y7);

endShape();

Detailed benefits

This API

  • solves all ten of the original problems
  • preserves the simplicity of the original proposal
  • removes the need to replace bezier() with quadraticBezier() and cubicBezier() (Problem 6) 1
  • eliminates naming ambiguity in quadraticVertex() and bezierVertex() (Problem 7), without longer function names
  • fixes the segment disambiguation flaw of the original proposal, without extra boilerplate per segment

It also solves an additional problem not identified in the original proposal's list of ten problems:

  1. The name curveVertex() uses a general term for a special shape. For example, in the p5 reference, “Curves” is used as a general section heading that includes Béziers, while curve() specifically creates Catmull-Rom splines only. This may lead to confusion.

The term "spline" is more specific, while also being general enough to allow other types of splines in the future, e.g. via a function such as splineType() (either natively or via an add-on library). If we were to use a function name like curveType(), the two uses of "curve" would make it harder to guess its purpose.

Note on arcVertex()

I’ve added arcVertex() to this version of the API, since it looks like we are going to add it. The syntax for arcVertex() that’s indicated here is different from the syntax originally proposed in #6459, but this change was discussed in the comments on that issue. A benefit is that it would make arcVertex() more consistent with the other vertex functions. Just as every four cubic Bézier vertices form a segment, every five arc vertices form a segment. However, we haven’t yet discussed the inclusion of texture coordinates in arcVertex().

Possible next steps

If this idea looks good overall, we could...

  1. Discuss how texture coordinates would work with arcVertex(). (We could do that over in Add arcVertex() for creating arcs with beginShape()/endShape() #6459.)
  2. Write mock documentation to flesh out the design, with code examples (e.g. from this comment).
  3. Invite the community to help flesh out use cases and provide feedback.
  4. While collecting feedback, work on Refactor vertex functions to enable composite paths #6560.

Footnotes

  1. By making n = 3 the default order, bezier() would already make Bézier curves of the default order. For non-default orders, users would still be able to use the more customizable beginShape()/endShape() and bezierVertex().

@davepagurek
Copy link
Contributor

I'm a fan of that proposal!

In your example, do you think that you can start with a bezierVertex() and then add two more to make a cubic bezier segment? Or would it instead be a little closer to the current API, where you start with a vertex() and then add two bezierVertex()es after that? I think I'm actually a bit more inclined to use the latter, because that means that a cubic bezier always involves adding two more calls to the last point to add a new segment, whereas it might be harder to determine where segments start/end if it can maybe also start with one.

@GregStanton
Copy link
Collaborator Author

GregStanton commented Aug 25, 2024

I'm a fan of that proposal!

Yay! Also, you've identified the exact issue that's been on my mind. It seems to be the only remaining detail to sort out (famous last words haha). For anyone who is following along, I'll describe it below, and I'll discuss a few ways we might proceed.

Explicit-first-vertex issue

In the version of the API under discussion, Bézier curves of the same order can require different numbers of function calls. For example, when a Bézier segment of order 2 (quadratic) is chained onto an existing segment, it requires two function calls, which is simple for users to remember; however, when it's the first segment in a shape, it requires three function calls.

Explanation

This issue arises because a Bézier curve of order n always requires n+1 vertices $V_0, V_1, ... , V_n$, but $V_0$ needs to be specified explicitly only when the curve is not chained onto a previous segment.

A few options for dealing with the explicit-first-vertex issue

I'll describe the options presented so far, along with one new option. I'll also indicate costs and benefits of each option.

1. No change

It'd be nice if a Bézier curve of order n always required n calls to bezierVertex(), but the proposed API does have a kind of consistency: a new Bézier curve of order n always requires n new Bézier vertices to be added to an existing vertex.

Also, this pattern for reusing vertices is precisely consistent with existing vertex features (triangle fans, triangle strips, and quad strips use the same pattern). For example, when using the TRIANGLE_STRIP shape kind with beginShape(), the first triangle requires three vertex calls, but thereafter, only one vertex call is required to add the next triangle, since the next triangle reuses two vertices from the previous triangle. This is the same idea as requiring three Bézier vertices for the first Bézier curve and requiring only two Bézier vertices for the next curve, since the next curve reuses a vertex from the first curve.

2. Use vertex() for an explicit first vertex

beginShape();

// two quadratic Bezier curves
// explicit starting vertex is required for first segment in a shape

bezierOrder(2);
vertex(x0, y0);

bezierVertex(x1, y1);
bezierVertex(x2, y2);

bezierVertex(x3, y3);
bezierVertex(x4, y4);

// one cubic Bezier curve
// implicit starting vertex is at x4, y4

bezierOrder(3);
bezierVertex(x5, y5);
bezierVertex(x6, y6);
bezierVertex(x7, y7);

endShape();

It might be reasonable to use vertex() to specify an explicit first vertex when an implicit one is not available, since that's how the existing p5 API works. My concern is that this introduces other inconsistencies:

  1. We could potentially use vertex() to start bezierVertex() and arcVertex() segments when an implicit vertex is not available, but it may be confusing for splineVertex() to work this way. If splineVertex() works differently, we'd reintroduce the inconsistency identified in Problem 3 of the original proposal.
  2. In the new API that we're discussing, each type of vertex function does one thing: vertex() makes shapes with straight sides, bezierVertex() makes Béziers, etc. If we mix commands by using vertex() to create part of a Bézier curve or arc, then the API will be less predictable (a newcomer seems less likely to predict that a Bézier curve needs to start with vertex(), followed by bezierVertex(), for example).
  3. Using vertex() with multiple types of curves may complicate its documentation.

3. Introduce bezierAnchor() and arcAnchor()

beginShape();

// two quadratic Bezier curves
// explicit starting vertex is required for first segment in a shape

bezierOrder(2);
bezierAnchor(x0, y0);

bezierVertex(x1, y1);
bezierVertex(x2, y2);

bezierVertex(x3, y3);
bezierVertex(x4, y4);

// one cubic Bezier curve
// implicit starting vertex is at x4, y4

bezierOrder(3);
bezierVertex(x5, y5);
bezierVertex(x6, y6);
bezierVertex(x7, y7);

endShape();

The names bezierAnchor() and arcAnchor() don't have "vertex" in them, which is good, since these functions are different: they're only ever called to explicitly identify an anchor (i.e. a starting vertex $V_0$), at the beginning of a shape or contour.

Some benefits:

  • A Bézier curve of order n would always be created with n calls to bezierVertex() (as in Option 2).
  • An arc would always be created with five calls to arcVertex() (as in Option 2).
  • Neither of the inconsistencies of Option 2 would be introduced.
  • The anchor functions specify the type of primitive that's being created, which may simplify the implementation.

Some disadvantages:

  • Adding two new functions would increase the size of the API.
  • Users would need to remember that Béziers and arcs must be started with special anchor functions when they're the first curves in a shape.
  • Since "anchor" is sometimes used to refer to both the first and last vertices of a Bézier (e.g. a quadratic Bézier is sometimes said to have two anchor points and a control point), I'm not sure about the name. My initial sense is that these terms aren't totally standardized, so it's probably okay, but maybe there's an alternative name that'd be better.

Thoughts?

Update: I refined the language and reasoning in Options 1, 2, and 3, e.g. by specifying additional disadvantages.

@davepagurek
Copy link
Contributor

I'm still thinking about what other options there might be, but in the mean time, one comment on this bit:

We could potentially use vertex() to start bezierVertex() and arcVertex() segments when an implicit vertex is not available, but it may be confusing for splineVertex() to work this way. If splineVertex() works differently, we'd reintroduce the inconsistency identified in Problem 3 of the original proposal.

I'm imagining that each type of shape drawing function continues off of the last point, using its parameters to define how that continuation happens.

The native drawing context arcTo also works kind of like this, where it adds a straight line segment to the start to make sure it's connected (blue is the initial "vertex", red is the control points of the arc):

ctx.moveTo(50, 50) // Analogous to `vertex` here
ctx.arcTo(50, 150, 150, 150, 40);
ctx.stroke()

image

It's a little different for spline vertices because they could be used without continuing on from something else, which is the current inconsistency. Here's how that currently renders (the curve vertex points drawn in red for illustration):

beginShape()
curveVertex(50, 50)
curveVertex(50, 100)
curveVertex(50, 150)
curveVertex(100, 150)
curveVertex(150, 150)
endShape()

image

If we were to make it consistent and require you to add a regular vertex to the start, I'd expect it to still draw the same shape that we currently do, but also connect it to the last vertex somehow. Some options for that:

Straight line: still as confusing as current curveVertex because of the non-drawn control points, but at least predictable

beginShape()
vertex(100, 50)
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
endShape()

image

Interpolated start point: under the hood, it places two catmull-rom points at the start so that it smoothly goes from the previous point through to the rest of the curve. This makes the end behaviour a little inconsistent with the start, which leads me to the next row.

beginShape()
vertex(100, 50)
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
endShape()

image

Interpolated start and end point: under the hood, it places two catmull-rom points at the start and the end so that it goes through every point. This would make it consistent with how bezier points work and might be more obvious to newcomers, but also makes it hard to make something like a closed catmull-rom shape without a cusp. (That could maybe be fixed with a separate spline mode for smoothly closed shapes?)

beginShape()
vertex(100, 50)
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
endShape()

image

@GregStanton
Copy link
Collaborator Author

GregStanton commented Aug 27, 2024

@davepagurek: Thanks so much for fleshing that out! It seems like you're moving toward making splines work more like polylines, which would be great for usability. It also makes sense, since polylines really are linear interpolating splines.

This got me digging into some different options, and it seems like we could maybe take this a step further. I need to develop this idea more, but I'll share what I'm thinking so far.

Making splines as intuitive as polylines

As an illustration of what I have in mind, you can try tinkering with this online demo. Try setting "Curve ends" to "Show," delete the default curve, and start adding control points one by one. You can leave the interpolation function on hybrid mode. The interface takes a little getting used to since points are added in random places, and you need to hover over the canvas to reveal the control points, but anyway...

The cool thing is that this demo allows the user to specify a smooth spline with just two vertices or more, and they're all rendered (as long as "Curve ends" is set to "Show"). With this kind of approach, making splines would be about as intuitive as making polylines! This would be simpler than the interpolated-start-and-end option in your last diagram, since that would still require the user to supply four vertices, and I think it'd be less clear why we need four vertices.

Note: This is a JavaScript and WebGL implementation of a new class of C^2 interpolating splines with some very nice properties; I'd be happy to share what I've learned about it (the researcher behind this is also an expert and seems available, e.g. via X). We may want to consider this new class for our implementation, but it may also be possible to come up with something similar without changing p5's implementation much.

Modes

While it may be nice to render all control vertices by default, the user would have less control if that were the only option. Perhaps we could support several modes, to be specified with a splineEnds() function:

  1. SHOW: This could be the default and would work like your last case (interpolated start and end points).
  2. HIDE: This would work more like curveVertex() does in the current version of p5, when more control is needed.
  3. JOIN: This would create a loop.

These would correspond to "show," "hide," and "loop" in the online demo. However, there are some possible problems with this:

  • HIDE mode is basically what p5 has now, so it may not work well with a vertex()-first API (i.e. where every type of segment starts with a call to vertex()). I do think it'd work if we keep the the latest proposal as is.
  • There's already a way to close contours and shapes in p5. Would it be confusing to add a different way of closing a spline? I haven't thought about this yet.

@davepagurek
Copy link
Contributor

davepagurek commented Aug 27, 2024

I think that paper makes for a good curve mode, as maybe would Hobby curves. I think all of them have pros and cons, so having a curveMode that we can add to makes sense to me.

This would be simpler than the interpolated-start-and-end option in your last diagram, since that would still require the user to supply four vertices

I'm imagining users won't need to specify four vertices to use Catmull-Rom if we want to modify it to always go through endpoints, since the duplication would happen under the hood. This would look something like this to end users (click to add points): https://editor.p5js.org/davepagurek/sketches/kVff25GyA In this way I think it would be interchangeable with the C^2 spline algorithm or Hobby curves.

I think the endpoint modes make sense! If using the HIDE mode and you add more vertices before or after, we could connect them with straight line segments like in the example below to maintain the behaviour where every contour keeps making a connected line.
image

For joining: definitely a little weird that there's a separate CLOSE, but possibly not the end of the world if we don't end up coming up with something better, as I think it still helps us cover a pretty common usage of smooth curves that is currently pretty hard to explain. I imagine if you have a closed loop in the middle of a path and you add more vertices afterwards, it would just continue off of the closed endpoint?

Also, here's a little implementation of how loop-closing Catmull-Rom with interpolated endpoints would look: https://editor.p5js.org/davepagurek/sketches/36Ro7HpR- Both this and Hobby curves are still cubic beziers under the hood, so it's maybe helpful for implementation that these strategies could both be implemented as translations or generators for a bezier path. (Edit for a little more context: other interpolation methods that aren't translatable to cubic bezier are still doable, it maybe would maybe just require us to convert it to a polyline for 2D mode to render it there. We'd be doing that anyway for WebGL mode so that's not a huge downside.)

@davepagurek
Copy link
Contributor

Anyway before getting too much more in the weeds with curveVertex options, a next step here might be to try to summarize the updated proposal and get some feedback on that again?

@GregStanton
Copy link
Collaborator Author

Thanks @davepagurek!

I went ahead and incorporated all the important updates into the original proposal. I included a section at the end to indicate which updates were made. I agree that gathering more feedback would be a good next step!

Also, your diagrams and sketches are all really helpful. I think that makes sense about Catmull-Rom not requiring four vertices if we're interpolating the control points at the beginning and end; thanks for the clarification. Your comments did give me some extra ideas about splines, which I've shared below.

Also, especially now that I've summarized the latest proposal, I have a couple extra points on the overall API that I'm curious to get your feedback on. I'll start with those, since they seem more fundamental.

Overall API: Predictability and Simplicity

  1. Predictability and simplicity: Since the committee's main goal is to increase predictability and simplicity, I'm starting to think that the no-change option may be the best option: if someone wants to make a spline, Bézier, or arc, then they'd likely predict that they should use splineVertex(), bezierVertex(), or arcVertex(), rather than starting with vertex() or anything else. This seems like an important point, which @tfadali pointed out to me in a real-time discussion (he's interested in this work, so I've been getting him up to speed).
  2. Existing reuse pattern: With the no-change option, the pattern for reusing vertices is precisely consistent with existing vertex features (triangle fans, triangle strips, and quad strips use the same pattern). If we're okay with that, then I guess the same situation would be acceptable with curves? Actually, although it's not as salient, vertex() already works the same way for making polylines: two vertices are required to make the first line segment, and thereafter, only one vertex is needed to generate a new segment.

Splines

I'll reply to a few points about splines as well, in case it helps us make progress.

Type modes

Yeah, I was thinking we could potentially have a splineType() or something similar :) I'm holding off on the details for now in order to focus on splineEnds() (or whatever we call it).

End modes

I wanted to reply quickly, so hopefully my reasoning makes sense in this section. If I'm thinking incorrectly, please let me know!

Hidden vertex()

In the version of HIDE mode you mentioned, how would you create the curve in the picture below with no blue vertex (and no line segment connecting to the blue vertex)?

image

In the vertex()-first approach, I'm imagining this code:

beginShape()
vertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
endShape()

It feels strange for vertex(50,50) to create a non-interpolated control point at the start of a shape, even in HIDE mode, since HIDE mode applies to the spline, whereas using vertex() makes it feel like the starting point isn't a part of the spline. Also, vertices at the start of a shape would be interpolated in every other situation. The possible confusion seems to be compounded by specifying one non-interpolated control point with vertex() and the other with splineVertex().

This kind of issue is another reason that I seem to be leaning toward the no-change option (i.e. leaving the usage proposed in the updated proposal as is, rather than using vertex() to start shapes).

Unintended consequence?

Based on the latest approach you shared, it seems like we'd get the following output in the two modes.

SHOW HIDE
image image

But, in SHOW mode, it seems like we'd want the following instead:

image

P.S. It seems like the JOIN case will take some more time to think through. I'm looking forward to working on that when I get a chance.

@davepagurek
Copy link
Contributor

Starting vertex

I'm ok with the idea that some things don't need to always start with a vertex, like the strip modes, and also splines. I think the consistent rule I'm imagining is that each new segment is responsible for transitioning from the last drawn point (not control point), if it exists, and that still holds for splines if they don't start with a vertex (I'll show examples in the next section.)

I think my concern for bezier segments is that it's pretty rare to just have one Bezier segment compared to a chain of many of them. In that case, the concern that it's hard to tell segments apart in the code becomes the primary issue. I worry that if there isn't consistency (n calls always adds a new segment) then splitting a segment into multiple control points becomes too confusing, and that's the primary change that I personally am hoping to get out of the refactor. Forcing it to start with a vertex still keeps that rule above consistent: the difference between it and a spline is that splines can support arbitrarily many points in them, whereas non-splines always have a fixed amount (so I'd imagine arcVertex works the same, and would also need to start with a vertex.)

Splines

Slight aside, but I think spline ends only apply to some splines, like Catmull-Rom or something like B splines, so if we do add something like C^2 interpolating curves or Hobby curves, the HIDE mode may just not apply to them. That's probably fine, curveTightness would maybe not apply to everything either. I also wonder if renaming SHOW to TOUCH or INTERPOLATE or some synonym might make sense (especially if we make that the default so you don't have to actually type it most of the time) to make it clear it changes the curve? Maybe INCLUDE?

Anyway here's how I was imagining the end modes would work, keeping consistent that rule above:

beginShape()
splineEnds(HIDE)
// With no surrounding vertices
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
endShape()

image

beginShape()
splineEnds(HIDE)
// With starting vertex
vertex(100, 50)
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
// ...and ending vertex
vertex(150, 100)
endShape()

image

beginShape()
splineEnds(SHOW)
// With no surrounding vertices
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
endShape()

image

beginShape()
splineEnds(SHOW)
// With starting vertex, the spline is responsible
// for a smooth transition from there into the spline
vertex(100, 50)
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
// ...but the vertex call is responsible for transitioning
// out of the spline, so it's not smooth
vertex(150, 100)
endShape()

image

beginShape()
splineEnds(SHOW)
// With starting vertex
vertex(100, 50)
splineVertex(50, 50)
splineVertex(50, 100)
splineVertex(50, 150)
splineVertex(100, 150)
splineVertex(150, 150)
// Bringing the last vertex "into" the spline ensures a smooth transition:
splineVertex(150, 100)
endShape()

image

@GregStanton
Copy link
Collaborator Author

GregStanton commented Aug 29, 2024

Thanks for all the examples @davepagurek!

Starting vertex

In my last reply, I think I could've been clearer when I tried to answer your original question:

[To start a shape with a quadratic Bézier], do you think that you can start with a bezierVertex() and then add two more...? Or would it instead be a little closer to the current API, where you start with a vertex() and then add two bezierVertex()es after that? I think I'm actually a bit more inclined to use the latter, because that means that a cubic bezier always involves adding two more calls to the last point to add a new segment, whereas it might be harder to determine where segments start/end if it can maybe also start with one.

I'm totally empathetic to your position! I originally had the same concern. But, based on all the considerations so far, I'm actually proposing the former option... For the record, I'll try to clarify my current reasoning with an example. 1

Example

Quad strip (current usage) Bezier curves (proposed usage)
A quad strip consisting of three adjacent quadrilaterals. A chain of three quadratic Bézier curves, with their control points.
beginShape(QUAD_STRIP);

// first quad
// four vertices required
vertex(30, 20);
vertex(30, 75);
vertex(50, 20);
vertex(50, 75);

// one more quad
// two vertices required (two reused)
vertex(65, 20);
vertex(65, 75);

// one more quad
// two vertices required (two reused)
vertex(85, 20);
vertex(85, 75);

endShape();
beginShape();

// first quadratic Bezier curve
// three vertices required
bezierOrder(2);
bezierVertex(5, 12);
bezierVertex(41, 12);
bezierVertex(23, 30);

// one more quadratic Bezier curve
// two vertices required (one reused)
bezierVertex(5, 48);
bezierVertex(41, 48);

// one more quadratic Bezier curve
// two vertices required (one reused)
bezierVertex(182, 54);
bezierVertex(41, 57);

endShape();

Reasoning

With strips and fans, we've already accepted that the first piece of a shape will require more vertices than the remaining pieces. If we can accept that for strips and fans, then I think we could accept it for curves. This has several benefits:

  1. Predictability: Users likely expect to start an arc with arcVertex(), rather than vertex(). Same for other curve types.
  2. Consistency: We'd have the same pattern of vertex reuse across all vertex functions and all shape kinds.2
  3. Simplicity: We can create Bézier curves using only bezierVertex(), arcs using only arcVertex(), etc.

We do lose the simplicity of always calling bezierVertex() exactly n times to create a Bézier curve of order n. However, there is still a kind of consistency: a new Bézier curve of order n always requires n new Bézier vertices to be added to an existing vertex. So on the whole, the current API seems to be a net gain for simplicity and predictability.

Splines

I was actually thinking about whether the end modes would only apply to some types of splines. Thanks for putting that on record. I have an initial idea of how we could handle that, but it's not ideal. I was thinking we could hash that out once the basics are in place.

I do think end modes would apply to the $C^2$ interpolating splines, fortunately. The "Show" and "Hide" modes in the online demo of those splines were actually my inspiration for suggesting different end modes. I still need to look more closely at Hobby curves.

Real-time discussions?

For the rest of the issues around splines (names for the modes, etc.), maybe it'd make sense to set up one or more video calls at some point? In order to keep my GitHub replies from being super verbose, I've ended up omitting ideas or not communicating them clearly enough. Also, this discussion is getting long :) I usually prefer total transparency for anyone following along, but it might actually be easier for everyone if we just post summaries of real-time discussions?

Footnotes

  1. In the Bézier example, control points are rendered as a visual aid, but the code for that is not shown.

  2. As we've been discussing, the spline case is special and may support more advanced variations, but it's possible for this exact pattern to hold in the default case (e.g. if the default spline type is Catmull-Rom, with interpolated start and end points).

@Qianqianye Qianqianye moved this from Proposal to Implementation in p5.js 2.0 Sep 10, 2024
@GregStanton
Copy link
Collaborator Author

GregStanton commented Sep 14, 2024

Hi @nickmcintyre and @tabreturn! The last time you were pinged on this issue, we were in the midst of a fairly complicated discussion. Since then, we've ironed out the main issues, and I've updated the original proposal to summarize the key points. I'm hoping it's easier to digest now. If you have any feedback, that'd be amazing!

Also, @peilingjiang, the proposal now includes a function bezierOrder() that you might be interested to see. Thanks for your feedback so far!

@davepagurek
Copy link
Contributor

Thanks @GregStanton! @shiffman @stalgiag @outofambit, if any of you are interested and have the time, the issue description at the top has been updated with a new proposal. Since the last time you read it, we've reduced the scope of the changes, and they now mainly affect bezier and curve vertices, described in the "What's the solution?" section.

@davepagurek
Copy link
Contributor

  • However, the proposal can be implemented without breaking changes, at a small cost.

Just wanted to add an update about this footnote in the issue description! Greg and I took a look at our shape drawing APIs, and as he mentioned, by looking at the number of arguments, we can distinguish between 1.x's bezierVertex() and also the new proposed bezierVertex() that works with bezierOrder().

  • If we want to, we could build that directly into p5 and continue to support the old APIs (maybe marking them as deprecated so they don't clutter the docs too much.)
  • Alternatively, we're already planning on having a 1.x compatibility addon, so we could just implement it there instead of in core.

I'll be implementing it in one of those two spots at least, so we only need to decide whether the old API should remain a core feature or not.

We also did some checks to see how much would be affected by our breaking changes. We found:

  • None of the Nature of Code examples will be affected by these API changes!
  • We found very little use of the changed methods compared to single standalone curves like bezier().
    • Out of the few curveVertex() uses we saw, most of them were also just demos trying to visualize how it works with the control points you give it. Possibly good reason to change the default behaviour there actually, if everyone feels it needs visualization for it to make sense

@shiffman
Copy link
Member

Sounds amazing! @davepagurek thank you for being so thoughtful about checking the Nature of Code examples ❤️ ❤️ ❤️

@nickmcintyre
Copy link
Member

The main idea is to pass only one vertex into each vertex function.

This feels like a big step in the right direction! The examples @GregStanton shared seem like they'd be much easier to teach -- maybe draw the curve, then draw a point() at each vertex, taking care to add them in the same order.

@davepagurek the 1.x compatibility addon seems perfect for making the old API available for awhile.

@GregStanton
Copy link
Collaborator Author

Thanks so much for taking a look @nickmcintyre! That's a good observation about point(). It takes coordinates for a single point as input, just like the proposed vertex functions, so it'd be easier to adapt curve-drawing code for the purpose of rendering individual vertices. Another option would be to use the POINTS shape kind, together with vertex().

(Eventually, we might even consider making POINTS have the same effect when used with other vertex types, in which case any type of vertices could be rendered simply by changing the kind parameter. But that would require some more thought... For Béziers, would users expect only the interpolated vertices to be rendered? Also, in the current version of p5, none of the shape modes are intended for use with anything but vertex(). If we have the other vertex types support POINTS, users may expect they'd support all shape kinds. This may indeed be possible, e.g. bezierVertex() could make Bézier triangles and quads—this is one of the benefits of the proposal. However, we'd still need to consider how splineVertex() would work with the other values of kind.)

@GregStanton
Copy link
Collaborator Author

Quoting this comment from @golanlevin below, for the record. Here, an add-on library author acknowledges possible breaking changes and writes that the new API "is great" :)

@GregStanton My SVG-export library is now public: https://github.com/golanlevin/p5.plotSvg

I'm keenly aware that the new vertex-function API (which is great) for p5 v.2 will cause some breaking changes in my library. (Is testable code available somewhere, or is it still in the specification-design stage?)

@golanlevin
Copy link

Hi all, just chiming in to say that the vertex-functions proposal looks very thoughtfully conceived overall — and I look forward to adjusting my new SVG-exporting addon library, https://github.com/golanlevin/p5.plotSvg, as appropriate, in due time.

I'm glad to learn of the 1.x compatibility addon for lots of reasons, this is thoughtful. But relative to p5.plotSvg, my library is quite new and it doesn't yet have a large user base. When 2.x comes out I'll probably just shrink-wrap a 1.x-compatible version of p5.plotSvg and start a new branch.

@GregStanton
Copy link
Collaborator Author

A new splineEnds() function has been added to the API.

Also, @davepagurek and I continued the discussion about the explicit-first-vertex issue, and we settled on leaving the proposal as it currently stands. Specifically, Béziers start with bezierVertex(), splines start with splineVertex(), and arcs start with arcVertex(), whenever they're the first segment in a path. (This addresses Problem 3 about mixed commands, in the original proposal.) When any one of these is chained onto another segment, it'll start with a vertex from that segment.

@GregStanton
Copy link
Collaborator Author

GregStanton commented Dec 12, 2024

Before we lock in the names of the modes for splineEnds(), it'd be nice to get some feedback.

Problem

As @davepagurek pointed out, the names SHOW and HIDE may be somewhat inaccurate. For example, SHOW might just suggest that we're drawing the endpoint but not connecting it to the curve. In reality, these modes affect whether a spline connects to its endpoints, not whether the endpoints by themselves are shown.

Solution?

Dave previously suggested INCLUDE. So how about INCLUDE and EXCLUDE? I like these names. They're fairly common words. Also, given one mode, it's easy to guess the other. They also seem more accurate than SHOW and HIDE. "Include" suggests that the endpoints are included within the path as a whole, not that they're drawn in isolation. "Exclude" is perhaps slightly less clear without context, since it might mean that only the endpoint itself is excluded, when in reality we're excluding a curve that connects the point to the rest of the spline.

Less appealing alternatives

I haven't come up with anything better. I don't think INTERPOLATED and NON-INTERPOLATED work as well for beginners, and the hyphen could easily get confused with an underscore. Another option is TOUCH, but that reminds me of a curve that touches a tangent line, and it's hard to come up with an antonym for the other mode. I used the word "connect" at the start of this comment, but CONNECT suggests the ends are connected to each other, which isn't right. (We're handling that case by passing CLOSE to endShape() or endContour().)

Future vertexMode()?

Changing SHOW/HIDE to INCLUDE/EXCLUDE would also allow for a mode that shows (draws) a point at each position that's been specified to a vertex function. That could be done, for example, via a vertexMode(mode) function that takes SHOW or HIDE modes (these mode names would make sense in this case). This might be paired with a vertexSize() function that could be used to set the size of all vertices at once, or by calling it multiple times, to set the size of individual vertices. The default mode would be HIDE, which would disable any vertexSize() commands (analogous to how noStroke() effectively disables strokeWeight() commands).

Actually, @nickmcintyre already mentioned that rendering the vertices themselves could be helpful for teaching, and this could be a good way to do it. It'd be similar to debugMode() for adding visual guides that help with making 3D sketches (analogous to helpers like GridHelper and AxesHelper in three.js). But discussing such a feature would probably require a separate issue. For example, what about vertexStroke(), vertexFill(), and maybe noVertexStroke() and noVertexFill()? Maybe users would expect these, and maybe adding this many functions to the API wouldn't be worth the convenience of having a function like vertexMode().

@davepagurek
Copy link
Contributor

I like INCLUDE/EXCLUDE! I agree that EXCLUDE is a little less easy to understand, but maybe it's still a better overall compromise. Visualizing vertices with points could be a cool extension, so it's nice that this API would enable that.

@davepagurek
Copy link
Contributor

This will be in 2.0 as of #7373! @GregStanton do you think I should keep this issue open for a bit for more discussion + any follow up work?

@GregStanton
Copy link
Collaborator Author

Thanks @davepagurek! Yeah, I do think it'd be good to keep this open in case we can get a bit more feedback on the latest changes. I just read over all the discussion again, and I think the key points that remain are below.

This issue

There are two points that would be nice to discuss further.

INCLUDE/EXCLUDE

I think these names for the spline-end modes are probably good. It'd be nice to get a bit more feedback before locking them in. I think we've ruled out the following alternatives:

  • INTERPOLATED/NON-INTERPOLATED
  • SHOW/HIDE
  • CONNECT/DISCONNECT

New alternative names would be great to see, in case we can improve upon the current names.

splineType(type, [properties])

One aspect of the API that's not entirely extensible is the use of splineEnds() and splineTightness(), which may not apply to all types of splines. If we were to add a new type of spline in the future that isn't based on tightness, for example, then splineTightness() would be confusing.

A more general alternative like splineType(type, [properties]) may be preferable, as it'd allow different spline types to specified, each with their own special properties. However, this would only make sense if we support at least one other type of spline, in addition to Catmull-Rom.

If we can add in one other spline implementation for p5.js 2.0, then we'd allow for other types of splines to be added in the future, without having to deprecate and eventually remove splineEnds() and splineTightness() in p5.js 3.0. This would also pave the way for a complementary fittedVertex()/fitType() feature (see below).

Future issues

There are a few other ideas that arose from this issue, including some new ideas I just had, but I think these would be suitable for separate issues. They don't involve breaking changes and could be added after 2.0. They're listed below, as a reminder:

  • Consider supporting p5.Vector arguments in bezier() (and possibly other functions), for more readable parameter lists
  • Discuss adding support for n $> 3$ in bezierOrder(n)
  • Discuss vertexMode() for rendering individual vertices
  • Consider fittedVertex()/fitType() for approximate smoothing curves, to complement splineVertex()/splineType() for exact interpolating curves

I suspect curve fitting would go in an add-on library, at least initially. However, since we did discuss curve fitting in the current issue, I thought I'd mention fittedVertex() or simply fitVertex() as a solution that might fit well with the API we've developed (no pun intended!).

@nickmcintyre
Copy link
Member

+1 for INCLUDE/EXCLUDE! I think y'all found a nice balance between accuracy and clarity. Just for discussion's sake,ATTACH/DETACH might sidestep some of the ambiguity of CONNECT/DISCONNECT.

It'd be helpful to see a few quick examples of splineType() in use with existing Catmull-Rom splines. We may not need to fully implement another type of spline for 2.0 in order to determine whether splineType() feels both beginner-friendly and general-purpose.

As for vertexMode(), I'd love to see this and related features evolve for awhile in an addon library -- maybe a little toolkit for designing parametric curves and surfaces.

@GregStanton @davepagurek FWIW I appreciate the master class you two led while working on this part of the API 🙏🏽 Can't wait to use it!

@GregStanton
Copy link
Collaborator Author

GregStanton commented Dec 22, 2024

Thanks so much for the thoughtful reply and kind words @nickmcintyre! That's a great point about continuing the discussion of splineType() even if we aren't implementing more than one type of spline for p5.js 2.0. We'd at least have a framework so that add-ons can seamlessly implement other types of splines.

A revised API for spline settings

Below, I've included some examples of what user code could look like, for a sample of spline types, and two possible APIs. In either case, vertices would be specified with splineVertex() as in the main proposal.

For now, I'm using INCLUDE/EXCLUDE for the ends of a spline, when appropriate. I actually was going to use Nick's suggestion of ATTACH/DETACH in the examples below, but I guess users could still think the ends are attached to each other, as with CONNECT/DISCONNECT?

Spline type `splineType()` `splineMode()` Notes
Catmull-Rom
splineType(
  CATMULL_ROM, 
  {tightness: 0, ends: INCLUDE}
)
splineMode({
  type: CATMULL_ROM,
  tightness: 0,
  ends: INCLUDE
})
Default mode in p5.js 2.0.
Hobby
splineType(
  HOBBY, 
  {tension: 1, curlStart: 1, curlEnd: 1}
)
splineMode({
  type: HOBBY,
  tension: 1, 
  curlStart: 1, 
  curlEnd: 1
})
Here's an explainer with a JS implementation. There's also a p5 implementation by @golanlevin. And here's one more JS implementation.
Basis (B-spline)
splineType(
  BASIS, 
  {degree: 3, knots: myKnots, weights: myWeights}
)
splineMode({
  type: BASIS,
  degree: 3,
  knots: myKnots,
  weights: myWeights
})
B-splines are supported in software such as Inkscape and Maya Creative. Here's a JS implementation that supports NURBS, and an implementation of the uniform case that's optimized for performance. Knots and weights are optional; weights make NURBS, which can represent conics exactly.
Yuksel
splineType(
  YUKSEL,
  {ends: INCLUDE, interpolator: HYBRID}
)
splineMode({
  type: YUKSEL,
  ends: ATTACH,
  interpolator: HYBRID
})
This is a class of non-polynomial interpolating splines that was introduced in 2020 by Yuksel. These haven't yet been given a name in the literature.

Beginner-friendliness: splineType() vs. splineMode()

In terms of beginner-friendliness, I'm not certain which is better, but I'm leaning toward splineMode(). Although splineType() may obviate the need to pass in an object when the user wants default settings, its syntax in general feels more complicated, and the name doesn't quite convey there will be an optional parameter in addition to the type.

Comparison to splineTightness() + splineEnds()

Compared to having separate splineTightness() and splineEnds() functions, the options in the table are much more extensible but a bit less beginner friendly, since they take object literals as arguments. However, p5 already uses object literals as configuration objects, as in createFrameBuffer(). Frame buffers and non-default spline modes are relatively advanced features, so it may be a reasonable place to introduce users to configuration objects, since they're useful and offer benefits like better readability.

Thoughts?

Does anyone have more thoughts about this? Any ideas @davepagurek, @ksen0, @golanlevin, @nickmcintyre?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Implementation
Development

No branches or pull requests

8 participants