diff --git a/src/webgl/GeometryBuilder.js b/src/webgl/GeometryBuilder.js index b3b2d20216..7f34911bc7 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 fill change never splits. + this.parts = []; + this.currentPart = null; } /** @@ -64,11 +72,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) { @@ -121,6 +131,83 @@ class GeometryBuilder { for (const c of vertexColors) { this.geometry.vertexColors.push(c); } + + this._addToCurrentPart( + input, + transformedVertices, + transformedNormals, + vertexColors + ); + } + + /** + * @private + * 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; + 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; + } + + _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]); + } + + _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 + * 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 + 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)); + } } /** @@ -179,6 +266,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/visual/cases/webgl.js b/test/unit/visual/cases/webgl.js index ed2af5fdc6..a2daf5eeea 100644 --- a/test/unit/visual/cases/webgl.js +++ b/test/unit/visual/cases/webgl.js @@ -932,6 +932,59 @@ 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(); + } + ); + + 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 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 0000000000..4d7b4e4575 Binary files /dev/null and b/test/unit/visual/screenshots/WebGL/buildGeometry()/a texture change splits into parts that each keep their texture/000.png differ 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 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 0000000000..d6dc63e496 Binary files /dev/null and b/test/unit/visual/screenshots/WebGL/buildGeometry()/a textured part and an untextured part both render/000.png differ 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 diff --git a/test/unit/webgl/p5.Geometry.js b/test/unit/webgl/p5.Geometry.js index 44b492b859..85631d8f91 100644 --- a/test/unit/webgl/p5.Geometry.js +++ b/test/unit/webgl/p5.Geometry.js @@ -336,5 +336,82 @@ 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]); + } + ); + + 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(); + } + ); }); });