From 0e602bd6988f4bfc34dbabd6cc6d6fe52e2f41c9 Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Thu, 22 Oct 2026 22:09:48 +0530 Subject: [PATCH 1/8] refactor(webgl): make Geometry extend GeometryPart --- src/webgl/p5.Geometry.js | 33 +++-------- test/unit/webgl/p5.GeometryPart.js | 95 +++++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 27 deletions(-) diff --git a/src/webgl/p5.Geometry.js b/src/webgl/p5.Geometry.js index b72a0ac327..072aaa724e 100644 --- a/src/webgl/p5.Geometry.js +++ b/src/webgl/p5.Geometry.js @@ -16,11 +16,14 @@ import { import { Vector } from '../math/p5.Vector'; import { downloadFile } from '../io/utilities'; -class Geometry { +class Geometry extends GeometryPart { constructor(detailX, detailY, callback, renderer) { + super( + `_p5_Geometry_${Geometry.nextId++}`, + createPartState() + ); this.renderer = renderer; - this.vertices = []; - + this.boundingBoxCache = null; //an array containing every vertex for stroke drawing @@ -40,43 +43,23 @@ class Geometry { // represents. See the doc comments for _addCap and _addJoin for diagrams. this.lineSides = new DataArray(); - this.vertexNormals = []; - - // per-vertex surface tangents for normal mapping, stored flat as - // [x, y, z, w] where w is the bitangent handedness. computeTangents() fills - // this; empty until a normal-mapped model needs it. - this.vertexTangents = []; - - this.faces = []; - - this.uvs = []; // a 2D array containing edge connectivity pattern for create line vertices //based on faces for most objects; this.edges = []; - this.vertexColors = []; - + // One color per vertex representing the stroke color at that vertex this.vertexStrokeColors = []; - this.userVertexProperties = {}; - // One color per line vertex, generated automatically based on // vertexStrokeColors in _edgesToVertices() this.lineVertexColors = new DataArray(); this.detailX = detailX !== undefined ? detailX : 1; this.detailY = detailY !== undefined ? detailY : 1; - this.dirtyFlags = {}; + this._hasFillTransparency = undefined; this._hasStrokeTransparency = undefined; - this.gid = `_p5_Geometry_${Geometry.nextId}`; - Geometry.nextId++; - - // a geometry can act as its own single part: this is the default material - // state used when it is drawn that way (see _wrapInSinglePart). - this.partState = createPartState(); - // every geometry is one or more parts (see p5.GeometryPart). loaders that // know about materials fill parts themselves; anything built the old way // becomes its own single part below. diff --git a/test/unit/webgl/p5.GeometryPart.js b/test/unit/webgl/p5.GeometryPart.js index 1d7a3a1bf1..81c4356e9e 100644 --- a/test/unit/webgl/p5.GeometryPart.js +++ b/test/unit/webgl/p5.GeometryPart.js @@ -74,6 +74,18 @@ suite('p5.GeometryPart', function () { expect(geom.parts.length).toEqual(1); }); + test('Geometry inherits from GeometryPart', function () { + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + expect(geom).toBeInstanceOf(p5.GeometryPart); + expect(geom).toBeInstanceOf(p5.Geometry); + }); + test('the part is a live view onto the geometry buffers', function () { const geom = new p5.Geometry( undefined, @@ -81,6 +93,7 @@ suite('p5.GeometryPart', function () { undefined, myp5._renderer ); + expect(geom.parts[0].vertices).toBe(geom.vertices); expect(geom.parts[0].faces).toBe(geom.faces); }); @@ -92,11 +105,86 @@ suite('p5.GeometryPart', function () { undefined, myp5._renderer ); + geom.gid = 'my-model'; + expect(geom.parts[0]).toBe(geom); expect(geom.parts[0].gid).toEqual('my-model'); }); + test('single-part geometry shares UVs and dirty flags with itself', function () { + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + expect(geom.parts[0]).toBe(geom); + expect(geom.parts[0].uvs).toBe(geom.uvs); + expect(geom.parts[0].dirtyFlags).toBe(geom.dirtyFlags); + expect(geom.parts[0].userVertexProperties).toBe( + geom.userVertexProperties + ); + }); + + test('separate geometries have independent mutable state', function () { + const first = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + const second = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + expect(first.gid).not.toBe(second.gid); + expect(first.vertices).not.toBe(second.vertices); + expect(first.faces).not.toBe(second.faces); + expect(first.uvs).not.toBe(second.uvs); + expect(first.vertexNormals).not.toBe(second.vertexNormals); + expect(first.vertexTangents).not.toBe(second.vertexTangents); + expect(first.vertexColors).not.toBe(second.vertexColors); + expect(first.dirtyFlags).not.toBe(second.dirtyFlags); + expect(first.userVertexProperties).not.toBe( + second.userVertexProperties + ); + expect(first.partState).not.toBe(second.partState); + }); + + test('constructor callback runs after geometry initialization', function () { + let callbackCalled = false; + + const geom = new p5.Geometry( + 2, + 3, + function () { + callbackCalled = true; + + expect(this).toBeInstanceOf(p5.GeometryPart); + expect(this.vertices).toEqual([]); + expect(this.uvs).toEqual([]); + expect(this.edges).toEqual([]); + expect(this.parts).toEqual([]); + expect(this.detailX).toBe(2); + expect(this.detailY).toBe(3); + expect(this.renderer).toBe(myp5._renderer); + + this.vertices.push(new p5.Vector(1, 2, 3)); + }, + myp5._renderer + ); + + expect(callbackCalled).toBe(true); + expect(geom.vertices).toHaveLength(1); + expect(geom.parts[0]).toBe(geom); + }); + test('a built-in primitive also gets one part', function () { const geom = new p5.Geometry( 1, @@ -106,6 +194,7 @@ suite('p5.GeometryPart', function () { }, myp5._renderer ); + expect(geom.parts.length).toEqual(1); expect(geom.parts[0].vertices.length).toEqual(1); }); @@ -121,13 +210,15 @@ suite('p5.GeometryPart', function () { const fillSpy = vi.spyOn(renderer, '_drawFills'); const strokeSpy = vi.spyOn(renderer, '_drawStrokes'); + myp5.background(255); myp5.stroke(0); myp5.model(model); - // one fill draw per material part. + // One fill draw per material part. expect(fillSpy).toHaveBeenCalledTimes(model.parts.length); - // the model's outline is still stroked. + + // The model's outline is still stroked. expect(strokeSpy).toHaveBeenCalled(); fillSpy.mockRestore(); From a433329ff4fee00bf0c69cf89c8a3129b327e673 Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Thu, 22 Oct 2026 23:04:56 +0530 Subject: [PATCH 2/8] refactor(webgl): move face normals and tangents to GeometryPart --- src/webgl/p5.Geometry.js | 130 +---------------------------- src/webgl/p5.GeometryPart.js | 97 +++++++++++++++++++++ test/unit/webgl/p5.GeometryPart.js | 80 ++++++++++++++++++ 3 files changed, 180 insertions(+), 127 deletions(-) diff --git a/src/webgl/p5.Geometry.js b/src/webgl/p5.Geometry.js index 072aaa724e..17519127ed 100644 --- a/src/webgl/p5.Geometry.js +++ b/src/webgl/p5.Geometry.js @@ -23,7 +23,7 @@ class Geometry extends GeometryPart { createPartState() ); this.renderer = renderer; - + this.boundingBoxCache = null; //an array containing every vertex for stroke drawing @@ -46,7 +46,7 @@ class Geometry extends GeometryPart { // a 2D array containing edge connectivity pattern for create line vertices //based on faces for most objects; this.edges = []; - + // One color per vertex representing the stroke color at that vertex this.vertexStrokeColors = []; @@ -55,9 +55,7 @@ class Geometry extends GeometryPart { this.lineVertexColors = new DataArray(); this.detailX = detailX !== undefined ? detailX : 1; this.detailY = detailY !== undefined ? detailY : 1; - - this._hasFillTransparency = undefined; this._hasStrokeTransparency = undefined; // every geometry is one or more parts (see p5.GeometryPart). loaders that @@ -253,18 +251,6 @@ class Geometry extends GeometryPart { this.dirtyFlags = {}; } - hasFillTransparency() { - if (this._hasFillTransparency === undefined) { - this._hasFillTransparency = false; - for (let i = 0; i < this.vertexColors.length; i += 4) { - if (this.vertexColors[i + 3] < 1) { - this._hasFillTransparency = true; - break; - } - } - } - return this._hasFillTransparency; - } hasStrokeTransparency() { if (this._hasStrokeTransparency === undefined) { this._hasStrokeTransparency = false; @@ -862,27 +848,7 @@ class Geometry extends GeometryPart { return this; } - _getFaceNormal(faceId) { - //This assumes that vA->vB->vC is a counter-clockwise ordering - const face = this.faces[faceId]; - const vA = this.vertices[face[0]]; - const vB = this.vertices[face[1]]; - const vC = this.vertices[face[2]]; - const ab = Vector.sub(vB, vA); - const ac = Vector.sub(vC, vA); - const n = Vector.cross(ab, ac); - const ln = Vector.mag(n); - let sinAlpha = ln / (Vector.mag(ab) * Vector.mag(ac)); - if (sinAlpha === 0 || isNaN(sinAlpha)) { - console.warn( - 'p5.Geometry.prototype._getFaceNormal:', - 'face has colinear sides or a repeated vertex' - ); - return n; - } - if (sinAlpha > 1) sinAlpha = 1; // handle float rounding error - return n.mult(Math.asin(sinAlpha) / ln); - } + /** * Calculates the normal vector for each vertex on the geometry. * @@ -1374,79 +1340,6 @@ class Geometry extends GeometryPart { } - /** - * computes a per-vertex surface tangent from the uvs, needed for normal - * (bump) mapping. the tangent points along the +u texture direction; its w - * component stores the bitangent handedness so the shader can rebuild the - * bitangent as cross(normal, tangent) * w. results are stored flat as - * [x, y, z, w] per vertex on this.vertexTangents. needs uvs and vertex - * normals, so run computeNormals() first if the model has none. - * @private - * @chainable - */ - computeTangents() { - const vertices = this.vertices; - const faces = this.faces; - const uvs = this.uvs.flat(); - const normals = this.vertexNormals; - - // nothing to build a tangent basis from without uvs and normals - if (uvs.length === 0 || normals.length === 0) { - this.vertexTangents = []; - return this; - } - - // accumulate the +u direction (tan) and +v direction (bitan) per vertex - const tan = []; - const bitan = []; - for (let i = 0; i < vertices.length; i++) { - tan.push(new Vector(0, 0, 0)); - bitan.push(new Vector(0, 0, 0)); - } - const uvAt = i => ({ x: uvs[i * 2] || 0, y: uvs[i * 2 + 1] || 0 }); - - for (const face of faces) { - const [i0, i1, i2] = face; - const e1 = Vector.sub(vertices[i1], vertices[i0]); - const e2 = Vector.sub(vertices[i2], vertices[i0]); - const w0 = uvAt(i0); - const w1 = uvAt(i1); - const w2 = uvAt(i2); - const du1 = w1.x - w0.x; - const dv1 = w1.y - w0.y; - const du2 = w2.x - w0.x; - const dv2 = w2.y - w0.y; - - const denom = du1 * dv2 - du2 * dv1; - const r = denom === 0 ? 0 : 1 / denom; - const sdir = Vector.sub(Vector.mult(e1, dv2), Vector.mult(e2, dv1)).mult(r); - const tdir = Vector.sub(Vector.mult(e2, du1), Vector.mult(e1, du2)).mult(r); - - for (const idx of face) { - tan[idx].add(sdir); - bitan[idx].add(tdir); - } - } - - // orthonormalise each tangent against its normal and record handedness - const tangents = []; - for (let i = 0; i < vertices.length; i++) { - const n = normals[i] || new Vector(0, 0, 1); - let t = Vector.sub(tan[i], Vector.mult(n, n.dot(tan[i]))); - if (t.magSq() === 0) { - // degenerate uvs: pick any direction perpendicular to the normal - const seed = Math.abs(n.x) < 0.9 ? new Vector(1, 0, 0) : new Vector(0, 1, 0); - t = Vector.sub(seed, Vector.mult(n, n.dot(seed))); - } - t.normalize(); - const handedness = Vector.cross(n, t).dot(bitan[i]) < 0 ? -1 : 1; - tangents.push(t.x, t.y, t.z, handedness); - } - - this.vertexTangents = tangents; - return this; - } - /** * Averages the vertex normals. Used in curved * surfaces @@ -1985,24 +1878,7 @@ class Geometry extends GeometryPart { * model(geo); * } * - * @param {String} propertyName the name of the vertex property. - * @param {Number|Number[]} data the data tied to the vertex property. - * @param {Number} [size] optional size of each unit of data. */ - vertexProperty(propertyName, data, size) { - let prop; - if (!this.userVertexProperties[propertyName]) { - prop = this.userVertexProperties[propertyName] = - this._userVertexPropertyHelper(propertyName, data, size); - } - prop = this.userVertexProperties[propertyName]; - if (size) { - prop.pushDirect(data); - } else { - prop.setCurrentData(data); - prop.pushCurrentData(); - } - } _userVertexPropertyHelper(propertyName, data, size) { // shared with GeometryPart so parts carry custom attributes identically diff --git a/src/webgl/p5.GeometryPart.js b/src/webgl/p5.GeometryPart.js index 678b97c020..8681bb6d99 100644 --- a/src/webgl/p5.GeometryPart.js +++ b/src/webgl/p5.GeometryPart.js @@ -1,3 +1,5 @@ +import { Vector } from '../math/p5.Vector'; + /** * @module Shape * @submodule 3D Primitives @@ -128,6 +130,101 @@ class GeometryPart { } return false; } + + _getFaceNormal(faceId) { + //This assumes that vA->vB->vC is a counter-clockwise ordering + const face = this.faces[faceId]; + const vA = this.vertices[face[0]]; + const vB = this.vertices[face[1]]; + const vC = this.vertices[face[2]]; + const ab = Vector.sub(vB, vA); + const ac = Vector.sub(vC, vA); + const n = Vector.cross(ab, ac); + const ln = Vector.mag(n); + let sinAlpha = ln / (Vector.mag(ab) * Vector.mag(ac)); + if (sinAlpha === 0 || isNaN(sinAlpha)) { + console.warn( + 'p5.Geometry.prototype._getFaceNormal:', + 'face has colinear sides or a repeated vertex' + ); + return n; + } + if (sinAlpha > 1) sinAlpha = 1; // handle float rounding error + return n.mult(Math.asin(sinAlpha) / ln); + } + + /** + * computes a per-vertex surface tangent from the uvs, needed for normal + * (bump) mapping. the tangent points along the +u texture direction; its w + * component stores the bitangent handedness so the shader can rebuild the + * bitangent as cross(normal, tangent) * w. results are stored flat as + * [x, y, z, w] per vertex on this.vertexTangents. needs uvs and vertex + * normals, so run computeNormals() first if the model has none. + * @private + * @chainable + */ + computeTangents() { + const vertices = this.vertices; + const faces = this.faces; + const uvs = this.uvs.flat(); + const normals = this.vertexNormals; + + // nothing to build a tangent basis from without uvs and normals + if (uvs.length === 0 || normals.length === 0) { + this.vertexTangents = []; + return this; + } + + // accumulate the +u direction (tan) and +v direction (bitan) per vertex + const tan = []; + const bitan = []; + for (let i = 0; i < vertices.length; i++) { + tan.push(new Vector(0, 0, 0)); + bitan.push(new Vector(0, 0, 0)); + } + const uvAt = i => ({ x: uvs[i * 2] || 0, y: uvs[i * 2 + 1] || 0 }); + + for (const face of faces) { + const [i0, i1, i2] = face; + const e1 = Vector.sub(vertices[i1], vertices[i0]); + const e2 = Vector.sub(vertices[i2], vertices[i0]); + const w0 = uvAt(i0); + const w1 = uvAt(i1); + const w2 = uvAt(i2); + const du1 = w1.x - w0.x; + const dv1 = w1.y - w0.y; + const du2 = w2.x - w0.x; + const dv2 = w2.y - w0.y; + + const denom = du1 * dv2 - du2 * dv1; + const r = denom === 0 ? 0 : 1 / denom; + const sdir = Vector.sub(Vector.mult(e1, dv2), Vector.mult(e2, dv1)).mult(r); + const tdir = Vector.sub(Vector.mult(e2, du1), Vector.mult(e1, du2)).mult(r); + + for (const idx of face) { + tan[idx].add(sdir); + bitan[idx].add(tdir); + } + } + + // orthonormalise each tangent against its normal and record handedness + const tangents = []; + for (let i = 0; i < vertices.length; i++) { + const n = normals[i] || new Vector(0, 0, 1); + let t = Vector.sub(tan[i], Vector.mult(n, n.dot(tan[i]))); + if (t.magSq() === 0) { + // degenerate uvs: pick any direction perpendicular to the normal + const seed = Math.abs(n.x) < 0.9 ? new Vector(1, 0, 0) : new Vector(0, 1, 0); + t = Vector.sub(seed, Vector.mult(n, n.dot(seed))); + } + t.normalize(); + const handedness = Vector.cross(n, t).dot(bitan[i]) < 0 ? -1 : 1; + tangents.push(t.x, t.y, t.z, handedness); + } + + this.vertexTangents = tangents; + return this; + } } export { GeometryPart, createPartState, createUserVertexProperty }; diff --git a/test/unit/webgl/p5.GeometryPart.js b/test/unit/webgl/p5.GeometryPart.js index 81c4356e9e..771f46e1de 100644 --- a/test/unit/webgl/p5.GeometryPart.js +++ b/test/unit/webgl/p5.GeometryPart.js @@ -198,6 +198,86 @@ suite('p5.GeometryPart', function () { expect(geom.parts.length).toEqual(1); expect(geom.parts[0].vertices.length).toEqual(1); }); + + test('Geometry inherits vertexProperty from GeometryPart', function () { + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + expect(geom.vertexProperty).toBe( + p5.GeometryPart.prototype.vertexProperty + ); + + geom.vertexProperty('customAttr', [1, 2, 3], 3); + + expect(geom.userVertexProperties.customAttr).toBeDefined(); + expect(geom.customAttrSrc).toEqual([1, 2, 3]); + }); + + test('Geometry inherits hasFillTransparency from GeometryPart', function () { + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + expect(geom.hasFillTransparency).toBe( + p5.GeometryPart.prototype.hasFillTransparency + ); + }); + + test('detects transparent vertex colors after they change', function () { + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + geom.vertexColors = [1, 1, 1, 1]; + expect(geom.hasFillTransparency()).toBe(false); + + geom.vertexColors = [1, 1, 1, 0.5]; + expect(geom.hasFillTransparency()).toBe(true); + + geom.vertexColors = [1, 1, 1, 1]; + expect(geom.hasFillTransparency()).toBe(false); + }); + + test('detects transparent material fill on a single-part geometry', function () { + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + myp5._renderer + ); + + geom.partState.fill = [1, 0, 0, 0.5]; + + expect(geom.hasFillTransparency()).toBe(true); + }); + + test('computes face normals for a GeometryPart', function () { + const part = new p5.GeometryPart('face-normal-test'); + + part.vertices = [ + new p5.Vector(0, 0, 0), + new p5.Vector(1, 0, 0), + new p5.Vector(0, 1, 0) + ]; + + part.faces = [[0, 1, 2]]; + + const normal = part._getFaceNormal(0); + + expect(normal.x).toBeCloseTo(0); + expect(normal.y).toBeCloseTo(0); + expect(normal.z).toBeCloseTo(Math.PI / 2); + }); }); suite('multi-material rendering', function () { From 5d536563e813300b9aee13f2da442a4aed1d658d Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Fri, 23 Oct 2026 20:13:51 +0530 Subject: [PATCH 3/8] fix(webgl): gate fill rendering per geometry part --- src/core/p5.Renderer3D.js | 4 +- src/webgl/p5.GeometryPart.js | 2 +- test/unit/webgl/p5.GeometryPart.js | 116 +++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 3 deletions(-) diff --git a/src/core/p5.Renderer3D.js b/src/core/p5.Renderer3D.js index d6cd1ce5f5..6f793fe26d 100644 --- a/src/core/p5.Renderer3D.js +++ b/src/core/p5.Renderer3D.js @@ -567,7 +567,7 @@ export class Renderer3D extends Renderer { model(model, count) { count = count ?? this._instanceCount ?? 1; - if (model.vertices.length > 0) { + if (model.vertices.length > 0 || model.parts.some(part => part.vertices.length > 0)) { if (this.geometryBuilder) { this.geometryBuilder.addRetained(model); } else { @@ -605,7 +605,6 @@ export class Renderer3D extends Renderer { if ( this.states.fillColor && - geometry.vertices.length >= 3 && ![constants.LINES, constants.POINTS].includes(mode) ) { // draw every part. a geometry always has at least one part (the @@ -613,6 +612,7 @@ export class Renderer3D extends Renderer { // fallback is needed. a part with no material state draws straight; // multi-material parts apply their own material around the draw. for (const part of geometry.parts) { + if (part.vertices.length < 3) continue; const state = part.partState; // fill/ambientColor/specularColor are arrays and texture is a p5.Image, // so they're truthy when set; shininess is a number where 0 is valid. diff --git a/src/webgl/p5.GeometryPart.js b/src/webgl/p5.GeometryPart.js index 8681bb6d99..56828d7747 100644 --- a/src/webgl/p5.GeometryPart.js +++ b/src/webgl/p5.GeometryPart.js @@ -131,7 +131,7 @@ class GeometryPart { return false; } - _getFaceNormal(faceId) { + _getFaceNormal(faceId) { //This assumes that vA->vB->vC is a counter-clockwise ordering const face = this.faces[faceId]; const vA = this.vertices[face[0]]; diff --git a/test/unit/webgl/p5.GeometryPart.js b/test/unit/webgl/p5.GeometryPart.js index 771f46e1de..23dc4b38cf 100644 --- a/test/unit/webgl/p5.GeometryPart.js +++ b/test/unit/webgl/p5.GeometryPart.js @@ -305,4 +305,120 @@ suite('p5.GeometryPart', function () { strokeSpy.mockRestore(); }); }); + + test('draws parts when the aggregate has no vertices', function () { + const renderer = myp5.createCanvas(50, 50, myp5.WEBGL); + + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + renderer + ); + + const partA = new p5.GeometryPart(`${geom.gid}|part0`); + const partB = new p5.GeometryPart(`${geom.gid}|part1`); + + partA.vertices = [ + new p5.Vector(0, 0, 0), + new p5.Vector(10, 0, 0), + new p5.Vector(0, 10, 0) + ]; + partA.faces = [[0, 1, 2]]; + + partB.vertices = [ + new p5.Vector(0, 0, 0), + new p5.Vector(-10, 0, 0), + new p5.Vector(0, -10, 0) + ]; + partB.faces = [[0, 1, 2]]; + + geom.parts = [partA, partB]; + + expect(geom.vertices).toHaveLength(0); + + const fillSpy = vi.spyOn(renderer, '_drawFills'); + + try { + myp5.fill(255); + myp5.noStroke(); + myp5.model(geom); + + expect(fillSpy).toHaveBeenCalledTimes(2); + expect(fillSpy.mock.calls[0][0]).toBe(partA); + expect(fillSpy.mock.calls[1][0]).toBe(partB); + } finally { + fillSpy.mockRestore(); + } + }); + + test('skips empty parts but draws valid parts', function () { + const renderer = myp5.createCanvas(50, 50, myp5.WEBGL); + + const geom = new p5.Geometry( + undefined, + undefined, + undefined, + renderer + ); + + const emptyPart = new p5.GeometryPart(`${geom.gid}|empty`); + const validPart = new p5.GeometryPart(`${geom.gid}|valid`); + + validPart.vertices = [ + new p5.Vector(0, 0, 0), + new p5.Vector(10, 0, 0), + new p5.Vector(0, 10, 0) + ]; + validPart.faces = [[0, 1, 2]]; + + geom.parts = [emptyPart, validPart]; + + const fillSpy = vi.spyOn(renderer, '_drawFills'); + + try { + myp5.fill(255); + myp5.noStroke(); + myp5.model(geom); + + expect(fillSpy).toHaveBeenCalledTimes(1); + expect(fillSpy.mock.calls[0][0]).toBe(validPart); + } finally { + fillSpy.mockRestore(); + } + }); + + test('continues drawing single-part geometry', function () { + const renderer = myp5.createCanvas(50, 50, myp5.WEBGL); + + const geom = new p5.Geometry( + undefined, + undefined, + function () { + this.vertices.push( + new p5.Vector(0, 0, 0), + new p5.Vector(10, 0, 0), + new p5.Vector(0, 10, 0) + ); + + this.faces.push([0, 1, 2]); + }, + renderer + ); + + expect(geom.parts[0]).toBe(geom); + + const fillSpy = vi.spyOn(renderer, '_drawFills'); + + try { + myp5.fill(255); + myp5.noStroke(); + myp5.model(geom); + + expect(fillSpy).toHaveBeenCalledTimes(1); + expect(fillSpy.mock.calls[0][0]).toBe(geom); + } finally { + fillSpy.mockRestore(); + } + }); }); From f852a5dea6c97fe4261e7a6f23fe3a94917707e7 Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Mon, 2 Nov 2026 12:54:46 +0530 Subject: [PATCH 4/8] refactor(webgl): move edge and stroke generation into GeometryPart --- src/webgl/3d_primitives.js | 2 +- src/webgl/p5.Geometry.js | 350 ---------------------------- src/webgl/p5.GeometryPart.js | 356 ++++++++++++++++++++++++++++- test/unit/webgl/p5.GeometryPart.js | 80 +++++++ 4 files changed, 436 insertions(+), 352 deletions(-) diff --git a/src/webgl/3d_primitives.js b/src/webgl/3d_primitives.js index 15b22fe1ec..d9f0c9b2ad 100644 --- a/src/webgl/3d_primitives.js +++ b/src/webgl/3d_primitives.js @@ -1681,7 +1681,7 @@ function primitives3D(p5, fn) { arcGeom.computeNormals(); if (detail <= 50) { - arcGeom._edgesToVertices(arcGeom); + arcGeom._edgesToVertices(); } else if (this.states.strokeColor) { console.log( `Cannot apply a stroke to an ${shape} with more than 50 detail` diff --git a/src/webgl/p5.Geometry.js b/src/webgl/p5.Geometry.js index 17519127ed..8b79c7cacf 100644 --- a/src/webgl/p5.Geometry.js +++ b/src/webgl/p5.Geometry.js @@ -7,7 +7,6 @@ //some of the functions are adjusted from Three.js(http://threejs.org) import * as constants from '../core/constants'; -import { DataArray } from './p5.DataArray'; import { GeometryPart, createPartState, @@ -26,38 +25,9 @@ class Geometry extends GeometryPart { this.boundingBoxCache = null; - //an array containing every vertex for stroke drawing - this.lineVertices = new DataArray(); - - // The tangents going into or out of a vertex on a line. Along a straight - // line segment, both should be equal. At an endpoint, one or the other - // will not exist and will be all 0. In joins between line segments, they - // may be different, as they will be the tangents on either side of the join. - this.lineTangentsIn = new DataArray(); - this.lineTangentsOut = new DataArray(); - - // When drawing lines with thickness, entries in this buffer represent which - // side of the centerline the vertex will be placed. The sign of the number - // will represent the side of the centerline, and the absolute value will be - // used as an enum to determine which part of the cap or join each vertex - // represents. See the doc comments for _addCap and _addJoin for diagrams. - this.lineSides = new DataArray(); - - // a 2D array containing edge connectivity pattern for create line vertices - //based on faces for most objects; - this.edges = []; - - // One color per vertex representing the stroke color at that vertex - this.vertexStrokeColors = []; - - // One color per line vertex, generated automatically based on - // vertexStrokeColors in _edgesToVertices() - this.lineVertexColors = new DataArray(); this.detailX = detailX !== undefined ? detailX : 1; this.detailY = detailY !== undefined ? detailY : 1; - this._hasStrokeTransparency = undefined; - // every geometry is one or more parts (see p5.GeometryPart). loaders that // know about materials fill parts themselves; anything built the old way // becomes its own single part below. @@ -251,19 +221,6 @@ class Geometry extends GeometryPart { this.dirtyFlags = {}; } - hasStrokeTransparency() { - if (this._hasStrokeTransparency === undefined) { - this._hasStrokeTransparency = false; - for (let i = 0; i < this.lineVertexColors.length; i += 4) { - if (this.lineVertexColors[i + 3] < 1) { - this._hasStrokeTransparency = true; - break; - } - } - } - return this._hasStrokeTransparency; - } - /** * Removes the geometry’s internal colors. * @@ -1399,313 +1356,6 @@ class Geometry extends GeometryPart { return this; } - /** - * Create a 2D array for establishing stroke connections - * @private - * @chainable - */ - _makeTriangleEdges() { - this.edges.length = 0; - - for (let j = 0; j < this.faces.length; j++) { - this.edges.push([this.faces[j][0], this.faces[j][1]]); - this.edges.push([this.faces[j][1], this.faces[j][2]]); - this.edges.push([this.faces[j][2], this.faces[j][0]]); - } - - return this; - } - - /** - * @example - * let tetrahedron; - * function setup() { - * createCanvas(200, 200, WEBGL); - * describe('A rotating tetrahedron'); - * - * tetrahedron = new p5.Geometry(); - * - * // Give each geometry a unique gid - * tetrahedron.gid = 'tetrahedron'; - * - * // Add four points of the tetrahedron - * - * let radius = 50; - * // A 2D triangle: - * tetrahedron.vertices.push(createVector(radius, 0, 0)); - * tetrahedron.vertices.push(createVector(radius, 0, 0).rotate(TWO_PI / 3)); - * tetrahedron.vertices.push(createVector(radius, 0, 0).rotate(TWO_PI * 2 / 3)); - * // Add a tip in the z axis: - * tetrahedron.vertices.push(createVector(0, 0, radius)); - * - * // Create the four faces by connecting the sets of three points - * tetrahedron.faces.push([0, 1, 2]); - * tetrahedron.faces.push([0, 1, 3]); - * tetrahedron.faces.push([0, 2, 3]); - * tetrahedron.faces.push([1, 2, 3]); - * tetrahedron.makeEdgesFromFaces(); - * } - * - * function draw() { - * background(200); - * strokeWeight(2); - * orbitControl(); - * rotateY(millis() * 0.001); - * model(tetrahedron); - * } - */ - makeEdgesFromFaces() { - this._makeTriangleEdges(); - } - - /** - * Converts each line segment into the vertices and vertex attributes needed - * to turn the line into a polygon on screen. This will include: - * - Two triangles line segment to create a rectangle - * - Two triangles per endpoint to create a stroke cap rectangle. A fragment - * shader is responsible for displaying the appropriate cap style within - * that rectangle. - * - Four triangles per join between adjacent line segments, creating a quad on - * either side of the join, perpendicular to the lines. A vertex shader will - * discard the quad in the "elbow" of the join, and a fragment shader will - * display the appropriate join style within the remaining quad. - * - * @private - * @chainable - */ - _edgesToVertices() { - this.lineVertices.clear(); - this.lineTangentsIn.clear(); - this.lineTangentsOut.clear(); - this.lineSides.clear(); - - const potentialCaps = new Map(); - const connected = new Set(); - let lastValidDir; - for (let i = 0; i < this.edges.length; i++) { - const prevEdge = this.edges[i - 1]; - const currEdge = this.edges[i]; - const isPoint = currEdge[0] === currEdge[1]; - const begin = this.vertices[currEdge[0]]; - const end = this.vertices[currEdge[1]]; - const prevColor = - this.vertexStrokeColors.length > 0 && prevEdge - ? this.vertexStrokeColors.slice( - prevEdge[1] * 4, - (prevEdge[1] + 1) * 4 - ) - : [0, 0, 0, 0]; - const fromColor = - this.vertexStrokeColors.length > 0 - ? this.vertexStrokeColors.slice( - currEdge[0] * 4, - (currEdge[0] + 1) * 4 - ) - : [0, 0, 0, 0]; - const toColor = - this.vertexStrokeColors.length > 0 - ? this.vertexStrokeColors.slice( - currEdge[1] * 4, - (currEdge[1] + 1) * 4 - ) - : [0, 0, 0, 0]; - const dir = isPoint - ? new Vector(0, 1, 0) - : end.copy().sub(begin).normalize(); - const dirOK = dir.magSq() > 0; - if (dirOK) { - this._addSegment(begin, end, fromColor, toColor, dir); - } - if (!this.renderer?._simpleLines) { - if (i > 0 && prevEdge[1] === currEdge[0]) { - if (!connected.has(currEdge[0])) { - connected.add(currEdge[0]); - potentialCaps.delete(currEdge[0]); - // Add a join if this segment shares a vertex with the previous. Skip - // actually adding join vertices if either the previous segment or this - // one has a length of 0. - // - // Don't add a join if the tangents point in the same direction, which - // would mean the edges line up exactly, and there is no need for a join. - if (lastValidDir && dirOK && dir.dot(lastValidDir) < 1 - 1e-8) { - this._addJoin(begin, lastValidDir, dir, fromColor); - } - } - } else if (isPoint) { - this._addCap(begin, dir.copy().mult(-1), fromColor); - this._addCap(begin, dir, fromColor); - } else { - // Start a new line - if (dirOK && !connected.has(currEdge[0])) { - const existingCap = potentialCaps.get(currEdge[0]); - if (existingCap) { - this._addJoin(begin, existingCap.dir, dir, fromColor); - potentialCaps.delete(currEdge[0]); - connected.add(currEdge[0]); - } else { - potentialCaps.set(currEdge[0], { - point: begin, - dir: dir.copy().mult(-1), - color: fromColor - }); - } - } - if (!isPoint && lastValidDir && !connected.has(prevEdge[1])) { - const existingCap = potentialCaps.get(prevEdge[1]); - if (existingCap) { - this._addJoin( - this.vertices[prevEdge[1]], - lastValidDir, - existingCap.dir.copy().mult(-1), - prevColor - ); - potentialCaps.delete(prevEdge[1]); - connected.add(prevEdge[1]); - } else { - // Close off the last segment with a cap - potentialCaps.set(prevEdge[1], { - point: this.vertices[prevEdge[1]], - dir: lastValidDir, - color: prevColor - }); - } - lastValidDir = undefined; - } - } - - if (i === this.edges.length - 1 && !connected.has(currEdge[1])) { - const existingCap = potentialCaps.get(currEdge[1]); - if (existingCap) { - this._addJoin(end, dir, existingCap.dir.copy().mult(-1), toColor); - potentialCaps.delete(currEdge[1]); - connected.add(currEdge[1]); - } else { - potentialCaps.set(currEdge[1], { - point: end, - dir, - color: toColor - }); - } - } - - if (dirOK) { - lastValidDir = dir; - } - } - } - for (const { point, dir, color } of potentialCaps.values()) { - this._addCap(point, dir, color); - } - return this; - } - - /** - * Adds the vertices and vertex attributes for two triangles making a rectangle - * for a straight line segment. A vertex shader is responsible for picking - * proper coordinates on the screen given the centerline positions, the tangent, - * and the side of the centerline each vertex belongs to. Sides follow the - * following scheme: - * - * -1 -1 - * o-------------o - * | | - * o-------------o - * 1 1 - * - * @private - * @chainable - */ - _addSegment(begin, end, fromColor, toColor, dir) { - const a = begin.array(); - const b = end.array(); - const dirArr = dir.array(); - this.lineSides.push(1, 1, -1, 1, -1, -1); - for (const tangents of [this.lineTangentsIn, this.lineTangentsOut]) { - for (let i = 0; i < 6; i++) { - tangents.push(...dirArr); - } - } - this.lineVertices.push(...a, ...b, ...a, ...b, ...b, ...a); - if (!this.renderer?._simpleLines) { - this.lineVertexColors.push( - ...fromColor, - ...toColor, - ...fromColor, - ...toColor, - ...toColor, - ...fromColor - ); - } - return this; - } - - /** - * Adds the vertices and vertex attributes for two triangles representing the - * stroke cap of a line. A fragment shader is responsible for displaying the - * appropriate cap style within the rectangle they make. - * - * The lineSides buffer will include the following values for the points on - * the cap rectangle: - * - * -1 -2 - * -----------o---o - * | | - * -----------o---o - * 1 2 - * @private - * @chainable - */ - _addCap(point, tangent, color) { - const ptArray = point.array(); - const tanInArray = tangent.array(); - const tanOutArray = [0, 0, 0]; - for (let i = 0; i < 6; i++) { - this.lineVertices.push(...ptArray); - this.lineTangentsIn.push(...tanInArray); - this.lineTangentsOut.push(...tanOutArray); - this.lineVertexColors.push(...color); - } - this.lineSides.push(-1, 2, -2, 1, 2, -1); - return this; - } - - /** - * Adds the vertices and vertex attributes for four triangles representing a - * join between two adjacent line segments. This creates a quad on either side - * of the shared vertex of the two line segments, with each quad perpendicular - * to the lines. A vertex shader will discard all but the quad in the "elbow" of - * the join, and a fragment shader will display the appropriate join style - * within the remaining quad. - * - * The lineSides buffer will include the following values for the points on - * the join rectangles: - * - * -1 -2 - * -------------o----o - * | | - * 1 o----o----o -3 - * | | 0 | - * --------o----o | - * 2| 3 | - * | | - * | | - * @private - * @chainable - */ - _addJoin(point, fromTangent, toTangent, color) { - const ptArray = point.array(); - const tanInArray = fromTangent.array(); - const tanOutArray = toTangent.array(); - for (let i = 0; i < 12; i++) { - this.lineVertices.push(...ptArray); - this.lineTangentsIn.push(...tanInArray); - this.lineTangentsOut.push(...tanOutArray); - this.lineVertexColors.push(...color); - } - this.lineSides.push(-1, -3, -2, -1, 0, -3); - this.lineSides.push(3, 1, 2, 3, 0, 1); - return this; - } /** * Transforms the geometry's vertices to fit snugly within a 100×100×100 box diff --git a/src/webgl/p5.GeometryPart.js b/src/webgl/p5.GeometryPart.js index 56828d7747..fb459dc956 100644 --- a/src/webgl/p5.GeometryPart.js +++ b/src/webgl/p5.GeometryPart.js @@ -1,5 +1,5 @@ import { Vector } from '../math/p5.Vector'; - +import { DataArray } from './p5.DataArray'; /** * @module Shape * @submodule 3D Primitives @@ -97,6 +97,36 @@ class GeometryPart { this.uvs = []; this.vertexColors = []; + // A 2D array containing edge connectivity patterns + // based on faces for most objects. + this.edges = []; + + // One color per vertex representing the stroke color at that vertex. + this.vertexStrokeColors = []; + + // An array containing every vertex for stroke drawing. + this.lineVertices = new DataArray(); + + // The tangents going into or out of a vertex on a line. Along a straight + // line segment, both should be equal. At an endpoint, one or the other + // will not exist and will be all 0. In joins between line segments, they + // may be different, as they will be the tangents on either side of the join. + this.lineTangentsIn = new DataArray(); + this.lineTangentsOut = new DataArray(); + + // When drawing lines with thickness, entries in this buffer represent which + // side of the centerline the vertex will be placed. The sign of the number + // will represent the side of the centerline, and the absolute value will be + // used as an enum to determine which part of the cap or join each vertex + // represents. See the doc comments for _addCap and _addJoin for diagrams. + this.lineSides = new DataArray(); + + // One color per line vertex, generated automatically based on + // vertexStrokeColors in _edgesToVertices(). + this.lineVertexColors = new DataArray(); + + this._hasStrokeTransparency = undefined; + this.partState = partState || createPartState(); this.dirtyFlags = {}; @@ -225,6 +255,330 @@ class GeometryPart { this.vertexTangents = tangents; return this; } + + /** + * Create a 2D array for establishing stroke connections + * @private + * @chainable + */ + _makeTriangleEdges() { + this.edges.length = 0; + + for (let j = 0; j < this.faces.length; j++) { + this.edges.push([this.faces[j][0], this.faces[j][1]]); + this.edges.push([this.faces[j][1], this.faces[j][2]]); + this.edges.push([this.faces[j][2], this.faces[j][0]]); + } + + return this; + } + + /** + * @example + * let tetrahedron; + * function setup() { + * createCanvas(200, 200, WEBGL); + * describe('A rotating tetrahedron'); + * + * tetrahedron = new p5.Geometry(); + * + * // Give each geometry a unique gid + * tetrahedron.gid = 'tetrahedron'; + * + * // Add four points of the tetrahedron + * + * let radius = 50; + * // A 2D triangle: + * tetrahedron.vertices.push(createVector(radius, 0, 0)); + * tetrahedron.vertices.push(createVector(radius, 0, 0).rotate(TWO_PI / 3)); + * tetrahedron.vertices.push(createVector(radius, 0, 0).rotate(TWO_PI * 2 / 3)); + * // Add a tip in the z axis: + * tetrahedron.vertices.push(createVector(0, 0, radius)); + * + * // Create the four faces by connecting the sets of three points + * tetrahedron.faces.push([0, 1, 2]); + * tetrahedron.faces.push([0, 1, 3]); + * tetrahedron.faces.push([0, 2, 3]); + * tetrahedron.faces.push([1, 2, 3]); + * tetrahedron.makeEdgesFromFaces(); + * } + * + * function draw() { + * background(200); + * strokeWeight(2); + * orbitControl(); + * rotateY(millis() * 0.001); + * model(tetrahedron); + * } + */ + makeEdgesFromFaces() { + this._makeTriangleEdges(); + } + + /** + * Converts each line segment into the vertices and vertex attributes needed + * to turn the line into a polygon on screen. This will include: + * - Two triangles line segment to create a rectangle + * - Two triangles per endpoint to create a stroke cap rectangle. A fragment + * shader is responsible for displaying the appropriate cap style within + * that rectangle. + * - Four triangles per join between adjacent line segments, creating a quad on + * either side of the join, perpendicular to the lines. A vertex shader will + * discard the quad in the "elbow" of the join, and a fragment shader will + * display the appropriate join style within the remaining quad. + * + * @private + * @chainable + */ + _edgesToVertices(renderer = this.renderer) { + this.lineVertices.clear(); + this.lineTangentsIn.clear(); + this.lineTangentsOut.clear(); + this.lineSides.clear(); + this.lineVertexColors.clear(); + this._hasStrokeTransparency = undefined; + + const potentialCaps = new Map(); + const connected = new Set(); + let lastValidDir; + for (let i = 0; i < this.edges.length; i++) { + const prevEdge = this.edges[i - 1]; + const currEdge = this.edges[i]; + const isPoint = currEdge[0] === currEdge[1]; + const begin = this.vertices[currEdge[0]]; + const end = this.vertices[currEdge[1]]; + const prevColor = + this.vertexStrokeColors.length > 0 && prevEdge + ? this.vertexStrokeColors.slice( + prevEdge[1] * 4, + (prevEdge[1] + 1) * 4 + ) + : [0, 0, 0, 0]; + const fromColor = + this.vertexStrokeColors.length > 0 + ? this.vertexStrokeColors.slice( + currEdge[0] * 4, + (currEdge[0] + 1) * 4 + ) + : [0, 0, 0, 0]; + const toColor = + this.vertexStrokeColors.length > 0 + ? this.vertexStrokeColors.slice( + currEdge[1] * 4, + (currEdge[1] + 1) * 4 + ) + : [0, 0, 0, 0]; + const dir = isPoint + ? new Vector(0, 1, 0) + : end.copy().sub(begin).normalize(); + const dirOK = dir.magSq() > 0; + if (dirOK) { + this._addSegment(begin, end, fromColor, toColor, dir,renderer); + } + if (!renderer?._simpleLines) { + if (i > 0 && prevEdge[1] === currEdge[0]) { + if (!connected.has(currEdge[0])) { + connected.add(currEdge[0]); + potentialCaps.delete(currEdge[0]); + // Add a join if this segment shares a vertex with the previous. Skip + // actually adding join vertices if either the previous segment or this + // one has a length of 0. + // + // Don't add a join if the tangents point in the same direction, which + // would mean the edges line up exactly, and there is no need for a join. + if (lastValidDir && dirOK && dir.dot(lastValidDir) < 1 - 1e-8) { + this._addJoin(begin, lastValidDir, dir, fromColor); + } + } + } else if (isPoint) { + this._addCap(begin, dir.copy().mult(-1), fromColor); + this._addCap(begin, dir, fromColor); + } else { + // Start a new line + if (dirOK && !connected.has(currEdge[0])) { + const existingCap = potentialCaps.get(currEdge[0]); + if (existingCap) { + this._addJoin(begin, existingCap.dir, dir, fromColor); + potentialCaps.delete(currEdge[0]); + connected.add(currEdge[0]); + } else { + potentialCaps.set(currEdge[0], { + point: begin, + dir: dir.copy().mult(-1), + color: fromColor + }); + } + } + if (!isPoint && lastValidDir && !connected.has(prevEdge[1])) { + const existingCap = potentialCaps.get(prevEdge[1]); + if (existingCap) { + this._addJoin( + this.vertices[prevEdge[1]], + lastValidDir, + existingCap.dir.copy().mult(-1), + prevColor + ); + potentialCaps.delete(prevEdge[1]); + connected.add(prevEdge[1]); + } else { + // Close off the last segment with a cap + potentialCaps.set(prevEdge[1], { + point: this.vertices[prevEdge[1]], + dir: lastValidDir, + color: prevColor + }); + } + lastValidDir = undefined; + } + } + + if (i === this.edges.length - 1 && !connected.has(currEdge[1])) { + const existingCap = potentialCaps.get(currEdge[1]); + if (existingCap) { + this._addJoin(end, dir, existingCap.dir.copy().mult(-1), toColor); + potentialCaps.delete(currEdge[1]); + connected.add(currEdge[1]); + } else { + potentialCaps.set(currEdge[1], { + point: end, + dir, + color: toColor + }); + } + } + + if (dirOK) { + lastValidDir = dir; + } + } + } + for (const { point, dir, color } of potentialCaps.values()) { + this._addCap(point, dir, color); + } + return this; + } + + /** + * Adds the vertices and vertex attributes for two triangles making a rectangle + * for a straight line segment. A vertex shader is responsible for picking + * proper coordinates on the screen given the centerline positions, the tangent, + * and the side of the centerline each vertex belongs to. Sides follow the + * following scheme: + * + * -1 -1 + * o-------------o + * | | + * o-------------o + * 1 1 + * + * @private + * @chainable + */ + _addSegment(begin, end, fromColor, toColor, dir,renderer = this.renderer) { + const a = begin.array(); + const b = end.array(); + const dirArr = dir.array(); + this.lineSides.push(1, 1, -1, 1, -1, -1); + for (const tangents of [this.lineTangentsIn, this.lineTangentsOut]) { + for (let i = 0; i < 6; i++) { + tangents.push(...dirArr); + } + } + this.lineVertices.push(...a, ...b, ...a, ...b, ...b, ...a); + if (!renderer?._simpleLines) { + this.lineVertexColors.push( + ...fromColor, + ...toColor, + ...fromColor, + ...toColor, + ...toColor, + ...fromColor + ); + } + return this; + } + + /** + * Adds the vertices and vertex attributes for two triangles representing the + * stroke cap of a line. A fragment shader is responsible for displaying the + * appropriate cap style within the rectangle they make. + * + * The lineSides buffer will include the following values for the points on + * the cap rectangle: + * + * -1 -2 + * -----------o---o + * | | + * -----------o---o + * 1 2 + * @private + * @chainable + */ + _addCap(point, tangent, color) { + const ptArray = point.array(); + const tanInArray = tangent.array(); + const tanOutArray = [0, 0, 0]; + for (let i = 0; i < 6; i++) { + this.lineVertices.push(...ptArray); + this.lineTangentsIn.push(...tanInArray); + this.lineTangentsOut.push(...tanOutArray); + this.lineVertexColors.push(...color); + } + this.lineSides.push(-1, 2, -2, 1, 2, -1); + return this; + } + + /** + * Adds the vertices and vertex attributes for four triangles representing a + * join between two adjacent line segments. This creates a quad on either side + * of the shared vertex of the two line segments, with each quad perpendicular + * to the lines. A vertex shader will discard all but the quad in the "elbow" of + * the join, and a fragment shader will display the appropriate join style + * within the remaining quad. + * + * The lineSides buffer will include the following values for the points on + * the join rectangles: + * + * -1 -2 + * -------------o----o + * | | + * 1 o----o----o -3 + * | | 0 | + * --------o----o | + * 2| 3 | + * | | + * | | + * @private + * @chainable + */ + _addJoin(point, fromTangent, toTangent, color) { + const ptArray = point.array(); + const tanInArray = fromTangent.array(); + const tanOutArray = toTangent.array(); + for (let i = 0; i < 12; i++) { + this.lineVertices.push(...ptArray); + this.lineTangentsIn.push(...tanInArray); + this.lineTangentsOut.push(...tanOutArray); + this.lineVertexColors.push(...color); + } + this.lineSides.push(-1, -3, -2, -1, 0, -3); + this.lineSides.push(3, 1, 2, 3, 0, 1); + return this; + } + + hasStrokeTransparency() { + if (this._hasStrokeTransparency === undefined) { + this._hasStrokeTransparency = false; + for (let i = 0; i < this.lineVertexColors.length; i += 4) { + if (this.lineVertexColors[i + 3] < 1) { + this._hasStrokeTransparency = true; + break; + } + } + } + return this._hasStrokeTransparency; + } + } export { GeometryPart, createPartState, createUserVertexProperty }; diff --git a/test/unit/webgl/p5.GeometryPart.js b/test/unit/webgl/p5.GeometryPart.js index 23dc4b38cf..e1d6a508eb 100644 --- a/test/unit/webgl/p5.GeometryPart.js +++ b/test/unit/webgl/p5.GeometryPart.js @@ -421,4 +421,84 @@ suite('p5.GeometryPart', function () { fillSpy.mockRestore(); } }); + + test('parts have independent stroke data', function () { + const partA = new p5.GeometryPart('part-a'); + const partB = new p5.GeometryPart('part-b'); + + partA.edges.push([0, 1]); + partA.vertexStrokeColors.push(1, 0, 0, 1); + partA.lineVertices.push(0, 0, 0); + + expect(partB.edges).toEqual([]); + expect(partB.vertexStrokeColors).toEqual([]); + expect(partB.lineVertices.length).toBe(0); + }); + + test('generates edges from part-local faces', function () { + const part = new p5.GeometryPart('triangle'); + + part.vertices = [ + new p5.Vector(0, 0, 0), + new p5.Vector(10, 0, 0), + new p5.Vector(0, 10, 0) + ]; + + part.faces = [[0, 1, 2]]; + + part.makeEdgesFromFaces(); + + expect(part.edges).toEqual([ + [0, 1], + [1, 2], + [2, 0] + ]); + }); + test('generates stroke geometry for an independent part', function () { + const part = new p5.GeometryPart('line'); + + part.vertices = [ + new p5.Vector(0, 0, 0), + new p5.Vector(10, 0, 0) + ]; + + part.edges = [[0, 1]]; + part.vertexStrokeColors = [ + 1, 0, 0, 1, + 0, 1, 0, 1 + ]; + + part._edgesToVertices({ _simpleLines: false }); + + expect(part.lineVertices.length).toBeGreaterThan(0); + expect(part.lineTangentsIn.length).toBeGreaterThan(0); + expect(part.lineTangentsOut.length).toBeGreaterThan(0); + expect(part.lineSides.length).toBeGreaterThan(0); + expect(part.lineVertexColors.length).toBeGreaterThan(0); + + const previousColorCount = part.lineVertexColors.length; + + part._edgesToVertices({ _simpleLines: false }); + + expect(part.lineVertexColors.length).toBe(previousColorCount); + }); + + test('respects simple line mode on a standalone part', function () { + const part = new p5.GeometryPart('simple-line'); + + part.vertices = [ + new p5.Vector(0, 0, 0), + new p5.Vector(10, 0, 0) + ]; + + part.edges = [[0, 1]]; + + part._edgesToVertices({ _simpleLines: false }); + const fullLineVertices = part.lineVertices.length; + + part._edgesToVertices({ _simpleLines: true }); + const simpleLineVertices = part.lineVertices.length; + + expect(simpleLineVertices).toBeLessThan(fullLineVertices); + }); }); From 68660b77fd98c57c66309a0331eb46af867acf2b Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Wed, 4 Nov 2026 10:42:20 +0530 Subject: [PATCH 5/8] refactor(webgl): assign stroke edges to geometry parts --- src/webgl/GeometryBuilder.js | 40 ++++++++-- test/unit/webgl/p5.Geometry.js | 130 +++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 8 deletions(-) diff --git a/src/webgl/GeometryBuilder.js b/src/webgl/GeometryBuilder.js index dd73ae7778..23ab318c4e 100644 --- a/src/webgl/GeometryBuilder.js +++ b/src/webgl/GeometryBuilder.js @@ -176,19 +176,34 @@ class GeometryBuilder { * part stays aligned whatever shape the uvs are. */ _addToCurrentPart(input, vertices, normals, vertexColors) { - // parts are made of fills; a stroke-only draw contributes no faces - if (!this.renderer.states.fillColor) return; + const hasFill = Boolean( + this.renderer.states.fillColor && input.faces.length + ); + const hasStroke = Boolean( + this.renderer.states.strokeColor && input.edges.length + ); + + if (!hasFill && !hasStroke) return; const state = this._snapshotPartState(); - if ( - !this.currentPart || - !this._sameMaterial(state, this.currentPart.partState) - ) { + + if (!this.currentPart) { this.currentPart = new GeometryPart( `${this.geometry.gid}|part${this.parts.length}`, state ); this.parts.push(this.currentPart); + } else if (hasFill) { + if (this.currentPart.faces.length === 0) { + // A stroke-only part can adopt the material of its first fill. + this.currentPart.partState = state; + } else if (!this._sameMaterial(state, this.currentPart.partState)) { + this.currentPart = new GeometryPart( + `${this.geometry.gid}|part${this.parts.length}`, + state + ); + this.parts.push(this.currentPart); + } } const part = this.currentPart; @@ -205,8 +220,17 @@ class GeometryBuilder { for (const c of vertexColors) { part.vertexColors.push(c); } - for (const f of input.faces) { - part.faces.push(f.map(idx => idx + startIdx)); + + if (hasFill) { + for (const f of input.faces) { + part.faces.push(f.map(idx => idx + startIdx)); + } + } + + if (hasStroke) { + for (const edge of input.edges) { + part.edges.push(edge.map(idx => idx + startIdx)); + } } // carry custom per-vertex attributes into the part too, the same way the diff --git a/test/unit/webgl/p5.Geometry.js b/test/unit/webgl/p5.Geometry.js index 95f078b8a3..d3a3b7357b 100644 --- a/test/unit/webgl/p5.Geometry.js +++ b/test/unit/webgl/p5.Geometry.js @@ -888,6 +888,136 @@ suite('p5.Geometry', function () { .not.toEqual(geom.parts[1].partState.texture); }); + test('single-part geometry keeps its edges without duplication', function () { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const geom = myp5.buildGeometry(() => { + myp5.noFill(); + myp5.stroke(255); + myp5.beginShape(myp5.LINES); + myp5.vertex(0, 0); + myp5.vertex(10, 0); + myp5.endShape(); + }); + + expect(geom.parts).toHaveLength(1); + expect(geom.parts[0]).toBe(geom); + expect(geom.edges).toEqual([[0, 1]]); + expect(geom.parts[0].edges).toBe(geom.edges); + }); + + test('material parts own edges with local vertex indices', function () { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const texA = myp5.createGraphics(10, 10); + const texB = myp5.createGraphics(10, 10); + + const geom = myp5.buildGeometry(() => { + myp5.stroke(255); + + myp5.texture(texA); + myp5.box(8); + + myp5.texture(texB); + myp5.box(8); + }); + + expect(geom.parts).toHaveLength(2); + + const [partA, partB] = geom.parts; + + expect(partA.edges.length).toBeGreaterThan(0); + expect(partB.edges.length).toBeGreaterThan(0); + + // Each part's indices start within its own vertex array. + for (const part of geom.parts) { + for (const [a, b] of part.edges) { + expect(a).toBeGreaterThanOrEqual(0); + expect(b).toBeGreaterThanOrEqual(0); + expect(a).toBeLessThan(part.vertices.length); + expect(b).toBeLessThan(part.vertices.length); + } + } + + // The second part starts indexing its own vertices from zero. + expect(partB.edges[0]).toEqual(geom.edges[partA.edges.length].map( + index => index - partA.vertices.length + )); + + // Aggregate edges are retained until per-part rendering is implemented. + expect(geom.edges.length).toBe( + partA.edges.length + partB.edges.length + ); + }); + + test('stroke-only geometry is preserved in a multipart build', function () { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const texA = myp5.createGraphics(10, 10); + const texB = myp5.createGraphics(10, 10); + + const geom = myp5.buildGeometry(() => { + myp5.stroke(255); + + myp5.texture(texA); + myp5.box(8); + + myp5.noFill(); + myp5.beginShape(myp5.LINES); + myp5.vertex(0, 0); + myp5.vertex(10, 0); + myp5.endShape(); + + myp5.fill(255); + myp5.texture(texB); + myp5.box(8); + }); + + expect(geom.parts).toHaveLength(2); + + const [partA, partB] = geom.parts; + + // The stroke-only line belongs to the existing first part. + expect(partA.vertices.length).toBeGreaterThan(0); + expect(partA.edges.length).toBeGreaterThan(0); + + // The line adds two vertices but no faces. + expect(partA.vertices.length).toBeGreaterThan(partA.faces.length); + + // The second material remains a separate part. + expect(partB.faces.length).toBeGreaterThan(0); + + for (const part of geom.parts) { + for (const [a, b] of part.edges) { + expect(a).toBeGreaterThanOrEqual(0); + expect(b).toBeGreaterThanOrEqual(0); + expect(a).toBeLessThan(part.vertices.length); + expect(b).toBeLessThan(part.vertices.length); + } + } + }); + + test('stroke-only shapes are assigned to a part', function () { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const geom = myp5.buildGeometry(() => { + myp5.fill(255); + myp5.stroke(255); + myp5.box(8); + + myp5.noFill(); + myp5.beginShape(myp5.LINES); + myp5.vertex(0, 0); + myp5.vertex(10, 0); + myp5.endShape(); + }); + + expect(geom.parts).toHaveLength(1); + expect(geom.parts[0]).toBe(geom); + expect(geom.edges.length).toBeGreaterThan(0); + expect(geom.parts[0].edges).toBe(geom.edges); + }); + test('custom vertex attributes survive the multi-material part split', function() { myp5.createCanvas(50, 50, myp5.WEBGL); From 882d114ebd18db63564f8f7e89fa655d4c401632 Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Wed, 4 Nov 2026 13:01:23 +0530 Subject: [PATCH 6/8] refactor(webgl): pass renderer explicitly for stroke generation --- src/webgl/3d_primitives.js | 20 ++++++++++---------- src/webgl/ShapeBuilder.js | 2 +- src/webgl/p5.GeometryPart.js | 4 ++-- test/unit/webgl/p5.Geometry.js | 14 +++++++------- test/unit/webgl/p5.RendererGL.js | 2 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/webgl/3d_primitives.js b/src/webgl/3d_primitives.js index d9f0c9b2ad..abfdb59a76 100644 --- a/src/webgl/3d_primitives.js +++ b/src/webgl/3d_primitives.js @@ -1529,7 +1529,7 @@ function primitives3D(p5, fn) { this.uvs = [0, 0, 1, 0, 1, 1]; }; const triGeom = new Geometry(1, 1, _triangle, this); - triGeom._edgesToVertices(); + triGeom._edgesToVertices(this); triGeom.computeNormals(); triGeom.gid = gid; this.geometryBufferCache.ensureCached(triGeom); @@ -1681,7 +1681,7 @@ function primitives3D(p5, fn) { arcGeom.computeNormals(); if (detail <= 50) { - arcGeom._edgesToVertices(); + arcGeom._edgesToVertices(this); } else if (this.states.strokeColor) { console.log( `Cannot apply a stroke to an ${shape} with more than 50 detail` @@ -1743,7 +1743,7 @@ function primitives3D(p5, fn) { } }; const rectGeom = new Geometry(detailX, detailY, _rect, this); - rectGeom.computeFaces().computeNormals()._edgesToVertices(); + rectGeom.computeFaces().computeNormals()._edgesToVertices(this); rectGeom.gid = gid; this.geometryBufferCache.ensureCached(rectGeom); } @@ -1918,7 +1918,7 @@ function primitives3D(p5, fn) { const endVertex = vertexOrder[(i + 1) % vertexOrder.length]; quadGeom.edges.push([startVertex, endVertex]); } - quadGeom._edgesToVertices(); + quadGeom._edgesToVertices(this); quadGeom.gid = gid; this.geometryBufferCache.ensureCached(quadGeom); } @@ -2241,7 +2241,7 @@ function primitives3D(p5, fn) { const planeGeom = new Geometry(detailX, detailY, _plane, this); planeGeom.computeFaces().computeNormals(); if (detailX <= 1 && detailY <= 1) { - planeGeom._makeTriangleEdges()._edgesToVertices(); + planeGeom._makeTriangleEdges()._edgesToVertices(this); } else if (this.states.strokeColor) { console.log( 'Cannot draw stroke on plane objects with more' + @@ -2326,7 +2326,7 @@ function primitives3D(p5, fn) { const boxGeom = new Geometry(detailX, detailY, _box, this); boxGeom.computeNormals(); if (detailX <= 4 && detailY <= 4) { - boxGeom._edgesToVertices(); + boxGeom._edgesToVertices(this); } else if (this.states.strokeColor) { console.log( 'Cannot draw stroke on box objects with more' + @@ -2391,7 +2391,7 @@ function primitives3D(p5, fn) { const ellipsoidGeom = new Geometry(detailX, detailY, _ellipsoid, this); ellipsoidGeom.computeFaces(); if (detailX <= 24 && detailY <= 24) { - ellipsoidGeom._makeTriangleEdges()._edgesToVertices(); + ellipsoidGeom._makeTriangleEdges()._edgesToVertices(this); } else if (this.states.strokeColor) { console.log( 'Cannot draw stroke on ellipsoids with more' + @@ -2439,7 +2439,7 @@ function primitives3D(p5, fn) { ); // normals are computed in call to _truncatedCone if (detailX <= 24 && detailY <= 16) { - cylinderGeom._makeTriangleEdges()._edgesToVertices(); + cylinderGeom._makeTriangleEdges()._edgesToVertices(this); } else if (this.states.strokeColor) { console.log( 'Cannot draw stroke on cylinder objects with more' + @@ -2476,7 +2476,7 @@ function primitives3D(p5, fn) { this ); if (detailX <= 24 && detailY <= 16) { - coneGeom._makeTriangleEdges()._edgesToVertices(); + coneGeom._makeTriangleEdges()._edgesToVertices(this); } else if (this.states.strokeColor) { console.log( 'Cannot draw stroke on cone objects with more' + @@ -2544,7 +2544,7 @@ function primitives3D(p5, fn) { const torusGeom = new Geometry(detailX, detailY, _torus, this); torusGeom.computeFaces(); if (detailX <= 24 && detailY <= 16) { - torusGeom._makeTriangleEdges()._edgesToVertices(); + torusGeom._makeTriangleEdges()._edgesToVertices(this); } else if (this.states.strokeColor) { console.log( 'Cannot draw strokes on torus object with more' + diff --git a/src/webgl/ShapeBuilder.js b/src/webgl/ShapeBuilder.js index b5c9201e08..8932626792 100644 --- a/src/webgl/ShapeBuilder.js +++ b/src/webgl/ShapeBuilder.js @@ -149,7 +149,7 @@ export class ShapeBuilder { ); } if (shouldProcessEdges && !this.renderer.geometryBuilder) { - this.geometry._edgesToVertices(); + this.geometry._edgesToVertices(this.renderer); } if (this.shapeMode === constants.PATH) { diff --git a/src/webgl/p5.GeometryPart.js b/src/webgl/p5.GeometryPart.js index fb459dc956..ad3d422a26 100644 --- a/src/webgl/p5.GeometryPart.js +++ b/src/webgl/p5.GeometryPart.js @@ -330,7 +330,7 @@ class GeometryPart { * @private * @chainable */ - _edgesToVertices(renderer = this.renderer) { + _edgesToVertices(renderer) { this.lineVertices.clear(); this.lineTangentsIn.clear(); this.lineTangentsOut.clear(); @@ -474,7 +474,7 @@ class GeometryPart { * @private * @chainable */ - _addSegment(begin, end, fromColor, toColor, dir,renderer = this.renderer) { + _addSegment(begin, end, fromColor, toColor, dir, renderer) { const a = begin.array(); const b = end.array(); const dirArr = dir.array(); diff --git a/test/unit/webgl/p5.Geometry.js b/test/unit/webgl/p5.Geometry.js index d3a3b7357b..2bb24bf3ec 100644 --- a/test/unit/webgl/p5.Geometry.js +++ b/test/unit/webgl/p5.Geometry.js @@ -570,7 +570,7 @@ suite('p5.Geometry', function () { myp5.createVector(100, 0) ); geom.edges.push([0, 1], [1, 2], [2, 3]); - geom._edgesToVertices(); + geom._edgesToVertices(myp5._renderer); expect(geom._addSegment).toHaveBeenCalledTimes(3); expect(geom._addCap).toHaveBeenCalledTimes(2); @@ -584,7 +584,7 @@ suite('p5.Geometry', function () { myp5.createVector(0, 200) ); geom.edges.push([0, 1], [1, 2]); - geom._edgesToVertices(); + geom._edgesToVertices(myp5._renderer); expect(geom._addSegment).toHaveBeenCalledTimes(2); expect(geom._addCap).toHaveBeenCalledTimes(2); @@ -600,7 +600,7 @@ suite('p5.Geometry', function () { myp5.createVector(100, 0) ); geom.edges.push([0, 1], [2, 3]); - geom._edgesToVertices(); + geom._edgesToVertices(myp5._renderer); expect(geom._addSegment).toHaveBeenCalledTimes(2); expect(geom._addCap).toHaveBeenCalledTimes(4); @@ -615,7 +615,7 @@ suite('p5.Geometry', function () { myp5.createVector(100, 0) ); geom.edges.push([0, 1], [1, 2], [2, 3], [3, 0]); - geom._edgesToVertices(); + geom._edgesToVertices(myp5._renderer); expect(geom._addSegment).toHaveBeenCalledTimes(4); expect(geom._addCap).toHaveBeenCalledTimes(0); @@ -644,7 +644,7 @@ suite('p5.Geometry', function () { myp5.createVector(100, 0) ); geom.edges.push([0, 1], [1, 2], [2, 3], [3, 4]); - geom._edgesToVertices(); + geom._edgesToVertices(myp5._renderer); // The degenerate edge should be skipped without breaking the // polyline into multiple pieces @@ -662,7 +662,7 @@ suite('p5.Geometry', function () { myp5.createVector(100, 0) ); geom.edges.push([0, 1], [1, 2], [2, 3], [3, 4]); - geom._edgesToVertices(); + geom._edgesToVertices(myp5._renderer); // The degenerate edge should be skipped and caps should still be added // from the previous non degenerate edge @@ -680,7 +680,7 @@ suite('p5.Geometry', function () { myp5.createVector(100, 0) ); geom.edges.push([0, 1], [1, 2], [3, 4]); - geom._edgesToVertices(); + geom._edgesToVertices(myp5._renderer); expect(geom._addSegment).toHaveBeenCalledTimes(2); expect(geom._addCap).toHaveBeenCalledTimes(4); diff --git a/test/unit/webgl/p5.RendererGL.js b/test/unit/webgl/p5.RendererGL.js index 9ad13fdce9..603006f8ef 100644 --- a/test/unit/webgl/p5.RendererGL.js +++ b/test/unit/webgl/p5.RendererGL.js @@ -2367,7 +2367,7 @@ void main() { 0, 1 ); - this._edgesToVertices(); + this._edgesToVertices(myp5._renderer); }, myp5._renderer ); From a1b93fbb1cef5aa79928cf3c165b2fe97abe24e6 Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Wed, 4 Nov 2026 13:01:46 +0530 Subject: [PATCH 7/8] fix(webgl): render multipart strokes from geometry parts --- src/core/p5.Renderer3D.js | 29 ++++++++++++++++++++++++----- src/webgl/loading.js | 1 + 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/core/p5.Renderer3D.js b/src/core/p5.Renderer3D.js index 6f793fe26d..8d2df193e3 100644 --- a/src/core/p5.Renderer3D.js +++ b/src/core/p5.Renderer3D.js @@ -571,9 +571,24 @@ export class Renderer3D extends Renderer { if (this.geometryBuilder) { this.geometryBuilder.addRetained(model); } else { - if (!this.geometryInHash(model.gid)) { - model._edgesToVertices(); - this._getOrMakeCachedBuffers(model); + if (model.parts.length === 1 && model.parts[0] === model) { + if (!this.geometryInHash(model.gid)) { + model._edgesToVertices(this); + this._getOrMakeCachedBuffers(model); + } + } else { + if (!this.geometryInHash(model.gid)) { + this._getOrMakeCachedBuffers(model); + } + + for (const part of model.parts) { + if (!this.geometryInHash(part.gid)) { + if (part.edges.length > 0) { + part._edgesToVertices(this); + } + this._getOrMakeCachedBuffers(part); + } + } } this._drawGeometry(model, { count }); @@ -638,8 +653,12 @@ export class Renderer3D extends Renderer { } } - if (this.states.strokeColor && geometry.lineVertices.length >= 1) { - this._drawStrokes(geometry, { count }); + if (this.states.strokeColor) { + for (const part of geometry.parts) { + if (part.lineVertices.length > 0) { + this._drawStrokes(part, { count }); + } + } } this.buffers.user = []; diff --git a/src/webgl/loading.js b/src/webgl/loading.js index bfb086dcee..cd0effaaa4 100755 --- a/src/webgl/loading.js +++ b/src/webgl/loading.js @@ -240,6 +240,7 @@ function buildMaterialParts(model, faceMaterials, materials) { }); part.faces.push(localFace); } + part._makeTriangleEdges(); parts.push(part); } From 3036a275d41ac13b03d5d12186d51e9e7fd84da6 Mon Sep 17 00:00:00 2001 From: Srinidhi-4419 Date: Wed, 4 Nov 2026 13:03:20 +0530 Subject: [PATCH 8/8] fix(renderers): clean up multipart geometry GPU buffers --- src/webgl/3d_primitives.js | 13 ++- src/webgl/p5.GeometryPart.js | 9 ++ src/webgpu/p5.RendererWebGPU.js | 85 +++++++++++++----- test/unit/webgl/p5.Geometry.js | 46 ++++++++++ test/unit/webgpu/p5.RendererWebGPU.js | 121 ++++++++++++++++++++++---- 5 files changed, 233 insertions(+), 41 deletions(-) diff --git a/src/webgl/3d_primitives.js b/src/webgl/3d_primitives.js index abfdb59a76..e20646d40f 100644 --- a/src/webgl/3d_primitives.js +++ b/src/webgl/3d_primitives.js @@ -393,7 +393,18 @@ function primitives3D(p5, fn) { * } */ fn.freeGeometry = function (geometry) { - this._renderer.geometryBufferCache.freeBuffers(geometry.gid); + const renderer = this._renderer; + const cache = renderer.geometryBufferCache; + + cache.freeBuffers(geometry.gid); + + for (const part of geometry.parts) { + if (part !== geometry) { + cache.freeBuffers(part.gid); + } + } + + renderer._scheduleRetiredBufferDestruction?.(); }; /** diff --git a/src/webgl/p5.GeometryPart.js b/src/webgl/p5.GeometryPart.js index ad3d422a26..d4ed50d034 100644 --- a/src/webgl/p5.GeometryPart.js +++ b/src/webgl/p5.GeometryPart.js @@ -336,6 +336,15 @@ class GeometryPart { this.lineTangentsOut.clear(); this.lineSides.clear(); this.lineVertexColors.clear(); + for (const name of [ + 'lineVertices', + 'lineTangentsIn', + 'lineTangentsOut', + 'lineSides', + 'lineVertexColors' + ]) { + this.dirtyFlags[name] = true; + } this._hasStrokeTransparency = undefined; const potentialCaps = new Map(); diff --git a/src/webgpu/p5.RendererWebGPU.js b/src/webgpu/p5.RendererWebGPU.js index d4d26e1df2..e490db7e4e 100644 --- a/src/webgpu/p5.RendererWebGPU.js +++ b/src/webgpu/p5.RendererWebGPU.js @@ -1250,24 +1250,72 @@ function rendererWebGPU(p5, fn) { } _freeBuffers(buffers) { - const destroyIfExists = buf => { - if (buf && buf.destroy) { - buf.destroy(); + const geometry = buffers.geometry; + const retired = new Set(); + + // Retire cached buffers, rather than immediately destroying buffers + // that may still be referenced by pending GPU work. + const retire = buffer => { + if (buffer && typeof buffer.destroy === 'function') { + retired.add(buffer); } }; - destroyIfExists(buffers.indexBuffer); + retire(buffers.indexBuffer); + buffers.indexBuffer = null; - const freeDefs = defs => { - for (const def of defs) { - destroyIfExists(buffers[def.dst]); - buffers[def.dst] = null; + for (const def of [ + ...this.buffers.stroke, + ...this.buffers.fill, + ...this.buffers.user + ]) { + retire(buffers[def.dst]); + buffers[def.dst] = null; + } + + // Cached vertex buffers may also be tracked by this geometry's pool. + // Detach all records so they cannot be reused after freeGeometry(). + if (geometry?._vertexBufferPools) { + for (const groups of [ + geometry._vertexBufferPools, + geometry._vertexBuffersInUse, + geometry._vertexBuffersToReturn + ]) { + for (const records of Object.values(groups || {})) { + for (const info of records) { + retire(info.buffer); + info.dataCopy = null; + } + } } - }; - freeDefs(this.buffers.stroke); - freeDefs(this.buffers.fill); - freeDefs(this.buffers.user); + const index = this._geometriesWithPools.indexOf(geometry); + if (index !== -1) { + this._geometriesWithPools.splice(index, 1); + } + + delete geometry._vertexBufferPools; + delete geometry._vertexBuffersInUse; + delete geometry._vertexBuffersToReturn; + } + + this._retiredBuffers.push(...retired); + } + + _scheduleRetiredBufferDestruction() { + if (this._retiredBuffers.length === 0) return; + + // Submit any commands that might still reference these buffers. + this.flushDraw(); + + const retired = [...new Set(this._retiredBuffers)]; + this._retiredBuffers = []; + + this.queue.onSubmittedWorkDone().then(() => { + for (const buffer of retired) { + buffer.destroy(); + } + }); } _getValidSampleCount(requestedCount) { @@ -2308,17 +2356,8 @@ function rendererWebGPU(p5, fn) { // Return all vertex buffers to their pools this._returnVertexBuffersToPool(); - - // Destroy all retired buffers - const retired = this._retiredBuffers; - this._postSubmitCallbacks.push(() => { - for (const buffer of retired) { - if (buffer && buffer.destroy) { - buffer.destroy(); - } - } - }); - this._retiredBuffers = []; + // Retire buffers after their GPU work has completed. + this._scheduleRetiredBufferDestruction(); if (this._frameState === FRAME_STATE.PROMOTED) { for (const { fbo, diff } of states) { diff --git a/test/unit/webgl/p5.Geometry.js b/test/unit/webgl/p5.Geometry.js index 2bb24bf3ec..918f1b6193 100644 --- a/test/unit/webgl/p5.Geometry.js +++ b/test/unit/webgl/p5.Geometry.js @@ -997,6 +997,52 @@ suite('p5.Geometry', function () { } }); + test('freeGeometry releases multipart geometry buffers', function () { + const renderer = myp5.createCanvas(50, 50, myp5.WEBGL); + const texA = myp5.createGraphics(10, 10); + const texB = myp5.createGraphics(10, 10); + + const geom = myp5.buildGeometry(() => { + myp5.stroke(255); + + myp5.texture(texA); + myp5.box(8); + + myp5.texture(texB); + myp5.box(8); + }); + + expect(geom.parts).toHaveLength(2); + + const cache = renderer.geometryBufferCache; + const ids = [geom.gid, ...geom.parts.map(part => part.gid)]; + + myp5.model(geom); + + for (const id of ids) { + expect(cache.isCached(id)).toBe(true); + } + + const lineBuffers = geom.parts.map(part => part.lineVertices.length); + expect(lineBuffers.every(length => length > 0)).toBe(true); + + myp5.freeGeometry(geom); + + for (const id of ids) { + expect(cache.isCached(id)).toBe(false); + } + + myp5.model(geom); + + for (const id of ids) { + expect(cache.isCached(id)).toBe(true); + } + + expect(geom.parts.map(part => part.lineVertices.length)).toEqual( + lineBuffers + ); + }); + test('stroke-only shapes are assigned to a part', function () { myp5.createCanvas(50, 50, myp5.WEBGL); diff --git a/test/unit/webgpu/p5.RendererWebGPU.js b/test/unit/webgpu/p5.RendererWebGPU.js index 57aa98c397..51284e1481 100644 --- a/test/unit/webgpu/p5.RendererWebGPU.js +++ b/test/unit/webgpu/p5.RendererWebGPU.js @@ -62,38 +62,125 @@ suite('WebGPU p5.RendererWebGPU', function () { ); }); + test('freeGeometry releases and recreates multipart buffer pools', async function () { + const renderer = myp5._renderer; + + const texA = myp5.createGraphics(10, 10); + const texB = myp5.createGraphics(10, 10); + + const geom = myp5.buildGeometry(() => { + myp5.stroke(255); + + myp5.texture(texA); + myp5.box(10); + + myp5.texture(texB); + myp5.box(10); + }); + + expect(geom.parts.length).toBeGreaterThan(1); + + const getBuffers = part => + [ + part._vertexBufferPools, + part._vertexBuffersInUse, + part._vertexBuffersToReturn + ].flatMap(groups => + Object.values(groups || {}).flatMap(records => + records.map(info => info.buffer) + ) + ); + + myp5.background(255); + myp5.model(geom); + + const cache = renderer.geometryBufferCache; + const ids = [geom.gid, ...geom.parts.map(part => part.gid)]; + + for (const id of ids) { + expect(cache.isCached(id)).toBe(true); + } + + const previousBuffers = geom.parts.map(part => { + expect(part.lineVertices.length).toBeGreaterThan(0); + const buffers = getBuffers(part); + expect(buffers.length).toBeGreaterThan(0); + return buffers; + }); + + myp5.freeGeometry(geom); + + for (const id of ids) { + expect(cache.isCached(id)).toBe(false); + } + + for (const part of geom.parts) { + expect(part._vertexBufferPools).toBeUndefined(); + expect(part._vertexBuffersInUse).toBeUndefined(); + expect(part._vertexBuffersToReturn).toBeUndefined(); + expect(renderer._geometriesWithPools).not.toContain(part); + } + + myp5.background(255); + myp5.model(geom); + + for (const id of ids) { + expect(cache.isCached(id)).toBe(true); + } + + geom.parts.forEach((part, index) => { + const newBuffers = getBuffers(part); + expect(newBuffers.length).toBeGreaterThan(0); + expect(renderer._geometriesWithPools).toContain(part); + + for (const buffer of newBuffers) { + expect(previousBuffers[index]).not.toContain(buffer); + } + }); + }); + test('freeGeometry causes new buffer allocation on next draw', async function () { - // Create a simple geometry const geom = myp5.buildGeometry(() => { myp5.triangle(0, 0, 10, 0, 5, 10); }); - // Draw the geometry once + // First draw myp5.background(255); myp5.model(geom); - // Get initial buffer count - const poolForVertexBuffer = geom._vertexBufferPools?.vertexBuffer; - expect(poolForVertexBuffer).to.exist; - const initialTotalBuffers = - poolForVertexBuffer.length + - (geom._vertexBuffersInUse?.vertexBuffer?.length || 0); + const getBuffers = geometry => [ + geometry._vertexBufferPools, + geometry._vertexBuffersInUse, + geometry._vertexBuffersToReturn + ].flatMap(groups => + Object.values(groups || {}).flatMap(records => + records.map(info => info.buffer) + ) + ); + + const oldBuffers = getBuffers(geom); + expect(oldBuffers.length).to.be.greaterThan(0); - // Free the geometry + // Free the geometry and its buffer pools. myp5.freeGeometry(geom); - // Draw the geometry again + expect(geom._vertexBufferPools).to.be.undefined; + expect(geom._vertexBuffersInUse).to.be.undefined; + expect(geom._vertexBuffersToReturn).to.be.undefined; + expect(myp5._renderer._geometriesWithPools).not.toContain(geom); + + // Drawing again should allocate fresh GPU buffers. myp5.background(255); myp5.model(geom); - // After freeGeometry, new buffers should be allocated - const finalTotalBuffers = - poolForVertexBuffer.length + - (geom._vertexBuffersInUse?.vertexBuffer?.length || 0); + const newBuffers = getBuffers(geom); + expect(newBuffers.length).to.be.greaterThan(0); + + for (const buffer of newBuffers) { + expect(oldBuffers).not.toContain(buffer); + } - // We should have more buffers now since freeGeometry marks geometry as dirty - // and new buffers need to be created - expect(finalTotalBuffers).to.be.greaterThan(initialTotalBuffers); + expect(myp5._renderer._geometriesWithPools).toContain(geom); }); test('immediate mode geometry reuses buffers across frames', async function () {