Modify the lightingShader to allow per-vertex coloring

[inaridarkfox4231]

Currently, if you try to do per-vertex coloring, for example in immediateMode, using functions like pointLight won't enable it.
By modifying the shader, we will be able to achieve both coloring for each vertex and lighting processing by the shader.
Also, even in retainedMode, if vertexColors contains a color element, it will be possible to color each vertex.

Resolves #5936

Changes

Source code:

function setup() {
  createCanvas(400, 400, WEBGL);
  noStroke();
  
  pointLight(0, 255, 255, 0, 0, 30);

  beginShape();
  fill(255);
  vertex(-100,-100);
  fill(255,0,0);
  vertex(100,-100);
  fill(0,255,0);
  vertex(100,100);
  fill(0,0,255);
  vertex(-100,100);
  endShape();
}

Screenshots of the change:

PR Checklist

• WebGL
• Color

[inaridarkfox4231]

I don't know what went wrong...

[davepagurek]

Looks like the tests timed out. If you run npm run test locally, does it complete? And also does it run significantly slower than running the same command off of main?

[davepagurek]

Oh never mind, I don't think it's you, I think it was caused by this dependabot change: #5922 I'm going to see if reverting it causes tests to pass again

[davepagurek]

Ok looks like that fixes it for now. #5939 isn't merged into main yet but in the mean time you can run git merge revert-5922-dependabot/npm_and_yarn/flat-and-mocha-5.0.2 to merge that PR's branch into your branch to get your tests to run.

[inaridarkfox4231]

Thank you for your reply! Ok, I'll wait until the version comes back before doing that. Also, I'm thinking of rewriting the code in the direction of rewriting the shader instead of creating a new shader.

[inaridarkfox4231]

I created 6 unit tests...

First, tests that vertex colors are valid under the lighting shader in immediate mode.

test('immediate mode uses vertex colors (noLight)', function(done) {
  const renderer = myp5.createCanvas(256, 256, myp5.WEBGL);

  // upper color: (200, 0, 0, 255);
  // lower color: (0, 0, 200, 255);
  // expected center color: (100, 0, 100, 255);

  myp5.beginShape();
  myp5.fill(200, 0, 0);
  myp5.vertex(-128, -128);
  myp5.fill(200, 0, 0);
  myp5.vertex(128, -128);
  myp5.fill(0, 0, 200);
  myp5.vertex(128, 128);
  myp5.fill(0, 0, 200);
  myp5.vertex(-128, 128);
  myp5.endShape(myp5.CLOSE);

  assert.equal(renderer._useVertexColor, true);
  assert.deepEqual(myp5.get(128, 128), [100, 0, 100, 255]);
  done();
});

test('immediate mode uses vertex colors (light)', function(done) {
  const renderer = myp5.createCanvas(256, 256, myp5.WEBGL);

  myp5.directionalLight(255, 255, 255, 0, 0, -1);
  // diffuseFactor:0.73
  // so, expected color is (73, 0, 73, 255).

  myp5.beginShape();
  myp5.fill(200, 0, 0);
  myp5.vertex(-128, -128);
  myp5.fill(200, 0, 0);
  myp5.vertex(128, -128);
  myp5.fill(0, 0, 200);
  myp5.vertex(128, 128);
  myp5.fill(0, 0, 200);
  myp5.vertex(-128, 128);
  myp5.endShape(myp5.CLOSE);

  assert.equal(renderer._useVertexColor, true);
  assert.deepEqual(myp5.get(128, 128), [73, 0, 73, 255]);
  done();
});

Second, if you don't use vertex colors in retained mode, make sure _useVertexColor is off.

test('geom without vertex colors use curFillCol (noLight)', function(done) {
  const renderer = myp5.createCanvas(256, 256, myp5.WEBGL);

  // expected center color is curFillColor.

  myp5.fill(200, 0, 200);
  myp5.rectMode(myp5.CENTER);
  myp5.rect(0, 0, myp5.width, myp5.height);

  assert.equal(renderer._useVertexColor, false);
  assert.deepEqual(myp5.get(128, 128), [200, 0, 200, 255]);
  done();
});

test('geom without vertex colors use curFillCol (light)', function(done) {
  const renderer = myp5.createCanvas(256, 256, myp5.WEBGL);

  myp5.directionalLight(255, 255, 255, 0, 0, -1);
  // diffuseFactor:0.73
  // so, expected color is (146, 0, 146, 255).

  myp5.fill(200, 0, 200);
  myp5.rectMode(myp5.CENTER);
  myp5.rect(0, 0, myp5.width, myp5.height);

  assert.equal(renderer._useVertexColor, false);
  assert.deepEqual(myp5.get(128, 128), [146, 0, 146, 255]);
  done();
});

Finally, Make sure vertex colors are reflected in the drawing when enabled in retained mode (even if the lighting shader is working).

test('geom with vertex colors use their color (noLight)', function(done) {
  const renderer = myp5.createCanvas(256, 256, myp5.WEBGL);

  // upper color: (200, 0, 0, 255);
  // lower color: (0, 0, 200, 255);
  // expected center color: (100, 0, 100, 255);

  const myGeom = new p5.Geometry(1, 1, function() {
    this.gid = 'vertexColorTestWithNoLights';
    this.vertices.push(myp5.createVector(-128, -128));
    this.vertices.push(myp5.createVector(128, -128));
    this.vertices.push(myp5.createVector(128, 128));
    this.vertices.push(myp5.createVector(-128, 128));
    this.faces.push([0, 1, 2]);
    this.faces.push([0, 2, 3]);
    this.vertexColors.push(
      200/255, 0, 0, 1,
      200/255, 0, 0, 1,
      0, 0, 200/255, 1,
      0, 0, 200/255, 1
    );
    this.computeNormals();
  });

  myp5.noStroke();
  myp5.model(myGeom);

  assert.equal(renderer._useVertexColor, true);
  assert.deepEqual(myp5.get(128, 128), [100, 0, 100, 255]);
  done();
});

test('geom with vertex colors use their color (light)', function(done) {
  const renderer = myp5.createCanvas(256, 256, myp5.WEBGL);

  const myGeom = new p5.Geometry(1, 1, function() {
    this.gid = 'vertexColorTestWithLighs';
    this.vertices.push(myp5.createVector(-128, -128));
    this.vertices.push(myp5.createVector(128, -128));
    this.vertices.push(myp5.createVector(128, 128));
    this.vertices.push(myp5.createVector(-128, 128));
    this.faces.push([0, 1, 2]);
    this.faces.push([0, 2, 3]);
    this.vertexColors.push(
      200/255, 0, 0, 1,
      200/255, 0, 0, 1,
      0, 0, 200/255, 1,
      0, 0, 200/255, 1
    );
    this.computeNormals();
  });

  myp5.directionalLight(255, 255, 255, 0, 0, -1);
  // diffuseFactor:0.73
  // so, expected color is (73, 0, 73, 255).
  myp5.noStroke();
  myp5.model(myGeom);

  assert.equal(renderer._useVertexColor, true);
  assert.deepEqual(myp5.get(128, 128), [73, 0, 73, 255]);
  done();
});

However, even if I commit now, I will not be able to pass the checks, so I would like to wait for the situation to be resolved.

[Qianqianye]

Thank you both @inaridarkfox4231 and @davepagurek. I restarted the check process, and it's all passed. Ready to merge this PR if it looks good to you both.

[inaridarkfox4231]

@Qianqianye Thank you for responding! Since the unit test is not yet done, I think that it will be merged after adding it.

[inaridarkfox4231]

There are many small mistakes...sorry.

[davepagurek]

The code looks great! Seems like a very natural extension of the current system, and with detailed tests. Great work!

[inaridarkfox4231]

Thanks very much! ('ω') I'm glad I did my best.