Conversation
…gister in src/app.js
|
@davepagurek I’ve added the code for this. Let me know what you think or if anything needs to be changed! |
ksen0
left a comment
There was a problem hiding this comment.
Very exciting to see this! Added some high-level documentation comments. Non blocking
|
|
||
| # p5.svg Overview | ||
|
|
||
| `p5.svg` is an experimental native vector graphics system provided in p5.js starting from version 2. It aims to bring resolution-independent vector rendering, SVG file importing, and SVG XML exporting directly into p5.js without requiring external third-party addons. It allows users to record 2D drawing operations using familiar p5 APIs (`rect`, `circle`, `path`, `fill`, `stroke`, `translate`, `rotate`, etc.) and convert them into scalable vector structures. |
|
|
||
| `p5.svg` addresses several key goals: | ||
|
|
||
| - **Resolution-Independent Vector Output**: Traditional canvas rendering in p5.js is raster (pixel) based. `p5.svg` enables artists, designers, and educators to generate scalable vector graphics suitable for high-DPI displays, print, pen plotters, CNC routers, laser cutters, and embroidery machines. |
There was a problem hiding this comment.
"p5.svg enables artists, designers, and educators to generate scalable vector graphics suitable for high-DPI displays, print, pen plotters, CNC routers, laser cutters, and embroidery machines." Feels like it should be in the opening paragraph?
| `p5.svg` addresses several key goals: | ||
|
|
||
| - **Resolution-Independent Vector Output**: Traditional canvas rendering in p5.js is raster (pixel) based. `p5.svg` enables artists, designers, and educators to generate scalable vector graphics suitable for high-DPI displays, print, pen plotters, CNC routers, laser cutters, and embroidery machines. | ||
| - **Familiar p5 Drawing Workflow**: Rather than introducing a complex vector editing paradigm, `p5.svg` hooks directly into the existing 2D drawing pipeline. You can record shapes using standard p5 drawing functions inside `createShape()` or `buildShape()`. |
There was a problem hiding this comment.
I think this phrasing should be simplified, eg not "Rather than introducing a complex vector editing paradigm, p5.svg hooks directly into the existing 2D drawing pipeline. You can record shapes using standard p5 drawing functions inside createShape() or buildShape()." but, "to create SVG output, the only thing you need to do differently is use crateshape and build shape. Inside these functions, use standard drawing API (rect, circle, path, fill, stroke, translate, rotate, etc.) as you usually would
|
|
||
| # p5.svg Overview | ||
|
|
||
| `p5.svg` is an experimental native vector graphics system provided in p5.js starting from version 2. It aims to bring resolution-independent vector rendering, SVG file importing, and SVG XML exporting directly into p5.js without requiring external third-party addons. It allows users to record 2D drawing operations using familiar p5 APIs (`rect`, `circle`, `path`, `fill`, `stroke`, `translate`, `rotate`, etc.) and convert them into scalable vector structures. |
There was a problem hiding this comment.
I think this paragraph should use very simple terms, nothing significantly technical (ie not "resolution-independent")
|
|
||
| The main ways you can help develop `p5.svg` are: | ||
|
|
||
| - **API Ergonomics & Shape API**: Test vector shape recording and rendering functions like `createSVG()`, `loadSVG()`, `buildShape()`, `createShape()`, `shape()`, `getSVG()`, and `saveSVG()`. Share feedback on `shape()` playback, coordinate bounds, positioning, scaling options, and alignment modes (`CORNER`, `CENTER`, `VIEWBOX`). |
There was a problem hiding this comment.
This section could be just "Shape API". I do not recommend the phrase "ergonomics," because in general API ergonomics does not imply a focus on learning and beginners as a user group, which is a specific focus in p5.js (part of access statement that relates to this feature)
In this case, in the whole feedback section, maybe it is useful top provide a bit more context on what the priorities are: maintaining familiarity with rest of p5.js; creating non-OOP, top-level, readable function calls to allow smooth svg recording / loading. The question is not ergonomics generally but: "how does this feel as a beginner?" or "if you teach with p5.js, do you see API choices that would be tricky for students?"
That's still quite broad, but for example, feedback like "I think this should be more compact / more OOP" would not really be relevant/applicable.
Just a general feedback, please feel free to implement as much as it makes sense
| import strands from './strands/p5.strands'; | ||
| p5.registerAddon(strands); | ||
| import svg from './shape/svg/p5.svg'; | ||
| p5.registerAddon(svg); |
141d8e5 to
8b7e241
Compare
| // on p5.prototype. It hooks into predraw and postdraw lifecycles to automatically capture drawing commands | ||
| // when saveSVG() is called without explicit shape parameters. | ||
| export function SVGExportAddon(p5, fn, lifecycles) { | ||
| let pendingExport = null; |
There was a problem hiding this comment.
To make this work with multiple p5 instances, should we make this a property on the instance rather than effectively a global? I believe doing fn.pendingExport = null at the start and then referring to this.pendingExport in addon lifecycles and functions would make it instance state (see #7742 (comment))
|
|
||
| // RecordedShape manages the lifecycle of a recorded vector shape session. | ||
| // Calling begin() starts ShapeRecorder capture, and end() finalizes the AST data graph. | ||
| class RecordedShape { |
There was a problem hiding this comment.
To make TypeScript types and FES work for these, we may need to add some minimal doc comments. We probably also will need to move this class out of the addon function and then put p5.RecordedShape = RecordedShape within the addon function to make it exportable + referenceable in the jsdoc as p5.RecordedShape. (Maybe also worth a last thought to see if that's the right name for it?) We can then add the @beta tag to it to make it clear it's experimental. See notes on documenting classes here: https://p5js.org/contribute/contributing_to_the_p5js_reference/#creating-and-documenting-classes
There was a problem hiding this comment.
We can use p5.SVGShape as an alternative, or do you have something else in mind?
| } | ||
|
|
||
| // Instantiates a new RecordedShape vector container. | ||
| fn.createShape = function () { |
There was a problem hiding this comment.
We should probably add some possibly initially minimal jsdoc comments to these. If your examples can be easily added then that's great too
There was a problem hiding this comment.
Yes, examples can be easily fitted.
| } | ||
|
|
||
| visitRect(node, context) { | ||
| const w = this.num(node, "width"); |
There was a problem hiding this comment.
btw pretty minor but p5 currently uses 2-space tabs, might be worth doing a quick find-and-replace with before merging?
|
Looking good! My main notes before merging are around getting it to a point where it integrates with p5's docs, FES, and TypeScript types, which all comes down to adding jsdoc I believe. |
742b58b to
d9b6031
Compare
12c3451 to
77a3f33
Compare
|
I’ve added the unit tests to this PR. Once the review is complete, I’ll add the visual tests to the same PR as well. Please let me know if there are any changes or additional test cases you’d like me to consider. |


Resolves #4630
Changes:
Native SVG Module (
src/shape/svg/): Added core SVG export, import, and shape recording capabilities:svg_recorder.js:ShapeRecorderAST node graph (ScopeNode,ShapeNode,BackgroundNode,ClearNode,ImageNode) andTransformStack.svg_export.js:SVGExportAddon,SVGVisitorXML DOM renderer, and public APIs (createShape(),buildShape(),getSVG(),shape(),saveSVG()).svg_import.js:SVGImportAddon, path command parser (M,L,C,S,Q,A,Z), element converters, and public APIs (createSVG(),async loadSVG()).p5.svg.js: SVG module entry point combining export and import addons.Core Addon Registration (
src/app.js): Registeredsvgaddon viap5.registerAddon(svg), bundling SVG capabilities directly into mainlib/p5.js.Experimental Warning System (
src/core/experimental.js): Added'p5.svg'message toexperimentalMessagesand wrapped public SVG prototype methods withmarkExperimental('p5.svg', p5)warning decorators.Contributor Documentation (
contributor_docs/p5.svg.md): Created contributor guide covering project goals, API ergonomics, technical architecture, and community guidelines.PR Checklist
npm run lintpasses