p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

fix(webgl): normalize nearly identical vertices before tessellation

Open avinxshKD opened this issue 2 months ago • 4 comments

Resolves #8186

Changes

This PR fixes tessellation artifacts that occur when drawing shapes with multiple contours where consecutive vertices have nearly identical coordinates (differing by ~1e-8 or similar small amounts). This commonly happens when using font.textToContours().

  • Added visual test in test/unit/visual/cases/webgl.js:
    • New visual test "Tessellation → Handles nearly identical consecutive vertices"
    • Easier for contributors to debug visually when issues arise

Implementation Details

  • Modified _tesselateShape() in src/webgl/p5.RendererGL.Immediate.js:

    • Added coordinate normalization loop before passing vertices to libtess tessellator
    • Consecutive vertices with coordinates within 1e-6 (epsilon) of each other are normalized to use the exact same value
    • Prevents numerical precision issues in libtess that cause glitchy rendering
  • Added comprehensive unit test in test/unit/webgl/p5.RendererGL.js:

    • Tests tessellation with nearly identical consecutive vertices
    • Verifies that contour holes are properly cut out
    • Confirms shape fills work correctly with problematic coordinates

Technical Background

This workaround addresses a numerical precision issue in libtess, which is a JavaScript port of SGI C code from the 1990s. When consecutive vertices have coordinates that are almost (but not exactly) equal, libtess produces glitchy tessellation output. This issue is particularly likely to occur with contours automatically sampled from fonts via font.textToContours().

Screenshots of the change

Before:

  • Tessellation produces visual artifacts and incorrect geometry
  • Referenced in issue #8186

After (with this fix):

  • Clean tessellation with proper contour cutouts
  • No visual artifacts

PR Checklist

  • [x] npm run lint passes
  • [x] Inline reference is included / updated (N/A - internal fix, no API changes)
  • [x] Unit tests are included / updated

avinxshKD avatar Oct 26 '25 18:10 avinxshKD

🎉 Thanks for opening this pull request! For guidance on contributing, check out our contributor guidelines and other resources for contributors! 🤔 Please ensure that your PR links to an issue, which has been approved for work by a maintainer; otherwise, there might already be someone working on it, or still ongoing discussion about implementation. You are welcome to join the discussion in an Issue if you're not sure! 🌸 Once your PR is merged, be sure to add yourself to the list of contributors on the readme page !

Thank You!

welcome[bot] avatar Oct 26 '25 18:10 welcome[bot]

hey @davepagurek, kindly review when you get a chance

avinxshKD avatar Oct 29 '25 06:10 avinxshKD

Hi, thanks for taking this on! ~The approach looks good. I was initially testing in p5 2.x, have we confirmed that this issue exists in 1.x?~ (Update: this exists in 1.x too, so we can keep this PR into main around, but we'll need another one into the dev-2.0 branch for 2.x.) Regardless, it may be a good idea to start by making the change just in 2.x, in the dev-2.0 branch instead of main.

Thanks for the review @davepagurek

I've converted the pixel-checking unit test to a visual test as you suggested.

And also created a separate PR targeting the dev-2.0 branch with the same fixes, since the file structure is different there (ShapeBuilder.js instead of p5.RendererGL.Immediate.js). Here's the link https://github.com/processing/p5.js/pull/8221

Thanks

avinxshKD avatar Nov 04 '25 06:11 avinxshKD

Hey @davepagurek @perminder-17, made the necessary changes. Please review whenever you get a chance.

avinxshKD avatar Nov 24 '25 07:11 avinxshKD