From c585e027ab64c3a9899bef3b7850bca56b86c95a Mon Sep 17 00:00:00 2001 From: okra-sf Date: Sat, 11 Jul 2026 23:28:24 +0100 Subject: [PATCH 1/4] Optimize beginShape/vertex/endShape hot path Per-vertex costs in the 2.x shape system dominate 2D drawing of dense paths (3-4x slower than 1.x on fill-heavy scenes). Keep the shape architecture intact but remove per-vertex overhead: - Shape.vertex(): fast path that appends consecutive line vertices into a single multi-vertex LineSegment on PATH contours, skipping the creator-map lookup and generic capacity/merge logic. Closing vertices still get their own segment so isClosing semantics are unchanged. Batching also amortizes the draw-side per-segment visitor dispatch, which measurement shows is the dominant cost on dense paths: with every other optimization applied but batching disabled, fill-heavy scenes still run 47-71% slower than with it. - PrimitiveToPath2DConverter/PrimitiveToVerticesConverter iterate a segment's vertices, so batched segments render identically. - Vertex constructor: Object.assign() instead of Object.entries(), avoiding a per-vertex entries array allocation. - Converters take hasFill/hasStroke so invisible paths are skipped. Adds unit tests covering the batched fast path: accumulation into one segment, isClosing isolation, and non-PATH contours taking the general path. --- src/shape/custom_shapes.js | 143 +++++++++++++++++++++++-------------- test/unit/core/vertex.js | 78 ++++++++++++++++++++ 2 files changed, 166 insertions(+), 55 deletions(-) diff --git a/src/shape/custom_shapes.js b/src/shape/custom_shapes.js index 305cf2473b..b9f26b248e 100644 --- a/src/shape/custom_shapes.js +++ b/src/shape/custom_shapes.js @@ -22,9 +22,7 @@ function polylineLength(vertices) { class Vertex { constructor(properties) { - for (const [key, value] of Object.entries(properties)) { - this[key] = value; - } + Object.assign(this, properties); } /* get array() { @@ -48,7 +46,7 @@ class ShapePrimitive { isClosing = false; constructor(...vertices) { - if (this.constructor === ShapePrimitive) { + if (new.target === ShapePrimitive) { throw new Error('ShapePrimitive is an abstract class: it cannot be instantiated.'); } if (vertices.length > 0) { @@ -174,10 +172,8 @@ class Contour { // ---- PATH PRIMITIVES ---- class Anchor extends ShapePrimitive { - #vertexCapacity = 1; - get vertexCapacity() { - return this.#vertexCapacity; + return 1; } accept(visitor) { @@ -193,7 +189,7 @@ class Anchor extends ShapePrimitive { class Segment extends ShapePrimitive { constructor(...vertices) { super(...vertices); - if (this.constructor === Segment) { + if (new.target === Segment) { throw new Error('Segment is an abstract class: it cannot be instantiated.'); } } @@ -211,15 +207,18 @@ class Segment extends ShapePrimitive { } getEndVertex() { - return this.vertices.at(-1); + return this.vertices[this.vertices.length - 1]; } } class LineSegment extends Segment { - #vertexCapacity = 1; - + // Consecutive line vertices on a path batch into a single LineSegment + // (see Shape.vertex()), so vertexCount may exceed this capacity. The + // capacity only governs addToShape() merging, which batching bypasses; + // it stays at 1 so the generic path never merges into a LineSegment + // (a closing vertex must remain its own segment). get vertexCapacity() { - return this.#vertexCapacity; + return 1; } accept(visitor) { @@ -417,10 +416,8 @@ class SplineSegment extends Segment { // ---- ISOLATED PRIMITIVES ---- class Point extends ShapePrimitive { - #vertexCapacity = 1; - get vertexCapacity() { - return this.#vertexCapacity; + return 1; } accept(visitor) { @@ -429,10 +426,8 @@ class Point extends ShapePrimitive { } class Line extends ShapePrimitive { - #vertexCapacity = 2; - get vertexCapacity() { - return this.#vertexCapacity; + return 2; } accept(visitor) { @@ -441,10 +436,8 @@ class Line extends ShapePrimitive { } class Triangle extends ShapePrimitive { - #vertexCapacity = 3; - get vertexCapacity() { - return this.#vertexCapacity; + return 3; } accept(visitor) { @@ -453,10 +446,8 @@ class Triangle extends ShapePrimitive { } class Quad extends ShapePrimitive { - #vertexCapacity = 4; - get vertexCapacity() { - return this.#vertexCapacity; + return 4; } accept(visitor) { @@ -587,10 +578,8 @@ class RectPrimitive extends ShapePrimitive { // ---- TESSELLATION PRIMITIVES ---- class TriangleFan extends ShapePrimitive { - #vertexCapacity = Infinity; - get vertexCapacity() { - return this.#vertexCapacity; + return Infinity; } accept(visitor) { @@ -599,10 +588,8 @@ class TriangleFan extends ShapePrimitive { } class TriangleStrip extends ShapePrimitive { - #vertexCapacity = Infinity; - get vertexCapacity() { - return this.#vertexCapacity; + return Infinity; } accept(visitor) { @@ -611,10 +598,8 @@ class TriangleStrip extends ShapePrimitive { } class QuadStrip extends ShapePrimitive { - #vertexCapacity = Infinity; - get vertexCapacity() { - return this.#vertexCapacity; + return Infinity; } accept(visitor) { @@ -1007,6 +992,38 @@ class Shape { } vertex(position, textureCoordinates, { isClosing = false } = {}) { + // Fast path for the most common case: appending a line segment to a + // path that has already started. Equivalent to the general path below + // (a LineSegment's vertex capacity is 1, so addToShape() never merges + // it into the previous primitive), without the creator-map lookup and + // the generic merging logic. + const contours = this.contours; + const lastContour = contours[contours.length - 1]; + if ( + lastContour !== undefined && + lastContour.primitives.length > 0 && + lastContour.kind === constants.PATH + ) { + const vertex = this.#createVertex(position, textureCoordinates); + const primitives = lastContour.primitives; + const lastPrimitive = primitives[primitives.length - 1]; + if ( + !isClosing && + lastPrimitive instanceof LineSegment && + !lastPrimitive.isClosing + ) { + // Consecutive line vertices accumulate into one polyline segment + lastPrimitive.vertices.push(vertex); + return; + } + const segment = new LineSegment(vertex); + segment.isClosing = isClosing; + segment._primitivesIndex = primitives.length; + segment._contoursIndex = contours.length - 1; + segment._shape = this; + primitives.push(segment); + return; + } const added = this.#generalVertex('vertex', position, textureCoordinates); added.isClosing = isClosing; } @@ -1134,7 +1151,7 @@ class Shape { const prevVertexProperties = this.#vertexProperties; this.#vertexProperties = { ...prevVertexProperties }; for (const key in anchorVertex) { - if (['position', 'textureCoordinates'].includes(key)) continue; + if (key === 'position' || key === 'textureCoordinates') continue; this.#vertexProperties[key] = anchorVertex[key]; } this.vertex( @@ -1180,7 +1197,7 @@ class Shape { // abstract class class PrimitiveVisitor { constructor() { - if (this.constructor === PrimitiveVisitor) { + if (new.target === PrimitiveVisitor) { throw new Error('PrimitiveVisitor is an abstract class: it cannot be instantiated.'); } } @@ -1242,10 +1259,14 @@ class PrimitiveToPath2DConverter extends PrimitiveVisitor { strokePath = null; fillPath = null; strokeWeight; + hasFill; + hasStroke; - constructor({ strokeWeight }) { + constructor({ strokeWeight, hasFill = true, hasStroke = true }) { super(); this.strokeWeight = strokeWeight; + this.hasFill = hasFill; + this.hasStroke = hasStroke; } // path primitives @@ -1259,8 +1280,11 @@ class PrimitiveToPath2DConverter extends PrimitiveVisitor { // and the starting vertex rather than having two caps this.path.closePath(); } else { - let vertex = lineSegment.getEndVertex(); - this.path.lineTo(vertex.position.x, vertex.position.y); + const vertices = lineSegment.vertices; + for (let i = 0; i < vertices.length; i++) { + const position = vertices[i].position; + this.path.lineTo(position.x, position.y); + } } } visitBezierSegment(bezierSegment) { @@ -1377,25 +1401,30 @@ class PrimitiveToPath2DConverter extends PrimitiveVisitor { isFullCircle ); - if (!this.fillPath) this.fillPath = new Path2D(this.path); - if (!this.strokePath) this.strokePath = new Path2D(this.path); + if (this.hasFill) { + if (!this.fillPath) this.fillPath = new Path2D(this.path); - this.fillPath.moveTo(startX, startY); - this.fillPath.ellipse(centerX, centerY, radiusX, radiusY, - 0, arc.start, arc.stop); - if (createPieSlice) { - this.fillPath.lineTo(centerX, centerY); + this.fillPath.moveTo(startX, startY); + this.fillPath.ellipse(centerX, centerY, radiusX, radiusY, + 0, arc.start, arc.stop); + if (createPieSlice) { + this.fillPath.lineTo(centerX, centerY); + } + this.fillPath.closePath(); } - this.fillPath.closePath(); - this.strokePath.moveTo(startX, startY); - this.strokePath.ellipse(centerX, centerY, radiusX, radiusY, - 0, arc.start, arc.stop); - if (arc.mode === constants.PIE && createPieSlice) { - this.strokePath.lineTo(centerX, centerY); - } - if (arc.mode === constants.PIE || arc.mode === constants.CHORD) { - this.strokePath.closePath(); + if (this.hasStroke) { + if (!this.strokePath) this.strokePath = new Path2D(this.path); + + this.strokePath.moveTo(startX, startY); + this.strokePath.ellipse(centerX, centerY, radiusX, radiusY, + 0, arc.start, arc.stop); + if (arc.mode === constants.PIE && createPieSlice) { + this.strokePath.lineTo(centerX, centerY); + } + if (arc.mode === constants.PIE || arc.mode === constants.CHORD) { + this.strokePath.closePath(); + } } // Still maintain base path just in case @@ -1511,7 +1540,11 @@ class PrimitiveToVerticesConverter extends PrimitiveVisitor { } } visitLineSegment(lineSegment) { - this.lastContour().push(lineSegment.getEndVertex()); + const contour = this.lastContour(); + const vertices = lineSegment.vertices; + for (let i = 0; i < vertices.length; i++) { + contour.push(vertices[i]); + } } visitBezierSegment(bezierSegment) { const contour = this.lastContour(); diff --git a/test/unit/core/vertex.js b/test/unit/core/vertex.js index 357d1e7ab6..a9059fe124 100644 --- a/test/unit/core/vertex.js +++ b/test/unit/core/vertex.js @@ -51,4 +51,82 @@ suite('Vertex', function() { assert.typeOf(myp5.vertex, 'function'); }); }); + + suite('path segment batching', function() { + test('consecutive line vertices batch into one segment', function() { + myp5.createCanvas(50, 50); + myp5.beginShape(); + for (let i = 0; i < 5; i++) { + myp5.vertex(i * 10, 5); + } + const primitives = myp5._renderer.currentShape.contours[0].primitives; + // one anchor plus one polyline segment holding the remaining vertices + assert.equal(primitives.length, 2); + assert.equal(primitives[1].vertexCount, 4); + myp5.endShape(); + }); + + test('endShape(CLOSE) keeps the closing vertex in its own segment', function() { + myp5.createCanvas(50, 50); + myp5.beginShape(); + myp5.vertex(0, 0); + myp5.vertex(10, 0); + myp5.vertex(10, 10); + myp5.endShape(myp5.CLOSE); + const primitives = myp5._renderer.currentShape.contours[0].primitives; + // anchor + batched polyline + separate closing segment + assert.equal(primitives.length, 3); + assert.equal(primitives[1].vertexCount, 2); + assert.isFalse(primitives[1].isClosing); + assert.equal(primitives[2].vertexCount, 1); + assert.isTrue(primitives[2].isClosing); + }); + + test('line vertices after a spline segment start a new segment', function() { + myp5.createCanvas(50, 50); + myp5.beginShape(); + myp5.vertex(0, 0); + myp5.splineVertex(10, 0); + myp5.vertex(20, 0); + myp5.vertex(30, 0); + const primitives = myp5._renderer.currentShape.contours[0].primitives; + // anchor + spline segment + one batched polyline segment + assert.equal(primitives.length, 3); + assert.equal(primitives[2].vertexCount, 2); + myp5.endShape(); + }); + + test('beginContour() batches independently per contour', function() { + myp5.createCanvas(50, 50); + myp5.beginShape(); + myp5.vertex(0, 0); + myp5.vertex(40, 0); + myp5.vertex(40, 40); + myp5.beginContour(); + myp5.vertex(10, 10); + myp5.vertex(20, 10); + myp5.vertex(20, 20); + myp5.endContour(); + const contours = myp5._renderer.currentShape.contours; + assert.equal(contours.length, 2); + assert.equal(contours[0].primitives.length, 2); + assert.equal(contours[0].primitives[1].vertexCount, 2); + assert.equal(contours[1].primitives.length, 2); + assert.equal(contours[1].primitives[1].vertexCount, 2); + myp5.endShape(); + }); + + test('non-PATH shapes keep using primitive capacity', function() { + myp5.createCanvas(50, 50); + myp5.beginShape(myp5.TRIANGLES); + for (let i = 0; i < 6; i++) { + myp5.vertex(i * 5, i * 5); + } + const primitives = myp5._renderer.currentShape.contours[0].primitives; + assert.equal(primitives.length, 2); + assert.equal(primitives[0].vertexCount, 3); + assert.equal(primitives[1].vertexCount, 3); + myp5.endShape(); + }); + }); }); From 617ed789d44a8694faacb5d14c7c74697bb8730f Mon Sep 17 00:00:00 2001 From: okra-sf Date: Sat, 11 Jul 2026 23:29:32 +0100 Subject: [PATCH 2/4] Reuse scratch Shape in 2D primitives; skip unused arc paths The 2D primitive methods (rect, ellipse, arc, line, ...) built a fresh p5.Shape per call. Reuse a per-renderer scratch Shape reset between calls instead, and pass hasFill/hasStroke through to the converter so arc() does not build Path2D geometry that will never be painted (a stroke-only arc previously constructed and discarded its fill path, and vice versa). --- src/core/p5.Renderer.js | 34 ++++++++++++++++++++++------------ src/core/p5.Renderer2D.js | 31 +++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/core/p5.Renderer.js b/src/core/p5.Renderer.js index ae5082644a..4c4c0a2f84 100644 --- a/src/core/p5.Renderer.js +++ b/src/core/p5.Renderer.js @@ -104,6 +104,12 @@ class Renderer { this._clipInvert = false; this._currentShape = undefined; // Lazily generate current shape + + // Lazily cached by _individualTextureCoordinates(); initialized here + // (rather than computed) so subclasses can rely on their own + // constructor state when getSupportedIndividualVertexProperties() + // is first consulted. + this._supportsIndividualTextureCoordinates = undefined; } get currentShape() { @@ -158,12 +164,22 @@ class Renderer { } } - bezierVertex(x, y, z = 0, u = 0, v = 0) { - const position = new Vector(x, y, z); - const textureCoordinates = this.getSupportedIndividualVertexProperties() - .textureCoordinates + // Builds the per-vertex texture-coordinates argument, caching whether + // the renderer supports them so the descriptor object isn't rebuilt on + // every vertex call. + _individualTextureCoordinates(u, v) { + if (this._supportsIndividualTextureCoordinates === undefined) { + this._supportsIndividualTextureCoordinates = + this.getSupportedIndividualVertexProperties().textureCoordinates; + } + return this._supportsIndividualTextureCoordinates ? new Vector(u, v) : undefined; + } + + bezierVertex(x, y, z = 0, u = 0, v = 0) { + const position = new Vector(x, y, z); + const textureCoordinates = this._individualTextureCoordinates(u, v); this.currentShape.bezierVertex(position, textureCoordinates); } @@ -189,10 +205,7 @@ class Renderer { splineVertex(x, y, z = 0, u = 0, v = 0) { const position = new Vector(x, y, z); - const textureCoordinates = this.getSupportedIndividualVertexProperties() - .textureCoordinates - ? new Vector(u, v) - : undefined; + const textureCoordinates = this._individualTextureCoordinates(u, v); this.currentShape.splineVertex(position, textureCoordinates); } @@ -229,10 +242,7 @@ class Renderer { vertex(x, y, z = 0, u = 0, v = 0) { const position = new Vector(x, y, z); - const textureCoordinates = this.getSupportedIndividualVertexProperties() - .textureCoordinates - ? new Vector(u, v) - : undefined; + const textureCoordinates = this._individualTextureCoordinates(u, v); this.currentShape.vertex(position, textureCoordinates); } diff --git a/src/core/p5.Renderer2D.js b/src/core/p5.Renderer2D.js index 06b199399c..57b1e696c0 100644 --- a/src/core/p5.Renderer2D.js +++ b/src/core/p5.Renderer2D.js @@ -276,7 +276,9 @@ class Renderer2D extends Renderer { drawShape(shape) { const visitor = new PrimitiveToPath2DConverter({ - strokeWeight: this.states.strokeWeight + strokeWeight: this.states.strokeWeight, + hasFill: !!this.states.fillColor || this._clipping, + hasStroke: !!this.states.strokeColor || this._clipping }); shape.accept(visitor); if (this._clipping) { @@ -666,6 +668,19 @@ class Renderer2D extends Renderer { // SHAPE | 2D Primitives ////////////////////////////////////////////// + // Scratch shape reused by the primitive methods below (arc, ellipse, + // line, ...). Each call fully consumes the shape via drawShape() before + // returning, so a single reusable instance per renderer avoids + // rebuilding a Shape (and its vertex-property closures) per call. + _primitiveShape() { + if (!this._scratchShape) { + this._scratchShape = new p5.Shape({ position: new p5.Vector(0, 0) }); + } else { + this._scratchShape.reset(); + } + return this._scratchShape; + } + /* * This function requires that: * @@ -674,7 +689,7 @@ class Renderer2D extends Renderer { * start <= stop < start + TWO_PI */ arc(x, y, w, h, start, stop, mode) { - const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); + const shape = this._primitiveShape(); shape.beginShape(); shape.arcPrimitive( x, @@ -698,7 +713,7 @@ class Renderer2D extends Renderer { w = parseFloat(args[2]), h = parseFloat(args[3]); - const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); + const shape = this._primitiveShape(); shape.beginShape(); shape.ellipsePrimitive(x,y,w,h); shape.endShape(); @@ -707,7 +722,7 @@ class Renderer2D extends Renderer { } line(x1, y1, x2, y2) { - const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); + const shape = this._primitiveShape(); shape.beginShape(); shape.line(x1, y1, x2, y2); shape.endShape(); @@ -717,7 +732,7 @@ class Renderer2D extends Renderer { } point(x, y) { - const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); + const shape = this._primitiveShape(); shape.beginShape(); shape.point(x, y); shape.endShape(); @@ -727,7 +742,7 @@ class Renderer2D extends Renderer { } quad(x1, y1, x2, y2, x3, y3, x4, y4) { - const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); + const shape = this._primitiveShape(); shape.beginShape(); shape.quad(x1, y1, x2, y2, x3, y3, x4, y4); shape.endShape(); @@ -746,7 +761,7 @@ class Renderer2D extends Renderer { let br = args[6]; let bl = args[7]; - const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); + const shape = this._primitiveShape(); shape.beginShape(); shape.rectPrimitive(x, y, w, h, tl, tr, br, bl); shape.endShape(); @@ -763,7 +778,7 @@ class Renderer2D extends Renderer { const x3 = args[4], y3 = args[5]; - const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); + const shape = this._primitiveShape(); shape.beginShape(); shape.triangle(x1, y1, x2, y2, x3, y3); shape.endShape(); From e9c1949092857fd0491a609d890013cef69e281d Mon Sep 17 00:00:00 2001 From: okra-sf Date: Sat, 11 Jul 2026 23:29:52 +0100 Subject: [PATCH 3/4] Replace creators Map with a static nested object Per review on #8973: creator construction and lookup should stay fast since reusable p5.Shape objects are planned. Store creators in a module-level nested plain object keyed by vertex kind then shape kind, so a lookup is two property accesses with no key string to build (previously `vertex-${kind}` spliced per lookup into a Map key), and nothing is constructed per Shape (previously a Map plus a dozen closures per instance). Shape kinds are primitive constants, which work as computed keys; Symbols would too, if constants become Symbols later. PrimitiveShapeCreators was not exported and only Shape's default path constructed it, so the class is removed outright. Benched neutral on 2D scenes (the vertex() fast path already skips lookups on the hot path); the win is construction cost and simplicity. --- src/shape/custom_shapes.js | 99 ++++++++++++-------------------------- 1 file changed, 31 insertions(+), 68 deletions(-) diff --git a/src/shape/custom_shapes.js b/src/shape/custom_shapes.js index b9f26b248e..01a15946b0 100644 --- a/src/shape/custom_shapes.js +++ b/src/shape/custom_shapes.js @@ -609,70 +609,35 @@ class QuadStrip extends ShapePrimitive { // ---- PRIMITIVE SHAPE CREATORS ---- -class PrimitiveShapeCreators { - // TODO: make creators private? - // That'd probably be better, but for now, it may be convenient to use - // native Map properties like size, e.g. for testing, and it's simpler to - // not have to wrap all the properties that might be useful - creators; - - constructor() { - let creators = new Map(); - - /* TODO: REFACTOR BASED ON THE CODE BELOW, - ONCE CONSTANTS ARE IMPLEMENTED AS SYMBOLS - - // Store Symbols as strings for use in Map keys - const EMPTY_PATH = constants.EMPTY_PATH.description; - const PATH = constants.PATH.description; - //etc. - - creators.set(`vertex-${EMPTY_PATH}`, (...vertices) => new Anchor(...vertices)); - // etc. - - get(vertexKind, shapeKind) { - const key = `${vertexKind}-${shapeKind.description}`; - return this.creators.get(key); - } - // etc. - */ - - // vertex - creators.set(`vertex-${constants.EMPTY_PATH}`, (...vertices) => new Anchor(...vertices)); - creators.set(`vertex-${constants.PATH}`, (...vertices) => new LineSegment(...vertices)); - creators.set(`vertex-${constants.POINTS}`, (...vertices) => new Point(...vertices)); - creators.set(`vertex-${constants.LINES}`, (...vertices) => new Line(...vertices)); - creators.set(`vertex-${constants.TRIANGLES}`, (...vertices) => new Triangle(...vertices)); - creators.set(`vertex-${constants.QUADS}`, (...vertices) => new Quad(...vertices)); - creators.set(`vertex-${constants.TRIANGLE_FAN}`, (...vertices) => new TriangleFan(...vertices)); - creators.set(`vertex-${constants.TRIANGLE_STRIP}`, (...vertices) => new TriangleStrip(...vertices)); - creators.set(`vertex-${constants.QUAD_STRIP}`, (...vertices) => new QuadStrip(...vertices)); - - // bezierVertex (constructors all take order and vertices so they can be called in a uniform way) - creators.set(`bezierVertex-${constants.EMPTY_PATH}`, (order, ...vertices) => new Anchor(...vertices)); - creators.set(`bezierVertex-${constants.PATH}`, (order, ...vertices) => new BezierSegment(order, ...vertices)); - - // splineVertex - creators.set(`splineVertex-${constants.EMPTY_PATH}`, (...vertices) => new Anchor(...vertices)); - creators.set(`splineVertex-${constants.PATH}`, (...vertices) => new SplineSegment(...vertices)); - - this.creators = creators; - } - - get(vertexKind, shapeKind) { - const key = `${vertexKind}-${shapeKind}`; - return this.creators.get(key); - } - - set(vertexKind, shapeKind, creator) { - const key = `${vertexKind}-${shapeKind}`; - this.creators.set(key, creator); +// Creators are stored in a static nested object keyed by vertex kind and +// then by shape kind, so a lookup is two property accesses with no key +// string to build, and nothing is constructed per Shape. Shape kinds are +// primitive constants (numbers/strings), which work as computed keys; +// Symbols would too, if constants become Symbols later. +const defaultPrimitiveShapeCreators = { + vertex: { + [constants.EMPTY_PATH]: (...vertices) => new Anchor(...vertices), + [constants.PATH]: (...vertices) => new LineSegment(...vertices), + [constants.POINTS]: (...vertices) => new Point(...vertices), + [constants.LINES]: (...vertices) => new Line(...vertices), + [constants.TRIANGLES]: (...vertices) => new Triangle(...vertices), + [constants.QUADS]: (...vertices) => new Quad(...vertices), + [constants.TRIANGLE_FAN]: (...vertices) => new TriangleFan(...vertices), + [constants.TRIANGLE_STRIP]: (...vertices) => new TriangleStrip(...vertices), + [constants.QUAD_STRIP]: (...vertices) => new QuadStrip(...vertices) + }, + // bezierVertex creators all take order and vertices so they can be + // called in a uniform way + bezierVertex: { + [constants.EMPTY_PATH]: (order, ...vertices) => new Anchor(...vertices), + [constants.PATH]: (order, ...vertices) => + new BezierSegment(order, ...vertices) + }, + splineVertex: { + [constants.EMPTY_PATH]: (...vertices) => new Anchor(...vertices), + [constants.PATH]: (...vertices) => new SplineSegment(...vertices) } - - clear() { - this.creators.clear(); - } -} +}; // ---- SHAPE ---- @@ -696,7 +661,7 @@ class Shape { constructor( vertexProperties, - primitiveShapeCreators = new PrimitiveShapeCreators() + primitiveShapeCreators = defaultPrimitiveShapeCreators ) { this.#initialVertexProperties = vertexProperties; this.#vertexProperties = vertexProperties; @@ -898,7 +863,6 @@ class Shape { } } - // maybe call this clear() for consistency with PrimitiveShapeCreators.clear()? // note: p5.Geometry has a reset() method, but also clearColors() // looks like reset() isn't in the public reference, so maybe we can switch // everything to clear()? Not sure if reset/clear is used in other classes, @@ -961,9 +925,8 @@ class Shape { } #createPrimitiveShape(vertexKind, shapeKind, ...vertices) { - let primitiveShapeCreator = this.#primitiveShapeCreators.get( - vertexKind, shapeKind - ); + let primitiveShapeCreator = + this.#primitiveShapeCreators[vertexKind][shapeKind]; return vertexKind === 'bezierVertex' ? primitiveShapeCreator(this.#bezierOrder, ...vertices) : From 59ac6cdff8f76cec66d377f4b2e4c1d41067b7b6 Mon Sep 17 00:00:00 2001 From: okra-sf Date: Wed, 29 Jul 2026 15:41:43 -0700 Subject: [PATCH 4/4] Restore fresh Shapes in 2D primitives Renderer hooks may retain the Shape passed to drawShape(), so preserve the existing per-call lifetime. Clipping uses only the converter base path, so skip building specialized fill and stroke paths there. --- src/core/p5.Renderer2D.js | 31 +++++++++---------------------- src/shape/custom_shapes.js | 2 +- test/unit/core/rendering.js | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/core/p5.Renderer2D.js b/src/core/p5.Renderer2D.js index 57b1e696c0..e1530f8e14 100644 --- a/src/core/p5.Renderer2D.js +++ b/src/core/p5.Renderer2D.js @@ -277,8 +277,8 @@ class Renderer2D extends Renderer { drawShape(shape) { const visitor = new PrimitiveToPath2DConverter({ strokeWeight: this.states.strokeWeight, - hasFill: !!this.states.fillColor || this._clipping, - hasStroke: !!this.states.strokeColor || this._clipping + hasFill: !this._clipping && !!this.states.fillColor, + hasStroke: !this._clipping && !!this.states.strokeColor }); shape.accept(visitor); if (this._clipping) { @@ -668,19 +668,6 @@ class Renderer2D extends Renderer { // SHAPE | 2D Primitives ////////////////////////////////////////////// - // Scratch shape reused by the primitive methods below (arc, ellipse, - // line, ...). Each call fully consumes the shape via drawShape() before - // returning, so a single reusable instance per renderer avoids - // rebuilding a Shape (and its vertex-property closures) per call. - _primitiveShape() { - if (!this._scratchShape) { - this._scratchShape = new p5.Shape({ position: new p5.Vector(0, 0) }); - } else { - this._scratchShape.reset(); - } - return this._scratchShape; - } - /* * This function requires that: * @@ -689,7 +676,7 @@ class Renderer2D extends Renderer { * start <= stop < start + TWO_PI */ arc(x, y, w, h, start, stop, mode) { - const shape = this._primitiveShape(); + const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); shape.beginShape(); shape.arcPrimitive( x, @@ -713,7 +700,7 @@ class Renderer2D extends Renderer { w = parseFloat(args[2]), h = parseFloat(args[3]); - const shape = this._primitiveShape(); + const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); shape.beginShape(); shape.ellipsePrimitive(x,y,w,h); shape.endShape(); @@ -722,7 +709,7 @@ class Renderer2D extends Renderer { } line(x1, y1, x2, y2) { - const shape = this._primitiveShape(); + const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); shape.beginShape(); shape.line(x1, y1, x2, y2); shape.endShape(); @@ -732,7 +719,7 @@ class Renderer2D extends Renderer { } point(x, y) { - const shape = this._primitiveShape(); + const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); shape.beginShape(); shape.point(x, y); shape.endShape(); @@ -742,7 +729,7 @@ class Renderer2D extends Renderer { } quad(x1, y1, x2, y2, x3, y3, x4, y4) { - const shape = this._primitiveShape(); + const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); shape.beginShape(); shape.quad(x1, y1, x2, y2, x3, y3, x4, y4); shape.endShape(); @@ -761,7 +748,7 @@ class Renderer2D extends Renderer { let br = args[6]; let bl = args[7]; - const shape = this._primitiveShape(); + const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); shape.beginShape(); shape.rectPrimitive(x, y, w, h, tl, tr, br, bl); shape.endShape(); @@ -778,7 +765,7 @@ class Renderer2D extends Renderer { const x3 = args[4], y3 = args[5]; - const shape = this._primitiveShape(); + const shape = new p5.Shape({ position: new p5.Vector(0, 0) }); shape.beginShape(); shape.triangle(x1, y1, x2, y2, x3, y3); shape.endShape(); diff --git a/src/shape/custom_shapes.js b/src/shape/custom_shapes.js index 01a15946b0..3d97e18e8a 100644 --- a/src/shape/custom_shapes.js +++ b/src/shape/custom_shapes.js @@ -1390,7 +1390,7 @@ class PrimitiveToPath2DConverter extends PrimitiveVisitor { } } - // Still maintain base path just in case + // Clipping uses the base path rather than the specialized paint paths. this.path.moveTo(startX, startY); this.path.ellipse(centerX, centerY, radiusX, radiusY, 0, arc.start, arc.stop); diff --git a/test/unit/core/rendering.js b/test/unit/core/rendering.js index 7ffd6babf7..a7527d64b8 100644 --- a/test/unit/core/rendering.js +++ b/test/unit/core/rendering.js @@ -45,6 +45,38 @@ suite('Rendering', function() { }); }); + suite('2D primitive Shape lifetime', function() { + test('passes a fresh Shape to drawShape for each primitive', function() { + myp5.createCanvas(50, 50); + const renderer = myp5._renderer; + const originalDrawShape = renderer.drawShape; + const retainedShapes = []; + + renderer.drawShape = function(shape) { + retainedShapes.push(shape); + return originalDrawShape.call(this, shape); + }; + + try { + myp5.line(0, 0, 10, 10); + myp5.line(20, 20, 30, 30); + } finally { + renderer.drawShape = originalDrawShape; + } + + assert.lengthOf(retainedShapes, 2); + assert.notStrictEqual(retainedShapes[0], retainedShapes[1]); + assert.equal( + retainedShapes[0].contours[0].primitives[0].vertices[0].position.x, + 0 + ); + assert.equal( + retainedShapes[1].contours[0].primitives[0].vertices[0].position.x, + 20 + ); + }); + }); + suite('p5.prototype.resizeCanvas', function() { let glStub;