Browser freeze when creating >65k vertices in draw() loop (no validation or warning)

[Homaid]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• p5.strands
• WebGL
• DevOps, Build process, Unit testing
• Internationalization (i18n)
• Friendly Errors
• Other (specify if possible)

p5.js version

1.11.10 (also tested on 2.0.5,issue exists in all versions)

Web browser and version

all browser

Operating system

windows 11

Steps to reproduce this

Steps:

1. Paste the code below into the p5.js web editor (editor.p5js.org)
2. Click "Play"
3. Wait 1-2 seconds

Snippet:

function setup() {
  createCanvas(400, 400, WEBGL);
}

function draw() {
  background(200);
  beginShape();
  
  // Creating 100,000 vertices (exceeds 65k limit)
  for (let i = 0; i < 100000; i++) {
    vertex(random(-200, 200), random(-200, 200), random(-200, 200));
  }
  
  endShape();
}

[Homaid]

Hiii @davepagurek Currently, when users exceed the 65k vertex limit, there's complete browser freeze.it would helpful if we add a warning/error before reaching 65k vertex limit? this could save users hours of debugging

[Homaid]

I also noticed p5.js uses Uint16Array for vertex indices, which limits us to 65k vertices. I'm curious since The OES_element_index_uint extension has been widely supported using Uint32Array with this extension would increase the limit of vertices, was there a specific reason to stick with 16-bit indices?

[davepagurek]

This is supposedly already handled:

p5.js/src/webgl/GeometryBufferCache.js

Lines 58 to 75 in f78009a

	// If any face references a vertex with an index greater than the maximum
	// un-singed 16 bit integer, then we need to use a Uint32Array instead of a
	// Uint16Array
	const hasVertexIndicesOverMaxUInt16 = vals.some(v => v > 65535);
	let type = hasVertexIndicesOverMaxUInt16 ? Uint32Array : Uint16Array;
	this.renderer._bindBuffer(
	indexBuffer,
	gl.ELEMENT_ARRAY_BUFFER,
	vals,
	type
	);
	// If we're using a Uint32Array for our indexBuffer we will need to pass a
	// different enum value to WebGL draw triangles. This happens in
	// the _drawElements function.
	buffers.indexBufferType = hasVertexIndicesOverMaxUInt16
	? gl.UNSIGNED_INT
	: gl.UNSIGNED_SHORT;

So that tells me there's maybe a bug in the implementation that needs fixing, rather than a warning needing to be added?

[Homaid]

oh so GeometryBufferCache.js does handle 32-bit indices but i think it only works for pre-built shapes like sphere(), box(), loaded models and (beginShape()/vertex()/endShape()) uses a separate rendering path that doesn't include the 32-bit index logic and does not use GeometryBufferCache.js what do you think @davepagurek

[Homaid]

and from the code GeometryBufferCache.js has a method called ensureCached(geometry) that takes a p5.Geometry object with a geometry ID,only applies to pre-built shapes

[davepagurek]

Both immediate-mode shapes (beginShape/endShape) and cached shapes (sphere, model, etc) end up going throw _drawGeometry. A shape gets turned into a geometry and then drawn the usual way here:

p5.js/src/webgl/p5.RendererGL.js

Lines 579 to 582 in f78009a

	this._drawGeometry(this.shapeBuilder.geometry, {
	mode: this.shapeBuilder.shapeMode,
	count: this.drawShapeCount
	});

So I think they should both end up through the same code path, but immediate mode shapes end up reusing the same geometry and just resetting it each time: https://github.com/processing/p5.js/blob/dev-2.0/src/webgl/ShapeBuilder.js#L52

So it could be that it's accidentally caching an earlier index count from a previous shape with less indices?

[Homaid]

Yes, i think you're right When immediate mode reuses the same p5.Geometry object (same gid), the check if (this.isCached(geometry.gid)) return this.getCached(geometry); returns the old cached buffers immediately.
The code that checks vals.some(v => v > 65535) and sets the correct indexBufferType never runs again because it's after the early return!

[Homaid]

So if frame 1 has<65k vertices (caches with UNSIGNED_SHORT) and frame 2 has >65k vertices, it still uses the old UNSIGNED_SHORT buffer type, causing the freeze.

[davepagurek]

got it, thanks for debugging this! So I think we have to make sure that part gets reset too for immediate mode geometry (or like, maybe having one that we use for big shapes and one for small shapes, so we arent recreating buffers every time?)

[Homaid]

i think the second approach would be better since we can check the vertex count early and switch between two geometry objects (small shapes for <65k vertices and big shapes for >65k vertice) so it will not create any buffer every frame.