From b66d09a8d358845656145ca1fbba02d308da2aa3 Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Mon, 20 Jul 2026 12:27:11 -0700 Subject: [PATCH 01/10] Implemented a loading indicator that will appear when the canvas is being set up. --- src/core/loading.js | 31 +++++++++++++++++++++++++++++++ src/core/main.js | 12 +++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 src/core/loading.js diff --git a/src/core/loading.js b/src/core/loading.js new file mode 100644 index 0000000000..4a3ee81b45 --- /dev/null +++ b/src/core/loading.js @@ -0,0 +1,31 @@ +/** + * Creates a loading indicator + */ + +let loadingIndicator = null; + +export function showLoadingIndicator(canvas) { + if (!canvas || loadingIndicator) { + return; + } + + loadingIndicator = document.createElement('div'); + loadingIndicator.textContent = 'Loading...'; + + loadingIndicator.style.cssText = ` + position: fixed; + top: 0; + left: 0; + width: 100%; + height: 100%; + display: grid; + place-items: center; + `; + + canvas.parentElement?.appendChild(loadingIndicator); +} + +export function hideLoadingIndicator() { + loadingIndicator?.remove(); + loadingIndicator = null; +} \ No newline at end of file diff --git a/src/core/main.js b/src/core/main.js index 00e536d900..2fa3959538 100644 --- a/src/core/main.js +++ b/src/core/main.js @@ -243,7 +243,17 @@ class p5 { const context = this._isGlobal ? window : this; if (typeof context.setup === 'function') { - await context.setup(); + if(typeof window !== 'undefined' && this.canvas) { + showLoadingIndicator(this.canvas); + } + try { + await context.setup(); + } + finally { + if(typeof window !== 'undefined') { + hideLoadingIndicator(); + } + } } if (this.hitCriticalError) return; From 66de638482e26b262f02f4bb74c3645c78a6a6da Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Mon, 20 Jul 2026 13:25:33 -0700 Subject: [PATCH 02/10] Added inline references for loading.js --- src/core/loading.js | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/core/loading.js b/src/core/loading.js index 4a3ee81b45..0b9d885b05 100644 --- a/src/core/loading.js +++ b/src/core/loading.js @@ -1,9 +1,21 @@ /** - * Creates a loading indicator + * @module Loading + * @for p5 + * @private + * + * Creates a loading indicator when the sketch's setup() function is running. + * Currently, the loading indicator is basic and can be extended in the future. */ let loadingIndicator = null; +/** + * Creates a simple loading indicator at the center of the webpage. + * + * @method showLoadingIndicator + * @param {HTMLElement} canvas The canvas element of the webpage. + * @private + */ export function showLoadingIndicator(canvas) { if (!canvas || loadingIndicator) { return; @@ -25,6 +37,12 @@ export function showLoadingIndicator(canvas) { canvas.parentElement?.appendChild(loadingIndicator); } +/** + * Removes the loading indicator element from the webpage DOM. + * + * @method hideLoadingIndicator + * @private + */ export function hideLoadingIndicator() { loadingIndicator?.remove(); loadingIndicator = null; From 4ab5875dc825b19bcd11d04e6b2b30d0e6c40877 Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Mon, 20 Jul 2026 14:44:38 -0700 Subject: [PATCH 03/10] Added a unit test for testing the loading indicator. --- src/core/main.js | 1 + test/unit/core/loading.js | 78 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 test/unit/core/loading.js diff --git a/src/core/main.js b/src/core/main.js index 2fa3959538..12eb8d854e 100644 --- a/src/core/main.js +++ b/src/core/main.js @@ -5,6 +5,7 @@ */ import * as constants from './constants'; +import { showLoadingIndicator, hideLoadingIndicator } from './loading'; /** * This is the p5 instance constructor. diff --git a/test/unit/core/loading.js b/test/unit/core/loading.js new file mode 100644 index 0000000000..7f7aaaff6d --- /dev/null +++ b/test/unit/core/loading.js @@ -0,0 +1,78 @@ +import { vi } from 'vitest'; +import { showLoadingIndicator, hideLoadingIndicator } from '../../../src/core/loading.js'; + +suite('Loading indicator', function() { + let container; + let canvas; + + beforeEach(function() { + container = document.createElement('div'); + canvas = document.createElement('canvas'); + container.appendChild(canvas); + document.body.appendChild(container); + }); + + afterEach(function() { + hideLoadingIndicator(); + + if (container) { + container.remove(); + container = null; + canvas = null; + } + }); + + test('shows a loading indicator while async setup waits for load()', async function() { + let loadTest; + + const p = { + createCanvas: vi.fn(), + background: vi.fn(), + fill: vi.fn(), + circle: vi.fn(), + width: 400, + height: 400, + mouseX: 12, + mouseY: 34 + }; + + const load = async delay => { + await new Promise(resolve => { + loadTest = resolve; + }); + }; + + const setupPromise = (async function setup() { + showLoadingIndicator(canvas); + + try { + p.createCanvas(400, 400); + + await load(7000); + + p.background('#EB5580'); + p.fill(255); + p.circle(p.width / 2, p.height / 2, 100); + + p.circle(p.mouseX, p.mouseY, 20); + } + finally { + hideLoadingIndicator(); + } + })(); + + const loadingOverlay = container.querySelector('div'); + + assert.exists(loadingOverlay); + assert.equal(loadingOverlay.textContent, 'Loading...'); + + loadTest(); + await setupPromise; + + assert.isNull(container.querySelector('div')); + assert.deepEqual(p.createCanvas.mock.calls, [[400, 400]]); + assert.deepEqual(p.background.mock.calls, [['#EB5580']]); + assert.deepEqual(p.fill.mock.calls, [[255]]); + assert.deepEqual(p.circle.mock.calls, [[200, 200, 100], [12, 34, 20]]); + }); +}); \ No newline at end of file From 6ec7145daaa935072e2cf0405c6fdde13ad790dd Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Wed, 22 Jul 2026 11:16:05 -0700 Subject: [PATCH 04/10] Decoupled the loading indicators from main.js and replaced the loading text with a basic animation. --- src/core/loading.js | 68 ++++++++++++++++++++++++--------------------- src/core/main.js | 15 ++-------- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/src/core/loading.js b/src/core/loading.js index 0b9d885b05..a59147df04 100644 --- a/src/core/loading.js +++ b/src/core/loading.js @@ -9,41 +9,45 @@ let loadingIndicator = null; -/** - * Creates a simple loading indicator at the center of the webpage. - * - * @method showLoadingIndicator - * @param {HTMLElement} canvas The canvas element of the webpage. - * @private - */ -export function showLoadingIndicator(canvas) { - if (!canvas || loadingIndicator) { - return; - } +export default function loading(p5, fn, lifecycles) { + lifecycles.presetup = function() { + if (typeof window === 'undefined' || loadingIndicator) return; + + const container = this.canvas?.parentElement || document.body; + loadingIndicator = createLoadingIndicator(container); + }; + + lifecycles.postsetup = function() { + loadingIndicator?.remove(); + loadingIndicator = null; + }; +} - loadingIndicator = document.createElement('div'); - loadingIndicator.textContent = 'Loading...'; +function createLoadingIndicator(container) { + if (!document.getElementById('loading-style')) { + const style = document.createElement('style'); + style.id = 'loading-style'; + style.textContent = '@keyframes loading-spin { to { transform: translate(-50%, -50%) rotate(360deg);}}'; + document.head.appendChild(style); + } - loadingIndicator.style.cssText = ` + const indicator = document.createElement('div'); + indicator.className = 'loading-indicator'; + indicator.style.cssText = ` position: fixed; - top: 0; - left: 0; - width: 100%; - height: 100%; - display: grid; - place-items: center; - `; + top: 50%; + left: 50%; + transform: translate(-50%, -50%); + width: 30px; + height: 30px; + border-radius: 50%; - canvas.parentElement?.appendChild(loadingIndicator); -} + border: 3px solid rgba(0, 0, 0, 0.1); + border-top-color: rgba(0, 0, 0, 0.8); + animation: loading-spin 1s linear infinite; + z-index: 9999; + `; -/** - * Removes the loading indicator element from the webpage DOM. - * - * @method hideLoadingIndicator - * @private - */ -export function hideLoadingIndicator() { - loadingIndicator?.remove(); - loadingIndicator = null; + container.appendChild(indicator); + return indicator; } \ No newline at end of file diff --git a/src/core/main.js b/src/core/main.js index 12eb8d854e..f002b5ced3 100644 --- a/src/core/main.js +++ b/src/core/main.js @@ -5,7 +5,6 @@ */ import * as constants from './constants'; -import { showLoadingIndicator, hideLoadingIndicator } from './loading'; /** * This is the p5 instance constructor. @@ -244,17 +243,7 @@ class p5 { const context = this._isGlobal ? window : this; if (typeof context.setup === 'function') { - if(typeof window !== 'undefined' && this.canvas) { - showLoadingIndicator(this.canvas); - } - try { - await context.setup(); - } - finally { - if(typeof window !== 'undefined') { - hideLoadingIndicator(); - } - } + await context.setup(); } if (this.hitCriticalError) return; @@ -649,6 +638,7 @@ import rendering from './rendering'; import renderer from './p5.Renderer'; import renderer2D from './p5.Renderer2D'; import graphics from './p5.Graphics'; +import loading from './loading'; p5.registerAddon(transform); p5.registerAddon(structure); @@ -657,6 +647,7 @@ p5.registerAddon(rendering); p5.registerAddon(renderer); p5.registerAddon(renderer2D); p5.registerAddon(graphics); +p5.registerAddon(loading); export default p5; From 51cc0c1e6e2e460c190198a1e8dbbb42c04a8940 Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Wed, 22 Jul 2026 11:33:31 -0700 Subject: [PATCH 05/10] Refactored loading.js and added inline references. Also modified the unit test for the loading indicator to work with the updated loading.js file. --- src/core/loading.js | 32 +++++++++++++++++++++++--------- test/unit/core/loading.js | 21 ++++++++++++--------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/core/loading.js b/src/core/loading.js index a59147df04..fa200b5e54 100644 --- a/src/core/loading.js +++ b/src/core/loading.js @@ -3,12 +3,20 @@ * @for p5 * @private * - * Creates a loading indicator when the sketch's setup() function is running. + * Handles the logic for creating a loading indicator. * Currently, the loading indicator is basic and can be extended in the future. */ let loadingIndicator = null; +/** + * Creates a loading indicator when the sketch's setup() function is running. + * It is called and removed automatically using the presetup and postsetup lifecycles hooks. + * + * @param {*} p5 The p5 constructor + * @param {*} fn The p5 prototype object + * @param {*} lifecycles Lifecycle hooks for the sketch + */ export default function loading(p5, fn, lifecycles) { lifecycles.presetup = function() { if (typeof window === 'undefined' || loadingIndicator) return; @@ -23,21 +31,28 @@ export default function loading(p5, fn, lifecycles) { }; } +/** + * Creates and stylizes the loading indicator. + * As a helper function, it can be extensible and modified in future versions. + * + * @private + * @param {HTMLElement} container The HTML element to append the indicator to + * @returns {HTMLElement} The loading indicator div element + */ function createLoadingIndicator(container) { if (!document.getElementById('loading-style')) { - const style = document.createElement('style'); - style.id = 'loading-style'; - style.textContent = '@keyframes loading-spin { to { transform: translate(-50%, -50%) rotate(360deg);}}'; - document.head.appendChild(style); + const loadingStyle = document.createElement('style'); + loadingStyle.id = 'loading-style'; + loadingStyle.textContent = '@keyframes loading-spin { to { transform: rotate(360deg); } }'; + document.head.appendChild(loadingStyle); } const indicator = document.createElement('div'); indicator.className = 'loading-indicator'; indicator.style.cssText = ` position: fixed; - top: 50%; - left: 50%; - transform: translate(-50%, -50%); + inset: 0; + margin: auto; width: 30px; height: 30px; border-radius: 50%; @@ -45,7 +60,6 @@ function createLoadingIndicator(container) { border: 3px solid rgba(0, 0, 0, 0.1); border-top-color: rgba(0, 0, 0, 0.8); animation: loading-spin 1s linear infinite; - z-index: 9999; `; container.appendChild(indicator); diff --git a/test/unit/core/loading.js b/test/unit/core/loading.js index 7f7aaaff6d..79da1992d1 100644 --- a/test/unit/core/loading.js +++ b/test/unit/core/loading.js @@ -1,10 +1,13 @@ -import { vi } from 'vitest'; -import { showLoadingIndicator, hideLoadingIndicator } from '../../../src/core/loading.js'; +import { vi, suite, test, assert } from 'vitest'; +import loading from '../../../src/core/loading.js'; suite('Loading indicator', function() { let container; let canvas; + const lifecycles = {}; + loading(null, null, lifecycles); + beforeEach(function() { container = document.createElement('div'); canvas = document.createElement('canvas'); @@ -13,7 +16,7 @@ suite('Loading indicator', function() { }); afterEach(function() { - hideLoadingIndicator(); + lifecycles.postsetup?.(); if (container) { container.remove(); @@ -26,6 +29,7 @@ suite('Loading indicator', function() { let loadTest; const p = { + canvas: canvas, createCanvas: vi.fn(), background: vi.fn(), fill: vi.fn(), @@ -43,7 +47,7 @@ suite('Loading indicator', function() { }; const setupPromise = (async function setup() { - showLoadingIndicator(canvas); + lifecycles.presetup.call(p); try { p.createCanvas(400, 400); @@ -57,19 +61,18 @@ suite('Loading indicator', function() { p.circle(p.mouseX, p.mouseY, 20); } finally { - hideLoadingIndicator(); + lifecycles.postsetup.call(p); } })(); - const loadingOverlay = container.querySelector('div'); + const loadingIndicator = container.querySelector('.loading-indicator'); - assert.exists(loadingOverlay); - assert.equal(loadingOverlay.textContent, 'Loading...'); + assert.exists(loadingIndicator); loadTest(); await setupPromise; - assert.isNull(container.querySelector('div')); + assert.isNull(container.querySelector('.loading-indicator')); assert.deepEqual(p.createCanvas.mock.calls, [[400, 400]]); assert.deepEqual(p.background.mock.calls, [['#EB5580']]); assert.deepEqual(p.fill.mock.calls, [[255]]); From 83a439df3d2c15fa0e99aa994c6e636abae017a1 Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Tue, 28 Jul 2026 12:43:05 -0700 Subject: [PATCH 06/10] Modified loading.js so that instead of a global variable for the loading indicator, "this" is referenced so that every p5 instance has their own loading indicator. Also changed the animation and style names to include "p5" --- src/core/loading.js | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/core/loading.js b/src/core/loading.js index fa200b5e54..16d677b2b3 100644 --- a/src/core/loading.js +++ b/src/core/loading.js @@ -7,8 +7,6 @@ * Currently, the loading indicator is basic and can be extended in the future. */ -let loadingIndicator = null; - /** * Creates a loading indicator when the sketch's setup() function is running. * It is called and removed automatically using the presetup and postsetup lifecycles hooks. @@ -19,15 +17,23 @@ let loadingIndicator = null; */ export default function loading(p5, fn, lifecycles) { lifecycles.presetup = function() { - if (typeof window === 'undefined' || loadingIndicator) return; + if (typeof window === 'undefined' || this._loadingIndicator) return; + + const canvasParent = this.canvas?.parentElement; + let container = this._userNode || canvasParent || document.body; + + if (typeof container === 'string') { + container = document.getElementById(container) || document.body; + } - const container = this.canvas?.parentElement || document.body; - loadingIndicator = createLoadingIndicator(container); + this._loadingIndicator = createLoadingIndicator(container); }; lifecycles.postsetup = function() { - loadingIndicator?.remove(); - loadingIndicator = null; + if (this._loadingIndicator) { + this._loadingIndicator.remove(); + this._loadingIndicator = null; + } }; } @@ -40,10 +46,10 @@ export default function loading(p5, fn, lifecycles) { * @returns {HTMLElement} The loading indicator div element */ function createLoadingIndicator(container) { - if (!document.getElementById('loading-style')) { + if (!document.getElementById('p5-loading-style')) { const loadingStyle = document.createElement('style'); - loadingStyle.id = 'loading-style'; - loadingStyle.textContent = '@keyframes loading-spin { to { transform: rotate(360deg); } }'; + loadingStyle.id = 'p5-loading-style'; + loadingStyle.textContent = '@keyframes p5-loading-spin { to { transform: rotate(360deg); } }'; document.head.appendChild(loadingStyle); } @@ -59,7 +65,8 @@ function createLoadingIndicator(container) { border: 3px solid rgba(0, 0, 0, 0.1); border-top-color: rgba(0, 0, 0, 0.8); - animation: loading-spin 1s linear infinite; + animation: p5-loading-spin 1s linear infinite; + z-index: 9999; `; container.appendChild(indicator); From 1e4df8d479cfecbd7335d1622cfd3330f0e54e4f Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Tue, 28 Jul 2026 14:27:28 -0700 Subject: [PATCH 07/10] Updated test unit for instance mode and multiple instance testing for loading.js --- src/core/loading.js | 4 +- test/unit/core/loading.js | 79 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/core/loading.js b/src/core/loading.js index 16d677b2b3..21158962f6 100644 --- a/src/core/loading.js +++ b/src/core/loading.js @@ -17,7 +17,9 @@ */ export default function loading(p5, fn, lifecycles) { lifecycles.presetup = function() { - if (typeof window === 'undefined' || this._loadingIndicator) return; + if (typeof window === 'undefined' || this._loadingIndicator) { + return; + } const canvasParent = this.canvas?.parentElement; let container = this._userNode || canvasParent || document.body; diff --git a/test/unit/core/loading.js b/test/unit/core/loading.js index 79da1992d1..c7ee619200 100644 --- a/test/unit/core/loading.js +++ b/test/unit/core/loading.js @@ -16,8 +16,6 @@ suite('Loading indicator', function() { }); afterEach(function() { - lifecycles.postsetup?.(); - if (container) { container.remove(); container = null; @@ -52,7 +50,7 @@ suite('Loading indicator', function() { try { p.createCanvas(400, 400); - await load(7000); + await load(2000); p.background('#EB5580'); p.fill(255); @@ -78,4 +76,79 @@ suite('Loading indicator', function() { assert.deepEqual(p.fill.mock.calls, [[255]]); assert.deepEqual(p.circle.mock.calls, [[200, 200, 100], [12, 34, 20]]); }); + + test('test the loading indicator in an instance', function() { + const p = { + _userNode: container + }; + + lifecycles.presetup.call(p); + assert.exists(container.querySelector('.loading-indicator')); + + lifecycles.postsetup.call(p); + assert.isNull(container.querySelector('.loading-indicator')); + }); + + test('test multiple indicators for multiple instances', async function() { + const instance1 = document.createElement('div'); + const instance2 = document.createElement('div'); + document.body.appendChild(instance1); + document.body.appendChild(instance2); + + + let resolveLoad1; + let resolveLoad2; + + const load1 = async delay => { + await new Promise(resolve => { + resolveLoad1 = resolve; + }); + }; + + const load2 = async delay => { + await new Promise(resolve => { + resolveLoad2 = resolve; + }); + }; + + const p1 = { _userNode: instance1 }; + const p2 = { _userNode: instance2 }; + + const setup1 = (async function() { + lifecycles.presetup.call(p1); + try { + await load1(2000); + } + finally { + lifecycles.postsetup.call(p1); + } + })(); + + const setup2 = (async function() { + lifecycles.presetup.call(p2); + try { + await load2(4000); + } + finally { + lifecycles.postsetup.call(p2); + } + })(); + + assert.exists(instance1.querySelector('.loading-indicator'), 'Container 1 should have a spinner'); + assert.exists(instance2.querySelector('.loading-indicator'), 'Container 2 should have a spinner'); + + resolveLoad1(); + await setup1; + + assert.isNull(instance1.querySelector('.loading-indicator'), 'Container 1 spinner should be removed'); + assert.exists(instance2.querySelector('.loading-indicator'), 'Container 2 spinner MUST still exist'); + + resolveLoad2(); + await setup2; + + assert.isNull(instance2.querySelector('.loading-indicator'), 'Container 2 spinner should now be removed'); + + instance1.remove(); + instance2.remove(); + }); }); \ No newline at end of file From 41e23a1e940cd936aa46cb5afaa6737cdba0188a Mon Sep 17 00:00:00 2001 From: Johnny Huynh Date: Tue, 28 Jul 2026 15:42:17 -0700 Subject: [PATCH 08/10] Loading indicators can now appear in independent canvases instead of the center of the webpage. --- src/core/loading.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/loading.js b/src/core/loading.js index 21158962f6..2b592f0ef5 100644 --- a/src/core/loading.js +++ b/src/core/loading.js @@ -21,8 +21,7 @@ export default function loading(p5, fn, lifecycles) { return; } - const canvasParent = this.canvas?.parentElement; - let container = this._userNode || canvasParent || document.body; + let container = this._userNode || document.body; if (typeof container === 'string') { container = document.getElementById(container) || document.body; @@ -58,7 +57,7 @@ function createLoadingIndicator(container) { const indicator = document.createElement('div'); indicator.className = 'loading-indicator'; indicator.style.cssText = ` - position: fixed; + position: absolute; inset: 0; margin: auto; width: 30px; From f4f5e628eed7faed096e16249b067057786e00bf Mon Sep 17 00:00:00 2001 From: kit Date: Thu, 30 Jul 2026 17:30:20 +0200 Subject: [PATCH 09/10] Revert change that fails tests --- src/core/loading.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/loading.js b/src/core/loading.js index 2b592f0ef5..21158962f6 100644 --- a/src/core/loading.js +++ b/src/core/loading.js @@ -21,7 +21,8 @@ export default function loading(p5, fn, lifecycles) { return; } - let container = this._userNode || document.body; + const canvasParent = this.canvas?.parentElement; + let container = this._userNode || canvasParent || document.body; if (typeof container === 'string') { container = document.getElementById(container) || document.body; @@ -57,7 +58,7 @@ function createLoadingIndicator(container) { const indicator = document.createElement('div'); indicator.className = 'loading-indicator'; indicator.style.cssText = ` - position: absolute; + position: fixed; inset: 0; margin: auto; width: 30px; From 46b1a19f4c950231b1c8320f3df0d7a26bbd3cc5 Mon Sep 17 00:00:00 2001 From: Dave Pagurek Date: Thu, 30 Jul 2026 13:17:41 -0400 Subject: [PATCH 10/10] Merge pull request #9025 from okra-sf/perf/shape-hot-path Optimize the 2D shape hot path --- src/core/p5.Renderer.js | 34 +++-- src/core/p5.Renderer2D.js | 4 +- src/shape/custom_shapes.js | 244 ++++++++++++++++++------------------ test/unit/core/rendering.js | 32 +++++ test/unit/core/vertex.js | 78 ++++++++++++ 5 files changed, 255 insertions(+), 137 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..e1530f8e14 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._clipping && !!this.states.fillColor, + hasStroke: !this._clipping && !!this.states.strokeColor }); shape.accept(visitor); if (this._clipping) { diff --git a/src/shape/custom_shapes.js b/src/shape/custom_shapes.js index 305cf2473b..3d97e18e8a 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) { @@ -624,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; +// 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) } - - 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); - } - - clear() { - this.creators.clear(); - } -} +}; // ---- SHAPE ---- @@ -711,7 +661,7 @@ class Shape { constructor( vertexProperties, - primitiveShapeCreators = new PrimitiveShapeCreators() + primitiveShapeCreators = defaultPrimitiveShapeCreators ) { this.#initialVertexProperties = vertexProperties; this.#vertexProperties = vertexProperties; @@ -913,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, @@ -976,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) : @@ -1007,6 +955,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 +1114,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 +1160,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 +1222,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 +1243,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,28 +1364,33 @@ 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 + // 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); @@ -1511,7 +1503,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/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; 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(); + }); + }); });