From 4144e9ff27e6df0e4ab17883323ed5cfc6b23875 Mon Sep 17 00:00:00 2001 From: nityam Date: Sun, 9 Aug 2026 19:10:25 +0530 Subject: [PATCH 1/5] split buildGeometry into parts when material state changes --- src/webgl/GeometryBuilder.js | 112 ++++++++++++++++++++++++++++++++- test/unit/webgl/p5.Geometry.js | 50 +++++++++++++++ 2 files changed, 160 insertions(+), 2 deletions(-) diff --git a/src/webgl/GeometryBuilder.js b/src/webgl/GeometryBuilder.js index 777feb838e..c42a9b70d3 100644 --- a/src/webgl/GeometryBuilder.js +++ b/src/webgl/GeometryBuilder.js @@ -1,6 +1,7 @@ import * as constants from '../core/constants'; import { Matrix } from '../math/p5.Matrix'; import { Geometry } from './p5.Geometry'; +import { GeometryPart, createPartState } from './p5.GeometryPart'; /** * @private @@ -22,6 +23,13 @@ class GeometryBuilder { this.geometry.gid = `_p5_GeometryBuilder_${GeometryBuilder.nextGeometryId}`; GeometryBuilder.nextGeometryId++; this.hasTransform = false; + + // material parts. when the material state (texture, specular, ambient, + // shininess) changes between draws inside the callback, a new part is + // opened, so model() renders the result per part like a multi-material obj. + // fill stays baked into vertexColors, so a plain colour change never splits. + this.parts = []; + this.currentPart = null; } /** @@ -63,11 +71,13 @@ class GeometryBuilder { ); } + const transformedVertices = this.transformVertices(input.vertices); + const transformedNormals = this.transformNormals(input.vertexNormals); let startIdx = this.geometry.vertices.length; - for (const v of this.transformVertices(input.vertices)) { + for (const v of transformedVertices) { this.geometry.vertices.push(v); } - for (const vn of this.transformNormals(input.vertexNormals)) { + for (const vn of transformedNormals) { this.geometry.vertexNormals.push(vn); } for (const val of input.uvs) { @@ -118,6 +128,95 @@ class GeometryBuilder { for (const c of vertexColors) { this.geometry.vertexColors.push(c); } + + this._addToCurrentPart( + input, + transformedVertices, + transformedNormals, + vertexColors + ); + } + + /** + * @private + * Snapshots the renderer's current per-part material state. Only the material + * uniforms that cannot be stored per vertex are tracked here (texture, + * specular, ambient, shininess); fill stays baked into vertexColors, so a + * plain fill() change never opens a new part. Uses p5's own state names, the + * same vocabulary the .mtl importer translates into. + */ + _snapshotPartState() { + const s = this.renderer.states; + const state = createPartState(); + if (s._tex) state.texture = s._tex; + if (s._useSpecularMaterial) state.specularColor = s.curSpecularColor; + if (s._hasSetAmbient) state.ambientColor = s.curAmbientColor; + if (s._useShininess !== 1) state.shininess = s._useShininess; + return state; + } + + /** + * @private + * Compares two colour arrays (or nulls) for equality. + */ + _sameColor(a, b) { + if (a === b) return true; + if (!a || !b) return false; + return a.length === b.length && a.every((v, i) => v === b[i]); + } + + /** + * @private + * True when two part states describe the same material, so consecutive draws + * can share one part. + */ + _sameMaterial(a, b) { + return a.texture === b.texture && + a.shininess === b.shininess && + this._sameColor(a.specularColor, b.specularColor) && + this._sameColor(a.ambientColor, b.ambientColor); + } + + /** + * @private + * Appends one draw's geometry to the current material part, opening a new + * part when the material state has changed since the last draw. Data is + * copied the same way as the combined geometry above (element by element, in + * vertex order) so the part's buffers stay aligned regardless of uv shape. + */ + _addToCurrentPart(input, vertices, normals, vertexColors) { + // parts are made of fills; a stroke-only draw contributes no faces + if (!this.renderer.states.fillColor) return; + + const state = this._snapshotPartState(); + if ( + !this.currentPart || + !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; + const startIdx = part.vertices.length; + for (const v of vertices) { + part.vertices.push(v); + } + for (const vn of normals) { + part.vertexNormals.push(vn); + } + for (const val of input.uvs) { + part.uvs.push(val); + } + for (const c of vertexColors) { + part.vertexColors.push(c); + } + for (const f of input.faces) { + part.faces.push(f.map(idx => idx + startIdx)); + } } /** @@ -174,6 +273,15 @@ class GeometryBuilder { */ finish() { this.renderer._pInst.pop(); + // expose the material parts only when there really are multiple materials, + // and not while custom per-vertex attributes are in play (those aren't + // split per part yet). single-material builds keep the geometry as its own + // part, so nothing changes for them (zero regression). + const hasUserProps = + Object.keys(this.geometry.userVertexProperties).length > 0; + if (this.parts.length >= 2 && !hasUserProps) { + this.geometry.parts = this.parts; + } return this.geometry; } } diff --git a/test/unit/webgl/p5.Geometry.js b/test/unit/webgl/p5.Geometry.js index f48ecbef2e..bb838adc94 100644 --- a/test/unit/webgl/p5.Geometry.js +++ b/test/unit/webgl/p5.Geometry.js @@ -328,5 +328,55 @@ suite('p5.Geometry', function() { expect(geom.vertices.length).toBeGreaterThan(0); expect(geom.faces.length).toEqual(0); }); + + test('a texture change splits the build into parts', function() { + myp5.createCanvas(50, 50, myp5.WEBGL); + const texA = myp5.createGraphics(10, 10); + const texB = myp5.createGraphics(10, 10); + const geom = myp5.buildGeometry(() => { + myp5.texture(texA); + myp5.box(8); + myp5.texture(texB); + myp5.sphere(8); + }); + // one part per material, in draw order + expect(geom.parts.length).toEqual(2); + expect(geom.parts[0].partState.texture).not.toBeNull(); + expect(geom.parts[1].partState.texture).not.toBeNull(); + expect(geom.parts[0].partState.texture) + .not.toEqual(geom.parts[1].partState.texture); + }); + + test('a fill change alone does not split the build', function() { + myp5.createCanvas(50, 50, myp5.WEBGL); + const geom = myp5.buildGeometry(() => { + myp5.fill('red'); + myp5.box(8); + myp5.fill('blue'); + myp5.sphere(8); + }); + // fill bakes into vertexColors, so the geometry stays a single part + expect(geom.parts.length).toEqual(1); + expect(geom.vertexColors.length).toBeGreaterThan(0); + }); + + test('per-part materials render the same as drawing them directly', + function() { + assertGeometryRendersMatch(function() { + myp5.push(); + myp5.translate(-10, 0); + myp5.specularMaterial(255, 0, 0); + myp5.shininess(50); + myp5.box(8); + myp5.pop(); + myp5.push(); + myp5.translate(10, 0); + myp5.specularMaterial(0, 0, 255); + myp5.shininess(200); + myp5.box(8); + myp5.pop(); + }, [checkLights]); + } + ); }); }); From 18cdaf9f21d08f1d64ca19a0dc37dab53bfcf160 Mon Sep 17 00:00:00 2001 From: nityam Date: Sun, 9 Aug 2026 21:29:22 +0530 Subject: [PATCH 2/5] test per-part instancing on multi-material buildGeometry --- test/unit/webgl/p5.Geometry.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/unit/webgl/p5.Geometry.js b/test/unit/webgl/p5.Geometry.js index bb838adc94..7e957ea262 100644 --- a/test/unit/webgl/p5.Geometry.js +++ b/test/unit/webgl/p5.Geometry.js @@ -378,5 +378,32 @@ suite('p5.Geometry', function() { }, [checkLights]); } ); + + test('instancing draws every material part with the instance count', + 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.texture(texA); + myp5.box(8); + myp5.texture(texB); + myp5.sphere(8); + }); + expect(geom.parts.length).toEqual(2); + + const fillSpy = vi.spyOn(renderer, '_drawFills'); + myp5.background(255); + myp5.fill(255); + myp5.model(geom, 4); + + // one instanced draw per part, each carrying the same instance count + expect(fillSpy).toHaveBeenCalledTimes(geom.parts.length); + for (const call of fillSpy.mock.calls) { + expect(call[1].count).toEqual(4); + } + fillSpy.mockRestore(); + } + ); }); }); From fad82edac9eb9afcb5a1b5a6ecbd71552cdbd72c Mon Sep 17 00:00:00 2001 From: nityam Date: Sun, 9 Aug 2026 21:47:34 +0530 Subject: [PATCH 3/5] lowercase and trim comments --- src/webgl/GeometryBuilder.js | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/webgl/GeometryBuilder.js b/src/webgl/GeometryBuilder.js index c42a9b70d3..faaede957e 100644 --- a/src/webgl/GeometryBuilder.js +++ b/src/webgl/GeometryBuilder.js @@ -27,7 +27,7 @@ class GeometryBuilder { // material parts. when the material state (texture, specular, ambient, // shininess) changes between draws inside the callback, a new part is // opened, so model() renders the result per part like a multi-material obj. - // fill stays baked into vertexColors, so a plain colour change never splits. + // fill stays baked into vertexColors, so a plain fill change never splits. this.parts = []; this.currentPart = null; } @@ -139,11 +139,9 @@ class GeometryBuilder { /** * @private - * Snapshots the renderer's current per-part material state. Only the material - * uniforms that cannot be stored per vertex are tracked here (texture, - * specular, ambient, shininess); fill stays baked into vertexColors, so a - * plain fill() change never opens a new part. Uses p5's own state names, the - * same vocabulary the .mtl importer translates into. + * snapshot the material state that can't live per vertex (texture, specular, + * ambient, shininess), in p5's own state names. fill stays in vertexColors, so + * a plain fill() change never opens a new part. */ _snapshotPartState() { const s = this.renderer.states; @@ -155,21 +153,12 @@ class GeometryBuilder { return state; } - /** - * @private - * Compares two colour arrays (or nulls) for equality. - */ _sameColor(a, b) { if (a === b) return true; if (!a || !b) return false; return a.length === b.length && a.every((v, i) => v === b[i]); } - /** - * @private - * True when two part states describe the same material, so consecutive draws - * can share one part. - */ _sameMaterial(a, b) { return a.texture === b.texture && a.shininess === b.shininess && @@ -179,10 +168,9 @@ class GeometryBuilder { /** * @private - * Appends one draw's geometry to the current material part, opening a new - * part when the material state has changed since the last draw. Data is - * copied the same way as the combined geometry above (element by element, in - * vertex order) so the part's buffers stay aligned regardless of uv shape. + * append one draw to the current part, opening a new part when the material + * changed. copied element by element like the combined geometry above, so the + * 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 From dcd46d763fe3bf2faf54dd1a01fa5e4037b2a8c2 Mon Sep 17 00:00:00 2001 From: nityam Date: Sun, 9 Aug 2026 22:18:14 +0530 Subject: [PATCH 4/5] add visual test for multi-material buildGeometry --- test/unit/visual/cases/webgl.js | 27 ++++++++++++++++++ .../000.png | Bin 0 -> 2169 bytes .../metadata.json | 3 ++ 3 files changed, 30 insertions(+) create mode 100644 test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/000.png create mode 100644 test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/metadata.json diff --git a/test/unit/visual/cases/webgl.js b/test/unit/visual/cases/webgl.js index ed2af5fdc6..d44e38a871 100644 --- a/test/unit/visual/cases/webgl.js +++ b/test/unit/visual/cases/webgl.js @@ -932,6 +932,33 @@ visualSuite('WebGL', function () { p5.model(geom); screenshot(); }); + + visualTest( + 'a texture change splits into parts that each keep their texture', + async function (p5, screenshot) { + p5.createCanvas(50, 50, p5.WEBGL); + const texA = await p5.loadImage('test/unit/assets/cat.jpg'); + const texB = await p5.loadImage('test/unit/assets/spheremap.jpg'); + const geom = p5.buildGeometry(() => { + p5.push(); + p5.translate(-12, 0, 0); + p5.texture(texA); + p5.box(12); + p5.pop(); + p5.push(); + p5.translate(12, 0, 0); + p5.texture(texB); + p5.box(12); + p5.pop(); + }); + p5.background(255); + p5.rotateX(0.4); + p5.rotateY(0.4); + p5.noStroke(); + p5.model(geom); + screenshot(); + } + ); }); visualSuite('font data', () => { diff --git a/test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/000.png b/test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/000.png new file mode 100644 index 0000000000000000000000000000000000000000..4d7b4e45751e60e11da7ac0c6917b872f98a8db6 GIT binary patch literal 2169 zcmV-<2!{8GP)Nwi!4MEO9&;#G6Jd+yCm=DnHz({u7B;xxlC`6bW$p8S6A^L?K0^L>85_vXrK zUXUT<1sTmmo;M+z6=_zai7L?CT1`~gr-p4-24WRLErtDjk)WOhz=;?s&BAhL3+c`VJn2wsl(yGh)!|Jun)L z$mQ}{YcTSU51hH4#s{Cr@hT-r447}XCyZ}pH-h-5|cO4tKTrJ+8i=7lBjUi(6&=g zCQg`yMJJ~m0~_{U!Js!%TwL;#gzNg_OmrGo)AAAIGE^E-b%l}|0R*)clUYxtprW!` zMRb~tPtPT?>2G)W<1y(2MUqHCejyK&lkoELsom%v;@wu1O|=*>*x!QD>`95b9l@g( z6TW&MLw~iL3b`7a&4wUI!r~sMucvS`)kc8Xips1(=hX_O)q>K~1l4lf2a2DUk)ZZ1 z@NelMwzmuMI}%(jm)pHglZ;p;I(6*Wg(=HEA??N)3Jdd5ihDdUC6%Ce-cvd=mfZM=@LcuVp{rkz|Yqkh01`rO%Q*In8D7~ zuhBDXK6i4}WacG

oJ&Kx)J!Jg!}`c&G^u8Bcj$0`aGZpvyZ(-}at#^6}*SzI9@I z840k;Fp2xGvc$o~J>lpr+FIYbE|Ns-rRDSs(({)Omk|5Ulepg_L#8ie!N#LZ43Fe~ zau%wzdna&|m(smk5LRNiePKGe z(QAmie~rT*4dLSt$J3^T9F0aGGd+P@M^_;=B#D^JCVYZ=Gj>h{3Zx5}bS4X7vsa@vd2uV* z&caE3$rN99WjQHaI(>>o;|DP$#GmDhmvUr#B&(+mqA0(BYDWoq;#~^Y*T_5}yyuqKMJ?r&+2pkH({m**mhvha6DGgJ@?|5~uwjxEERsYfhrfeU zqh(;%HjL@*kEP=P+J^KMkHjk8Tr!VcXJfb-|Cj|~p(KfIWwDxZiZ9Kj(?<|IY_?>; zviL0|#r++XO8kT%=upZv#3wx!vU7_i zaV3eMH86E>cftmC#vmTGt<(<88W}zM^rDu&Okmg2flQ0oh;P?W-rRYfPmf>0r^h%( zj~Xc%&}gcrus~E!ZlnvntWMj)O zlf;!IGI?+hM)V4--Q<~%GDy!VCf^1ng$|O_b2+$dF#6V=xOVa=vsZ3JCRX;n4I5ay zZk=Q>_d;1|wRpT55nK?ToQSr?Nb+mOlk4y#HCshtaW+5XB;Zn&;8MC!8FeJ2CsJBk zgwm*TL)2-KL{3EAXZ*r4^eSUw_YqbLrw<6uA6lJpSHBT6hT< zRB9|f8j8xYkQ$Oi+zx(0A-HYO#JS8|wSyTETUoRFvPf_v)0Vu8cjx~6kImf7@pqkGjc>*Ow4BwiAoy4n?asV=t8B+ommQ zLBZq^3)~QOTD?T-^bjX~1A_?+>dxp_XYtz1H6n>farVi6bMo@Io9d6rcO14nCwCL< z{QJ8+mi;D-MHBjo?~5q1i}Lt3CY|fI;&>4Mkdjh6!-viwY~(!2AWua^672~V0)ocy z%h1-@U-ltpV-t}>) zU6a*bpW)@DF5MZ>cOZR#F^ORV!kIKaf)(?2GiT~rme1cx&u;ymD7$A)L@LV;0X?La z>YRUWiPV{GJ(JBu>M71MS$bX-X>>7~i8Q)oP3+LDND~*h(eTZRG`eI>?9i-86BoGA v@Gp!a{{a91|NkN#{W|~v00v1!K~w_(@wT1H_BBWM00000NkvXXu0mjfuW}~s literal 0 HcmV?d00001 diff --git a/test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/metadata.json b/test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/metadata.json new file mode 100644 index 0000000000..2d4bfe30da --- /dev/null +++ b/test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/metadata.json @@ -0,0 +1,3 @@ +{ + "numScreenshots": 1 +} \ No newline at end of file From ccfc52693a1894512940ec949dd87fed75468589 Mon Sep 17 00:00:00 2001 From: nityam Date: Sun, 9 Aug 2026 22:22:12 +0530 Subject: [PATCH 5/5] add visual test for mixed textured and untextured parts --- test/unit/visual/cases/webgl.js | 26 ++++++++++++++++++ .../000.png | Bin 0 -> 1456 bytes .../metadata.json | 3 ++ 3 files changed, 29 insertions(+) create mode 100644 test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/000.png create mode 100644 test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/metadata.json diff --git a/test/unit/visual/cases/webgl.js b/test/unit/visual/cases/webgl.js index d44e38a871..a2daf5eeea 100644 --- a/test/unit/visual/cases/webgl.js +++ b/test/unit/visual/cases/webgl.js @@ -959,6 +959,32 @@ visualSuite('WebGL', function () { screenshot(); } ); + + visualTest( + 'a textured part and an untextured part both render', + async function (p5, screenshot) { + p5.createCanvas(50, 50, p5.WEBGL); + const tex = await p5.loadImage('test/unit/assets/cat.jpg'); + const geom = p5.buildGeometry(() => { + p5.push(); + p5.translate(-12, 0, 0); + p5.texture(tex); + p5.box(12); + p5.pop(); + p5.push(); + p5.translate(12, 0, 0); + p5.fill('red'); + p5.box(12); + p5.pop(); + }); + p5.background(255); + p5.rotateX(0.4); + p5.rotateY(0.4); + p5.noStroke(); + p5.model(geom); + screenshot(); + } + ); }); visualSuite('font data', () => { diff --git a/test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/000.png b/test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/000.png new file mode 100644 index 0000000000000000000000000000000000000000..d6dc63e4961d83a9d0c879e1366b1cea7a66bc9c GIT binary patch literal 1456 zcmV;h1yA~kP)(9LFs&v?CZI=L8Eq}%3Os`=Y?B`-n8?758r^U3<@2EdnbQNbFbm)*$yu+FEkoW zoiv&GsllpM5EcgVDvBVAPcxB9qV;NzwW<;i z=W0r&DYQwAnL7JbXLq*T~kB1ZatYCvxV#n z$0#i=##y?@?_}L0`+gxRlaY~QMuXaorPGIyb!rnkmd^l{8@194rNa&oawD$8pCe(BbD5KLfC*6v+{i4zC2$}i<083q<}^8%Pm`Bh zgp0Ek6*VtXQ>%OxL3G|ae1irs_On==oL$*}DxJ;o3)!-AHs?<6W@+NL%$PNwJ6Q!p zE&qn-^~vZu1(LA)9Mv_|ttzHTCs9;1Q7dUYbUuX7jKf{mk;|Dz=1v?$zVxtrRG3BT z(O;Q2ei&hW0$4OZn*Ey+h>I9TS#b%q<_e0WRVl8$_7y=UMXko-=FZSwof$JA0H1E} zBi^(w|y1VOrCBSL!< zKC}l~>8JK>S2NyjYWfcB53-k9#kT07OpQ&#uU80*x2Ev@p|kk*9mnWVA1WramqISd zX6#9!eHqz5ud-? zXYWl#ki%zgFn;b@CNEsih=@5jsUFc@`rnnAag)0@uaaZYVl=DybV&@?uUur!s@23N zydDU-Ns1t{vP)1OWEnPL7Soq)W?Jlbtl0j$1enCs1#5Yy#}Izmzl&oh(nxp}5PH2_ zmQ86C#HNR|=@%GGU{G&He>{UJ(^p6k3DT~e-+Db#BE=-F0#S_C9EBi&AT-DFnN3<+ zqY$#;s|QJtrY9F>wSp}2XIb=m5CjkexvqK<1dw%tP=DRX`PG9oEJhGO5b8TF7cI+w z0R^FrT!?BJM)og3WZ z@Q#VJxMg;Ba7@I`4Q_GxH)bOL0ssL2|5nhVm;e9(21!IgR09AWLVUZbgHDnF0000< KMNUMnLSTaNb+TFj literal 0 HcmV?d00001 diff --git a/test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/metadata.json b/test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/metadata.json new file mode 100644 index 0000000000..2d4bfe30da --- /dev/null +++ b/test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/metadata.json @@ -0,0 +1,3 @@ +{ + "numScreenshots": 1 +} \ No newline at end of file