Modify the shader so that the line can be colored per vertex #5915

inaridarkfox4231 · 2022-12-22T01:46:29Z

Currently, calling the stroke function per vertex in immediateMode does not cause the line color to be tinted per vertex.

These changes are meant to make that possible.

Resolves #5916

Changes:

First, modify line.vert and line.frag to prepare an attribute variable to receive the color of each vertex, and pass it from vert to frag with varying.
In addition, prepare a flag for whether or not to use it as a color. This is to coexist with conventional coloring methods.
We also prepare a new buffer to store the color of each vertex in stroke drawing.
And in immediateMode, by calling the stroke function just before the vertex function, you can store the line color for each vertex.
By processing this in the _edgesToVertices function, the vertices and colors will correspond properly.

Screenshots of the change:

sample code:

function setup(){
  createCanvas(400, 400, WEBGL);
  background(0);
  strokeWeight(3);

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

left: before, right:after

Furthermore, even in retainedMode, if you build your own using p5.Geometry, you can color the line for each vertex.

sample code:

function setup(){
  createCanvas(400, 400, WEBGL);
  strokeWeight(3);

  const _gl = this._renderer;
  const geom = new p5.Geometry();
  geom.vertices = [createVector(-100, 0, 0), createVector(100, 0, 0)];
  geom.lineVertexColors = [1, 1, 1, 1, 0, 1, 0, 1];
  geom.edges = [[0, 1]];
  geom._edgesToVertices();
  _gl.createBuffers("customLine", geom);

  _gl.drawBuffers("customLine");
}

PR Checklist

WebGL

Add a new attribute variable "aVertexColor" that represents the vertex color. At the same time, prepare a varying variable "vColor" to send to the fragmentshader. This shader assigns aVertexColor to vColor.

Prepare a variable called uUseLineColor that indicates whether to change the line color for each vertex. At the same time, prepare vColor, which is a common variable with vertexShader, and use this flag to determine which one to use with uMaterialColor.

Removed unnecessary lines at the end of file, sorry.

Prepare a buffer for storing information for coloring lines in both retained and immediate modes. In addition, prepare a new flag for whether or not to color the line for each vertex, and add a process to register it with setUniform.

In the vertex function, register the color used to color the line for each vertex as an argument of the stroke function. In addition, add a process to calculate the flag whether to color the line for each vertex in the _drawImmediateStroke function.

Added a process to calculate the flag whether to color the line for each vertex. Although it cannot be used when drawing primitives, we prepare a process to calculate the flag even in retainedMode so that it can be used when constructing your own using p5.Geometry.

In the _edgesToVertices function, we process the array that stores the data for coloring the lines and synchronizes it with the array used to draw the lines. Also, where p5.Geometry is prepared, prepare a new array to store the data used to color the lines.

There was a line with a long number of characters, so I shortened it with a line break.

Irregular whitespace existed, so delete it

davepagurek · 2022-12-22T15:16:06Z

Hi, thanks for making this! For organizational purposes, we generally want PRs to have an associated issue, but the issue linked has more to do with a renamed method than a request for per vertex colors for strokes. Would you mind making a new issue for this feature request? We can discuss there what the feature should entail, and then that way we can review this PR with respect to the feature in the issue. Thanks! 🙂

inaridarkfox4231 · 2022-12-22T15:27:34Z

Thank you for your reply! I'm sorry, it's certainly better to have an issue that follows the content...
I'd like to make a issue as soon as possible. Please wait...

davepagurek · 2022-12-22T15:42:10Z

Oops just realized I accidentally typed a sad face instead of a smiley one, I must need new glasses! sorry if that sent the wrong tone, it's all good, no rush!

inaridarkfox4231 · 2022-12-22T16:08:47Z

fine! At the end of that issue, there was a little bit of content related to my claim, so it just seemed like there was no problem...
For the time being, I raised an issue. so I changed the issue number.

davepagurek

Thanks for taking this on, great work! I left a question and some comments, but all very minor things 🙂

davepagurek · 2022-12-23T15:29:30Z

src/webgl/p5.Geometry.js

@@ -238,12 +240,16 @@ p5.Geometry.prototype._makeTriangleEdges = function() {
 * @chainable
 */
 p5.Geometry.prototype._edgesToVertices = function() {
+  const data = this.lineVertexColors.slice();


Do we need to make a copy of lineVertexColors or does it not work if we reference it directly? If we do need to copy it, maybe we can call this something more descriptive, like lineColorData

Here, lineVertices are newly created from vertices, but since the color arrangement of lineVertexColors corresponds to vertices, it does not correspond to lineVertices as it is.
So, after making a copy of lineVertexColors corresponding to vertices in advance, we use that data to create an array of colors corresponding to lineVertices. So I thought I should make a copy.
As for the name, lineColorData is certainly easier to understand, so I would like to do so!

It makes sense that lineVertexColors is built using info in lineColorData to correspond to the data in lineVertices. Does the code work if we do const lineColorData = this.lineVertexColors;? So, replacing this.lineVertexColors.slice() with lineVertexColors? Having a separate variable for it is fine if that communicates the intention of the variable more clearly, but I'm just trying to avoid the performance cost of doing an extra copy if we can.

After thinking about it overnight, I realized I didn't need to make a copy, so I'd like to rewrite it! I'm sorry...

No need to apologize! that's what code review is for :D

davepagurek · 2022-12-23T15:30:22Z

src/webgl/p5.Geometry.js

  this.lineVertices.length = 0;
  this.lineNormals.length = 0;

  for (let i = 0; i < this.edges.length; i++) {
-    const begin = this.vertices[this.edges[i][0]];
-    const end = this.vertices[this.edges[i][1]];
+    const e0 = this.edges[i][0];


It's a little more verbose but maybe we can call these endIndex0 and endIndex1 to be clearer what they represent

It's certainly easier to understand... I think it's better to read and understand even if it's verbose, so I'm going to do it.

davepagurek · 2022-12-23T15:30:57Z

src/webgl/p5.Geometry.js

@@ -260,6 +266,13 @@ p5.Geometry.prototype._edgesToVertices = function() {
    dirSub.push(-1);
    this.lineNormals.push(dirAdd, dirSub, dirAdd, dirAdd, dirSub, dirSub);
    this.lineVertices.push(a, b, c, c, b, d);
+    if(data.length > 0){


Super minor, but do you mind adding spaces around the brackets to match the style of the if statements elsewhere?

It sure looks better with some space in it... ok!

davepagurek · 2022-12-23T15:34:38Z

src/webgl/p5.RendererGL.js

@@ -1257,6 +1261,7 @@ p5.RendererGL.prototype._setStrokeUniforms = function(strokeShader) {
  strokeShader.bindShader();

  // set the uniform values
+  strokeShader.setUniform('uUseLineColor', this._useLineColor);


thanks!('ω')

davepagurek · 2022-12-23T15:40:45Z

src/webgl/shaders/line.frag

We can maybe avoid doing this in the fragment shader if, in the vertex shader, we set vColor as either uUseLineColor ? aVertexColor : uMaterialColor. This is a pretty insignificant optimization, but there are usually more fragments than vertices, so it's marginally faster to do if statements and such in the vertex shader and do less checks in the fragment shader (but again, it's just one if statement, so this is more for "best practices" than for a measurable gain)

I see, the vertexShader receives uMaterialColor and writes the branch processing...certainly, considering the number of calls, that might be better...Okay, I'll give it a try!

davepagurek · 2022-12-23T15:45:35Z

I also mentioned in the issue but we should also make a p5 editor sketch that draws a bunch of lines so that we can measure performance before and after to double check that things still run quickly.

I'm excited to use this feature! Once it's in I'll update this PR #5802 to also include this code too.

Changed some variable names to make it easier to understand what they are doing.

The line length was over, so I prepared a variable and shortened it a little.

I added a space because the indent was missing.

I didn't change the name completely when changing data to lineColorData, so I fixed it. I'm sorry...

I decided to do the branch processing in the vertexShader, so I changed it so that it only receives vColor.

Make the vertexShader receive uUseLineColor and uMaterialColor, and let the value of uUseLineColor determine either aVertexColor or uMaterialColor as the vColor.

Create a new array, assign it to the array, and replace it at the end. This eliminates the need to make copies. Also, I decided not to use offset0 and offset1 because I didn't think it was necessary to prepare extra variables.

inaridarkfox4231 · 2022-12-24T01:19:51Z

Is it possible to do conflict resolution like this...?

precision mediump float;
precision mediump int;

varying vec4 vColor;

void main() {
  gl_FragColor = vec4(vColor.rgb, 1.) * vColor.a;
}

I've never resolved a conflict before, so I don't know...

davepagurek · 2022-12-24T04:46:26Z

That code looks correct!

inaridarkfox4231 · 2022-12-24T06:42:03Z

thanks for reply! I'm at work right now, so I'll try to work on it when I get home.

A semicolon was missing at the end of the line, so I added it.

davepagurek · 2022-12-24T17:33:24Z

Awesome, this code looks good to go! The last thing we should maybe consider adding, to make sure future changes don't accidentally break this feature, is maybe a simple unit test in test/unit/webgl/p5.RendererGL.js. Maybe something that draws a line from one color to another, and asserts that the pixels on either end are the right colors, and maybe also tests one pixel in the middle to make sure it's around halfway between?

Anyway, no rush on this! Thanks for all your work so far and happy holidays 🙂

inaridarkfox4231 · 2022-12-24T23:56:20Z

I've never made a unit test before, but I'll try my best...! Thank you!

davepagurek · 2022-12-25T00:14:48Z

Let me know if you need any pointers on how to get started or anything!

inaridarkfox4231 · 2022-12-25T00:30:43Z

I would like to ask you a question!
When I looked at the test of p5.RenderGL.js, I felt that the next test on line 912 was similar to what I'm implementing now.

test('TESS interpolates vertex data at intersections', function(done){

Should I create a new test for the beginShape function similar to this and add it? Or should I add a process to call stroke to this test and modify it so that it can be confirmed that the line color is also interpolated...?

inaridarkfox4231 · 2022-12-25T01:05:11Z

I feel like I should make a new one, but what kind of function should I use to get the color...?

inaridarkfox4231 · 2022-12-25T02:28:00Z

I misunderstood how to use the get function... Now I can get the color! I tried to make it as a trial, but is it okay to feel like this...?
sample code

test('line color interpolation test', function(done) {
  const renderer = myp5.createCanvas(10, 10, myp5.WEBGL);

  renderer.beginShape();
  renderer.stroke(255, 255, 0);
  renderer.vertex(-5, 0);
  renderer.stroke(0,0,255);
  renderer.vertex(5, 0);
  renderer.endShape();
  
  assert.deepEqual(myp5.get(0, 5), [121, 121, 6, 255]);
  assert.deepEqual(myp5.get(5, 5), [57, 57, 70, 255]);
  assert.deepEqual(myp5.get(9, 5), [6, 6, 121, 255]);

  done();
});

davepagurek · 2022-12-25T03:23:46Z

That's the right idea, that test looks like it's most of the way there! My comments so far would be:

is it possible to get the colors on the far left and far right to match the colors you inputted? you may have to use an expect(...).to.be.closeTo(...) assertion on each channel rather than deepEqual in case there's any antialiasing blending colours, and maybe also increase the stroke thickness in case thin strokes are also causing more antialiasing to affect the colors
the test name could maybe be phrased to state what we expect line interpolation to do, like "strokes should interpolate colors between vertices" or something in that style

inaridarkfox4231 · 2022-12-25T14:30:27Z

How about this...?

sample code

test('stroke() should interpolates the line color for each vertex', function(done) {
  const renderer = myp5.createCanvas(512, 4, myp5.WEBGL);

  // far left color: (242, 236, 40)
  // far right color: (42, 36, 240)
  // expected middle color: (142, 136, 140)

  renderer.strokeWeight(4);
  renderer.beginShape();
  renderer.stroke(242, 236, 40);
  renderer.vertex(-256, 0);
  renderer.stroke(42, 36, 240);
  renderer.vertex(256, 0);
  renderer.endShape();
  
  assert.deepEqual(myp5.get(0, 2), [242, 236, 40, 255]);
  assert.deepEqual(myp5.get(256, 2), [142, 136, 140, 255]);
  assert.deepEqual(myp5.get(511, 2), [42, 36, 240, 255]);

  done();
});

When I lengthened the width, the expected value was output, so I set it to deepEqual. I also made the line thicker so that it would not be affected by anti-aliasing.

davepagurek · 2022-12-27T18:04:45Z

Nice, that looks great!

inaridarkfox4231 · 2022-12-27T22:13:07Z

thank you for your reply! Then, I will add it to the test list of beginShape() in WEBGL mode.

Added a unit test to make sure that line colors are registered per vertex and interpolated properly when calling the stroke function just before the vertex function when drawing in immediateMode.

davepagurek

Great work on this!

inaridarkfox4231 · 2022-12-28T03:17:59Z

Thank you for all your help('ω')! I'm happy because it's the feature I wanted, thank you.

inaridarkfox4231 added 9 commits December 22, 2022 02:33

Prepare a color variable to send to the frag.

8c0b790

Add a new attribute variable "aVertexColor" that represents the vertex color. At the same time, prepare a varying variable "vColor" to send to the fragmentshader. This shader assigns aVertexColor to vColor.

Removed unnecessary lines at the end of file.

6522a55

Removed unnecessary lines at the end of file, sorry.

Prepare a buffer to use for coloring lines

c200fba

Prepare a buffer for storing information for coloring lines in both retained and immediate modes. In addition, prepare a new flag for whether or not to color the line for each vertex, and add a process to register it with setUniform.

Shorten lines that are too long

c54b7a9

There was a line with a long number of characters, so I shortened it with a line break.

delete Irregular whitespace

e951bcb

Irregular whitespace existed, so delete it

inaridarkfox4231 changed the title ~~Specify line coloring per vertex so that colors are interpolated~~ Modify the shader so that the line can be colored per vertex Dec 22, 2022

davepagurek reviewed Dec 23, 2022

View reviewed changes

inaridarkfox4231 added 7 commits December 24, 2022 02:02

Changed the variable name

86a99f8

Changed some variable names to make it easier to understand what they are doing.

Some lines have been shortened slightly.

2c851c3

The line length was over, so I prepared a variable and shortened it a little.

add indent

4d6178d

I added a space because the indent was missing.

Fixed because I forgot to change the name

a04d12e

I didn't change the name completely when changing data to lineColorData, so I fixed it. I'm sorry...

Changed to receive only vColor

a570ddf

I decided to do the branch processing in the vertexShader, so I changed it so that it only receives vColor.

Separate vColor values by flags

42f6608

Make the vertexShader receive uUseLineColor and uMaterialColor, and let the value of uUseLineColor determine either aVertexColor or uMaterialColor as the vColor.

Fix it so that you don't have to make a copy.

7d977e3

Create a new array, assign it to the array, and replace it at the end. This eliminates the need to make copies. Also, I decided not to use offset0 and offset1 because I didn't think it was necessary to prepare extra variables.

inaridarkfox4231 added 2 commits December 24, 2022 18:09

Merge branch 'main' into main

38db825

I fixed the missing semicolon.

aea54f2

A semicolon was missing at the end of the line, so I added it.

Added unit test for line color interpolation

a460068

Added a unit test to make sure that line colors are registered per vertex and interpolated properly when calling the stroke function just before the vertex function when drawing in immediateMode.

davepagurek approved these changes Dec 28, 2022

View reviewed changes

davepagurek merged commit b4f1320 into processing:main Dec 28, 2022

This was referenced Dec 30, 2022

Stroke cap+join support for WebGL #5802

Merged

WebGL stroke color interpolation breaks strokes when using QUADS or retained geometry #5926

Closed

davepagurek mentioned this pull request Jan 25, 2023

Mobile performance #5975

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify the shader so that the line can be colored per vertex #5915

Modify the shader so that the line can be colored per vertex #5915

inaridarkfox4231 commented Dec 22, 2022 •

edited

Loading

davepagurek commented Dec 22, 2022 •

edited

Loading

inaridarkfox4231 commented Dec 22, 2022

davepagurek commented Dec 22, 2022

inaridarkfox4231 commented Dec 22, 2022 •

edited

Loading

davepagurek left a comment

davepagurek Dec 23, 2022

inaridarkfox4231 Dec 23, 2022

davepagurek Dec 23, 2022

inaridarkfox4231 Dec 24, 2022

davepagurek Dec 24, 2022

davepagurek Dec 23, 2022

inaridarkfox4231 Dec 23, 2022

davepagurek Dec 23, 2022

inaridarkfox4231 Dec 23, 2022

davepagurek Dec 23, 2022

inaridarkfox4231 Dec 23, 2022

davepagurek Dec 23, 2022

inaridarkfox4231 Dec 23, 2022

davepagurek commented Dec 23, 2022

inaridarkfox4231 commented Dec 24, 2022

davepagurek commented Dec 24, 2022

inaridarkfox4231 commented Dec 24, 2022

davepagurek commented Dec 24, 2022 •

edited

Loading

inaridarkfox4231 commented Dec 24, 2022

davepagurek commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

davepagurek commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

davepagurek commented Dec 27, 2022

inaridarkfox4231 commented Dec 27, 2022

davepagurek left a comment

inaridarkfox4231 commented Dec 28, 2022

Modify the shader so that the line can be colored per vertex #5915

Modify the shader so that the line can be colored per vertex #5915

Conversation

inaridarkfox4231 commented Dec 22, 2022 • edited Loading

PR Checklist

davepagurek commented Dec 22, 2022 • edited Loading

inaridarkfox4231 commented Dec 22, 2022

davepagurek commented Dec 22, 2022

inaridarkfox4231 commented Dec 22, 2022 • edited Loading

davepagurek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davepagurek commented Dec 23, 2022

inaridarkfox4231 commented Dec 24, 2022

davepagurek commented Dec 24, 2022

inaridarkfox4231 commented Dec 24, 2022

davepagurek commented Dec 24, 2022 • edited Loading

inaridarkfox4231 commented Dec 24, 2022

davepagurek commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

davepagurek commented Dec 25, 2022

inaridarkfox4231 commented Dec 25, 2022

davepagurek commented Dec 27, 2022

inaridarkfox4231 commented Dec 27, 2022

davepagurek left a comment

Choose a reason for hiding this comment

inaridarkfox4231 commented Dec 28, 2022

inaridarkfox4231 commented Dec 22, 2022 •

edited

Loading

davepagurek commented Dec 22, 2022 •

edited

Loading

inaridarkfox4231 commented Dec 22, 2022 •

edited

Loading

davepagurek commented Dec 24, 2022 •

edited

Loading