Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/webgl/ShapeBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,27 @@ export class ShapeBuilder {
}
}

// Normalize nearly identical consecutive vertices to prevent tessellation artifacts
// This addresses numerical precision issues in libtess when consecutive vertices
// have coordinates that are almost (but not exactly) equal (e.g., differing by ~1e-8)
const epsilon = 1e-6;
for (const contour of contours) {
const stride = this.tessyVertexSize;
for (let i = stride; i < contour.length; i += stride) {
const prevX = contour[i - stride];
const prevY = contour[i - stride + 1];
const currX = contour[i];
const currY = contour[i + 1];

if (Math.abs(currX - prevX) < epsilon) {
contour[i] = prevX;
}
if (Math.abs(currY - prevY) < epsilon) {
contour[i + 1] = prevY;
}
}
}

const polyTriangles = this._triangulate(contours);

// If there were no valid faces, we still want to use the original vertices
Expand Down
31 changes: 31 additions & 0 deletions test/unit/visual/cases/webgl.js
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,37 @@ visualSuite('WebGL', function() {
});
});

visualSuite('Tessellation', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new visual test doesn’t actually cover this change. I see the same output before and after the patch. Please update the sketch to reproduce the bug which is fixed at your PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worst case we can just use the exact same tests as in the original issue. I don't fully understand either why this case seems OK but not the one in the issue haha. I wonder if my numbers were bigger in the original and that causes more numerical precision issues

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worst case we can just use the exact same tests as in the original issue. I don't fully understand either why this case seems OK but not the one in the issue haha. I wonder if my numbers were bigger in the original and that causes more numerical precision issues

Hello @perminder-17 @davepagurek thanks for the review

  1. As you said that Visual test not showing the bug
    Should I use the exact textToContours() example from issue [p5.js 2.0 Bug Report]: Tessellation of beginShape/endShape with multiple contours sometimes produces weird results #8186?
    That one clearly shows the glitchy tessellation before the fix. I can add that as the visual test.

  2. Also about Z coordinates
    I included Z because I thought it would be safer to normalize all three axes consistently. But you're right that the original issue only mentions X/Y coordinates. Should I:

    • Test if the issue happens with Z as Dave suggested (rotate the shape)?
    • Or remove Z normalization to keep the fix minimal?

Let me know and I'll update the PR accordingly and will also add similar changes to #8204

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you said that Visual test not showing the bug
Should I use the exact textToContours() example from issue #8186?
That one clearly shows the glitchy tessellation before the fix. I can add that as the visual test.

For this one, maybe yes. you could add that in this scenario.

Test if the issue happens with Z as Dave suggested (rotate the shape)?

for this one, I tested it and I don't think we need to snap z coordinates.

visualTest('Handles nearly identical consecutive vertices from textToContours', async function(p5, screenshot) {
p5.createCanvas(200, 200, p5.WEBGL);
p5.background(255);
p5.fill(0);
p5.noStroke();

const font = await p5.loadFont('/unit/assets/Inconsolata-Bold.ttf');
const contours = font.textToContours('p', 0, 0, 60);

if (contours && contours.length > 0) {
p5.translate(-p5.width / 4, -p5.height / 4);
p5.beginShape();
for (let contourIdx = 0; contourIdx < contours.length; contourIdx++) {
const contour = contours[contourIdx];
if (contourIdx > 0) {
p5.beginContour();
}
for (let i = 0; i < contour.length; i++) {
p5.vertex(contour[i].x, contour[i].y, 0);
Comment on lines +697 to +714
Copy link
Collaborator

@perminder-17 perminder-17 Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still not reproducing the issue for which you're adding the visual tests.

Please use this sketch for the tests:

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

function draw() {
  const contours = [
    [
      [-3.8642425537109375,-6.120738636363637,0],
      [3.2025188099254267,-6.120738636363637,0],
      [3.2025188099254267,-4.345170454545455,0],
      [-3.8642425537109375,-4.345170454545455,0],
      [-3.8642425537109375,-6.120738636363637,0],
    ],

    [
      [-1.8045834628018462,4.177556818181818,0],
      [-1.8045834628018462,-9.387784090909093,0],
      [0.29058699174360836,-9.387784090909093,0],
      [[0.2905869917436083,3.609374411367136,0]] // doesnt work
      [0.31044303036623855,4.068235883781435,0],
      [0.38522861430307975,4.522728865422799,0],
      [0.548044378107245,4.941051136363637,0],
      [0.8364672032828204,5.2932224887960775,0],
      [1.2227602871981542,5.526988636363637,0],
      [1.6572258237923885,5.634502949876295,0],
      [2.101666537198154,5.669034090909091,0],
      [2.6695604948237173,5.633568761673102,0],
      [3.0249619917436084,5.5625,0],
      [3.4510983553799726,7.4446022727272725,0],
      [2.8568950819856695,7.613138889205699,0],
      [2.3751340936529037,7.676962586830456,0],
      [1.8892600236717598,7.693181792704519,0],
      [1.2922705720786674,7.649533731133848,0],
      [0.7080836288276859,7.519788939617751,0],
      [0.14854153719815422,7.311434659090909,0],
      [-0.38643934048179873,7.00959666478984,0],
      [-0.858113258144025,6.61653855366859,0],
      [-1.25415732643821,6.1484375,0],
      [-1.5108595282965422,5.697682732328092,0],
      [-1.6824918355513252,5.207533878495854,0],
      [-1.7762971052870198,4.695933154267308,0],
      [-1.8045834628018462,4.177556818181818,0],
    ]
  ]
  
  background('red')
  push()
  stroke(0)
  fill('#EEE')
  scale(15)
  beginShape()
  for (const contour of contours) {
    beginContour()
    for (const v of contour) {
      vertex(...v)
    }
    endContour()
  }
  endShape()
  
  stroke(0, 255, 0)
  strokeWeight(5)
  beginShape(POINTS)
  for (const contour of contours) {
    for (const v of contour) {
      vertex(...v)
    }
  }
  endShape()
  pop()
}

}
if (contourIdx > 0) {
p5.endContour();
}
}
p5.endShape(p5.CLOSE);
}
screenshot();
});
});

visualSuite('textures in p5.strands', async () => {
visualTest('uniformTexture() works', async (p5, screenshot) => {
p5.createCanvas(50, 50, p5.WEBGL);
Expand Down
Loading