Skip to content

Commit db2ceae

Browse files
authored
Merge pull request #8342 from processing/fix/webgpu-memory-leak
Fix WebGPU buffer memory leaks
2 parents 05fa9d1 + de2e1e6 commit db2ceae

File tree

7 files changed

+197
-15
lines changed

7 files changed

+197
-15
lines changed

src/webgpu/p5.RendererWebGPU.js

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -286,22 +286,29 @@ function rendererWebGPU(p5, fn) {
286286
}
287287
}
288288

289-
const raw = map ? map(srcData) : srcData;
290-
const typed = this._normalizeBufferData(raw, Float32Array);
289+
// Check if we already have a buffer for this data
290+
let existingBuffer = buffers[dst];
291+
const needsNewBuffer = !existingBuffer;
292+
293+
// Only create new buffer and write data if buffer doesn't exist or data is dirty
294+
if (needsNewBuffer || geometry.dirtyFlags[src] !== false) {
295+
const raw = map ? map(srcData) : srcData;
296+
const typed = this._normalizeBufferData(raw, Float32Array);
291297

292-
// Always use pooled buffers - let the pool system handle sizing and reuse
293-
const pooledBufferInfo = this._getVertexBufferFromPool(geometry, dst, typed.byteLength);
298+
// Get pooled buffer (may reuse existing or create new)
299+
const pooledBufferInfo = this._getVertexBufferFromPool(geometry, dst, typed.byteLength);
294300

295-
// Create a copy of the data to avoid conflicts when geometry arrays are reset
296-
const dataCopy = new typed.constructor(typed);
297-
pooledBufferInfo.dataCopy = dataCopy;
301+
// Create a copy of the data to avoid conflicts when geometry arrays are reset
302+
const dataCopy = new typed.constructor(typed);
303+
pooledBufferInfo.dataCopy = dataCopy;
298304

299-
// Write the data to the pooled buffer
300-
device.queue.writeBuffer(pooledBufferInfo.buffer, 0, dataCopy);
305+
// Write the data to the pooled buffer
306+
device.queue.writeBuffer(pooledBufferInfo.buffer, 0, dataCopy);
301307

302-
// Update the buffers cache to use the pooled buffer
303-
buffers[dst] = pooledBufferInfo.buffer;
304-
geometry.dirtyFlags[src] = false;
308+
// Update the buffers cache to use the pooled buffer
309+
buffers[dst] = pooledBufferInfo.buffer;
310+
geometry.dirtyFlags[src] = false;
311+
}
305312

306313
shader.enableAttrib(attr, size);
307314
}
@@ -354,9 +361,9 @@ function rendererWebGPU(p5, fn) {
354361
}
355362
};
356363

357-
freeDefs(this.renderer.buffers.stroke);
358-
freeDefs(this.renderer.buffers.fill);
359-
freeDefs(this.renderer.buffers.user);
364+
freeDefs(this.buffers.stroke);
365+
freeDefs(this.buffers.fill);
366+
freeDefs(this.buffers.user);
360367
}
361368

362369
_getValidSampleCount(requestedCount) {
@@ -1014,6 +1021,11 @@ function rendererWebGPU(p5, fn) {
10141021
// Return all uniform buffers to their pools
10151022
this._returnUniformBuffersToPool();
10161023

1024+
// Mark all geometry buffers for return after frame is complete
1025+
for (const geometry of this._geometriesWithPools) {
1026+
this._markGeometryBuffersForReturn(geometry);
1027+
}
1028+
10171029
// Return all vertex buffers to their pools
10181030
this._returnVertexBuffersToPool();
10191031

test/unit/visual/cases/webgpu.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -752,6 +752,44 @@ visualSuite("WebGPU", function () {
752752
);
753753
});
754754

755+
visualSuite("Immediate Mode Buffer Reuse", function () {
756+
visualTest(
757+
"beginShape/endShape reuses buffers across frames",
758+
async function (p5, screenshot) {
759+
await p5.createCanvas(50, 50, p5.WEBGPU);
760+
761+
// Draw the same triangle in different positions across 3 frames
762+
// Each frame draws the same number of vertices (3) so that it
763+
// isn't FORCED to allocate new buffers, and should be trying
764+
// to reuse them.
765+
const positions = [
766+
{ x: -15, y: -10 }, // Frame 1: left
767+
{ x: 0, y: -10 }, // Frame 2: center
768+
{ x: 15, y: -10 } // Frame 3: right
769+
];
770+
771+
for (let frame = 0; frame < 3; frame++) {
772+
const pos = positions[frame];
773+
774+
p5.background(200);
775+
p5.fill('red');
776+
p5.noStroke();
777+
778+
// Draw triangle using immediate mode. This means it's using the
779+
// same geometry every frame. We expect to see different results
780+
// if it's correctly updating the buffers.
781+
p5.beginShape();
782+
p5.vertex(pos.x - 5, pos.y + 10); // bottom-left
783+
p5.vertex(pos.x + 5, pos.y + 10); // bottom-right
784+
p5.vertex(pos.x, pos.y); // top
785+
p5.endShape(p5.CLOSE);
786+
787+
await screenshot();
788+
}
789+
}
790+
);
791+
});
792+
755793
visualSuite("Image Based Lighting", function () {
756794
const shinesses = [50, 150];
757795
for (const shininess of shinesses) {
350 Bytes
Loading
344 Bytes
Loading
349 Bytes
Loading
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"numScreenshots": 3
3+
}
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
import p5 from '../../../src/app.js';
2+
import rendererWebGPU from "../../../src/webgpu/p5.RendererWebGPU";
3+
4+
p5.registerAddon(rendererWebGPU);
5+
6+
suite('WebGPU p5.RendererWebGPU', function() {
7+
let myp5;
8+
let prevPixelRatio;
9+
10+
beforeAll(async function() {
11+
prevPixelRatio = window.devicePixelRatio;
12+
window.devicePixelRatio = 1;
13+
myp5 = new p5(function(p) {
14+
p.setup = function() {};
15+
});
16+
});
17+
18+
beforeEach(async function() {
19+
await myp5.createCanvas(50, 50, 'webgpu');
20+
});
21+
22+
afterEach(function() {
23+
myp5.remove();
24+
});
25+
26+
afterAll(function() {
27+
window.devicePixelRatio = prevPixelRatio;
28+
});
29+
30+
suite('Buffer Pooling', function() {
31+
test('drawing geometry twice reuses vertex buffers', async function() {
32+
// Create a simple geometry
33+
const geom = myp5.buildGeometry(() => {
34+
myp5.triangle(0, 0, 10, 0, 5, 10);
35+
});
36+
37+
// Draw the geometry once
38+
myp5.background(255);
39+
myp5.model(geom);
40+
41+
// Check the vertex buffer pool size for position attribute
42+
const poolForVertexBuffer = geom._vertexBufferPools?.vertexBuffer;
43+
expect(poolForVertexBuffer).to.exist;
44+
const initialPoolSize = poolForVertexBuffer.length;
45+
const initialInUseSize = geom._vertexBuffersInUse?.vertexBuffer?.length || 0;
46+
47+
// Draw the geometry again
48+
myp5.background(255);
49+
myp5.model(geom);
50+
51+
// Verify the pool hasn't grown - buffers should be reused
52+
const finalPoolSize = poolForVertexBuffer.length;
53+
const finalInUseSize = geom._vertexBuffersInUse?.vertexBuffer?.length || 0;
54+
55+
// Pool size should stay the same or be smaller (buffers moved from pool to in-use)
56+
// The total number of buffers (pool + in-use) should remain constant
57+
expect(initialPoolSize + initialInUseSize).to.equal(finalPoolSize + finalInUseSize);
58+
});
59+
60+
test('freeGeometry causes new buffer allocation on next draw', async function() {
61+
// Create a simple geometry
62+
const geom = myp5.buildGeometry(() => {
63+
myp5.triangle(0, 0, 10, 0, 5, 10);
64+
});
65+
66+
// Draw the geometry once
67+
myp5.background(255);
68+
myp5.model(geom);
69+
70+
// Get initial buffer count
71+
const poolForVertexBuffer = geom._vertexBufferPools?.vertexBuffer;
72+
expect(poolForVertexBuffer).to.exist;
73+
const initialTotalBuffers = poolForVertexBuffer.length +
74+
(geom._vertexBuffersInUse?.vertexBuffer?.length || 0);
75+
76+
// Free the geometry
77+
myp5.freeGeometry(geom);
78+
79+
// Draw the geometry again
80+
myp5.background(255);
81+
myp5.model(geom);
82+
83+
// After freeGeometry, new buffers should be allocated
84+
const finalTotalBuffers = poolForVertexBuffer.length +
85+
(geom._vertexBuffersInUse?.vertexBuffer?.length || 0);
86+
87+
// We should have more buffers now since freeGeometry marks geometry as dirty
88+
// and new buffers need to be created
89+
expect(finalTotalBuffers).to.be.greaterThan(initialTotalBuffers);
90+
});
91+
92+
test('immediate mode geometry reuses buffers across frames', async function() {
93+
// Function to draw the same shape using immediate mode
94+
const drawSameShape = () => {
95+
myp5.background(255);
96+
myp5.beginShape();
97+
myp5.vertex(0, 0);
98+
myp5.vertex(10, 0);
99+
myp5.vertex(5, 10);
100+
myp5.endShape();
101+
};
102+
103+
// Draw the shape for the first frame
104+
drawSameShape();
105+
await myp5._renderer.finishDraw();
106+
107+
// Get the immediate mode geometry (shapeBuilder geometry)
108+
const immediateGeom = myp5._renderer.shapeBuilder.geometry;
109+
const poolForVertexBuffer = immediateGeom._vertexBufferPools?.vertexBuffer;
110+
expect(poolForVertexBuffer).to.exist;
111+
112+
const initialTotalBuffers = poolForVertexBuffer.length +
113+
(immediateGeom._vertexBuffersInUse?.vertexBuffer?.length || 0);
114+
115+
// Draw the same shape for several more frames
116+
for (let frame = 0; frame < 5; frame++) {
117+
drawSameShape();
118+
await myp5._renderer.finishDraw();
119+
120+
// Check that total buffer count hasn't increased
121+
const currentTotalBuffers = poolForVertexBuffer.length +
122+
(immediateGeom._vertexBuffersInUse?.vertexBuffer?.length || 0);
123+
124+
expect(currentTotalBuffers).to.equal(initialTotalBuffers,
125+
`Buffer count should stay constant across frames (frame ${frame})`);
126+
}
127+
});
128+
});
129+
});

0 commit comments

Comments
 (0)