Allowing per-vertex fill / stroke changes significantly decreases performance

[wxs]

Nature of issue?

• Found a bug
• Existing feature enhancement
• New feature request

Most appropriate sub-area of p5.js?

• Color
• Core
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL

New feature details:

As part of my investigation of #2011, I found that drawing complex shapes with beginShape() and endShape() can be very slow with the default Renderer2D

This can be significantly improved by batching calls to the fill and stroke methods of the canvas. One reason right now they are not batched is that the renderer partially supports per-vertex colours, and the way this is implemented is by re-setting the fill and stroke for each primitive in the shape. Here is what that looks like for the TRIANGLES shape type:

 if (shapeKind === constants.TRIANGLES) {
      for (i = 0; i + 2 < numVerts; i += 3) {
        v = vertices[i];
        this.drawingContext.beginPath();
        this.drawingContext.moveTo(v[0], v[1]);
        this.drawingContext.lineTo(vertices[i + 1][0], vertices[i + 1][1]);
        this.drawingContext.lineTo(vertices[i + 2][0], vertices[i + 2][1]);
        this.drawingContext.lineTo(v[0], v[1]);
        if (this._doFill) {
          this._pInst.fill(vertices[i + 2][5]);
          this.drawingContext.fill();
        }
        if (this._doStroke) {
          this._pInst.stroke(vertices[i + 2][6]);
          this.drawingContext.stroke();
        }
        this.drawingContext.closePath();
      }
    }

Note that it calls drawingContext.fill() for every single triangle separately.

I have an experimental branch on my machine where I batch these calls instead, so that I only call fill and stroke once for the entire set of triangles, and I see order of magnitude performance improvements when drawing shapes that include many triangles.

Solutions

There are two ways to solve this:

A) Stop supporting per-vertex-colours

The simplest and less error-prone is just to stop supporting per-vertex colours. These are not documented in p5.js anyway, and in Processing are not supported by the default renderer.

B) Detect when vertex colours change

The more complex approach is to batch the calls most of the time, but detect when colours have been changed per-vertex and in that case "un-batch" the calls.

--

I prefer the first approach, as if the user wants they can always just use multiple calls to beginShape and endShape, changing the fill and stroke in between. This has the added benefit that the vertex colour behaviour currently is pretty non-intuitive. For instance, if drawing with TRIANGLES only the colour of every third vertex matters and the rest are ignored.

I have the more complex approach partially implemented (just for TRIANGLE_FAN so far). The advantage here is that it's backwards compatible. The disadvantage is that it makes the code more complex and bug prone. It is also perhaps not as intuitive to users that they will see a performance decrease when setting fills and strokes inside a beginShape, this might be more obvious if they had to explicitly endShape and then beginShape again.

[lmccart]

I'm in favor of removing support for per-vertex-colors rather than adding to the complexity of the code further. are there other opinions.

[wxs]

If you do decide you want to move forward with this I can re-visit my branch where I tested it out and do a PR. LMK!

[Spongman]

just wanted to note that while Processing doesn't support per-vertex attributes in its default mode, the P2D and P3D modes do have this support, and that doing this in p5.js WEBGL mode would be trivial.

void setup() {
  size(400, 400, P3D);
}

void draw() {
  beginShape();

  fill(255, 0, 0);
  vertex(0, 0, 0);
  fill(0, 255, 0);
  vertex(width, height/2, 0);
  fill(0, 0, 255);
  vertex(0, height, 0);

  endShape(CLOSE);
}

[davepagurek]

Hi! I'm going to close this for now as it seems that as of 1.5.0, 2D mode doesn't support per-vertex fills, and WebGL mode does in the way described in the comment above. Feel free to reopen this or continue discussing if new performance issues come up with the WebGL version of this!