fixed geometry builder to accomodate vertexStrokeColors

[Vaivaswat2244]

Resolves #7840

Changes: Added per-vertex stroke color handling to GeometryBuilder.addGeometry() method, following the same pattern as the existing fill color logic:

• Copies stroke colors from input geometry to built geometry
• Pads missing stroke colors with current renderer stroke state
• Uses [-1, -1, -1, -1] fallback indicator when no stroke color is set
• Maintains 4 values per vertex format (r, g, b, a)

The following screenshot shows the given fixes...

preview/index.html:

    import p5 from '../src/app.js';

    const sketch = function (p) {
      let geom;
      
      p.setup = async function () {
        p.createCanvas(400, 400, p.WEBGL);
        p.angleMode(p.DEGREES);
        
        p.strokeWeight(5);
        p.noFill();
        p.randomSeed(5);
        
        geom = p.buildGeometry(() => {
          // *** remove/replace POINTS and works as expected
          p.beginShape(p.LINES); // LINES or TRIANGLES or empty works fine
          for(let i = 0; i < 50; i++){
            p.push();
            p.stroke(p.random(255), p.random(255), p.random(255));
            let x = p.random(-p.width/2, p.width/2);
            let y = p.random(-p.width/2, p.width/2);
            let z = p.random(-p.width/2, p.width/2);
            p.vertex(x, y, z);
            p.vertex(x, y, z+10);
            p.pop();
          }
          p.endShape();
        });
      };

      p.draw = function () {
        p.background(0);
        p.orbitControl(3);
        p.rotateY(p.frameCount/3);
        
        p.strokeWeight(5);
        // p.stroke(255); // uncomment to show points
        p.model(geom);
      };
    };

    new p5(sketch);

Screenshots of the change:

PR Checklist

• [x] npm run lint passes
• [ ] Inline reference is included / updated
• [ ] Unit tests are included / updated

[Vaivaswat2244]

Hi @davepagurek ! I've been working on implementing changes you suggested, but I'm running into some test failures that I'd like your guidance on.

Changes Made

Following your recommendation, I added the stroke color state management similar to how fill colors are handled:

In p5.RendererGL.js:

// Modified beginGeometry()
beginGeometry() {
 if (this.geometryBuilder) {
   throw new Error(
     "It looks like `beginGeometry()` is being called while another p5.Geometry is already being build."
   );
 }
 this.geometryBuilder = new GeometryBuilder(this);
 this.geometryBuilder.prevFillColor = this.states.fillColor;
 this.geometryBuilder.prevStrokeColor = this.states.strokeColor; // Added this
 this.fill(new Color([-1, -1, -1, -1]));
 this.stroke(new Color([-1, -1, -1, -1])); // Added this
}

// Modified endGeometry() 
endGeometry() {
 // ... existing code ...
 this.fill(this.geometryBuilder.prevFillColor);
 this.stroke(this.geometryBuilder.prevStrokeColor); // Added this
 // ... rest of method
}

For the test sections, I wrote two tests,

visualSuite('buildGeometry stroke colors', () => {
    visualTest('Geometry without stroke colors, global stroke override', (p5, screenshot) => {
      p5.createCanvas(50, 50, p5.WEBGL);
      
      // Build geometry without any stroke() calls inside
      const geom = p5.buildGeometry(() => {
        p5.beginShape();
        p5.vertex(-15, -15, 0);
        p5.vertex(15, -15, 0);
        p5.vertex(15, 15, 0);
        p5.vertex(-15, 15, 0);
        p5.endShape(p5.CLOSE);
      });
      
      p5.background(220);
      p5.stroke('red'); // Should override and make all strokes red
      p5.strokeWeight(2);
      p5.noFill();
      p5.model(geom);
      screenshot();
    });

    visualTest('Geometry with internal stroke colors not overridden', (p5, screenshot) => {
      p5.createCanvas(50, 50, p5.WEBGL);
      
      // Build geometry WITH stroke() calls inside
      const geom = p5.buildGeometry(() => {
        p5.beginShape();
        
        p5.stroke('blue');
        p5.vertex(-15, -15, 0);
        
        p5.stroke('green');
        p5.vertex(15, -15, 0);
        
        p5.stroke('purple');
        p5.vertex(15, 15, 0);
        
        p5.stroke('orange');
        p5.vertex(-15, 15, 0);
        
        p5.endShape(p5.CLOSE);
      });
      
      p5.background(220);
      p5.stroke('red'); // This should NOT override the internal colors
      p5.strokeWeight(2);
      p5.noFill();
      p5.model(geom);
      screenshot();
    });
  });

So now some of the tests are failing after the changes in p5.Renderer file,

Test 1: /buildgeometry()/can draw models

visualTest('can draw models', (p5, screenshot) => {
    p5.createCanvas(50, 50, p5.WEBGL);

    const sphere = p5.buildGeometry(() => {
      p5.scale(0.25);
      p5.sphere();
    });

    const geom = p5.buildGeometry(() => {
      p5.model(sphere);
    });

    p5.background(255);
    p5.lights();
    p5.model(geom);
    screenshot();
  });

This test was passing before the renderer changes

Test 2: Test 2: Geometry without stroke colors, global stroke override

visualTest('Geometry without stroke colors, global stroke override', (p5, screenshot) => {
      p5.createCanvas(50, 50, p5.WEBGL);
      
      // Build geometry without any stroke() calls inside
      const geom = p5.buildGeometry(() => {
        p5.beginShape();
        p5.vertex(-15, -15, 0);
        p5.vertex(15, -15, 0);
        p5.vertex(15, 15, 0);
        p5.vertex(-15, 15, 0);
        p5.endShape(p5.CLOSE);
      });
      
      p5.background(220);
      p5.stroke('red'); // Should override and make all strokes red
      p5.strokeWeight(2);
      p5.noFill();
      p5.model(geom);
      screenshot();
    });

This test passes when I undo the renderer changes

image of diffs of first and second test

Should I commit the changes and tests so you could have a look at them. we can revert them if needed

[davepagurek]

It looks like we also need to update the line shader.

In the fill shader, it checks that the x value of the color is at least 0 as a quick way to see if it's a real color: https://github.com/processing/p5.js/blob/2606c21ef4924bd995053193c1c8b66f3e014d25/src/webgl/shaders/normal.vert#L40

but currently in the stroke shader, it doesn't, it just looks like this: https://github.com/processing/p5.js/blob/2606c21ef4924bd995053193c1c8b66f3e014d25/src/webgl/shaders/line.vert#L102

[davepagurek]

Looks like this test is failing now: https://github.com/processing/p5.js/blob/2606c21ef4924bd995053193c1c8b66f3e014d25/test/unit/webgl/p5.RendererGL.js#L1406-L1429

This test should probably be a visual test since it's hard to understand what's actually failing by looking at a big array of color channel values, but I wonder if the noStroke() is getting overridden. After building geometry, we set stroke() again with the previous color. We probably need to call noStroke() if the previous stroke is null, or stroke(...) otherwise. This is likely also something we have to do with fills but we just didn't notice because the noFill() case isn't tested? Maybe it's worth adding a test for that too.