-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix(webgl): normalize nearly identical vertices before tessellation #8221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-2.0
Are you sure you want to change the base?
fix(webgl): normalize nearly identical vertices before tessellation #8221
Conversation
| if (Math.abs(currZ - prevZ) < epsilon) { | ||
| contour[i + 2] = prevZ; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a reason for snapping the z coordinates as well? I don't think we need to do that for z?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually not 100% sure how libtess works internally, but maybe we could update my test sketch from the original issue to like, swap the x and z axes, and then draw it with a 90 degree rotation to see if we notice the same issue happening? If so, we probably need to do this for z too, otherwise maybe we don't
| }); | ||
| }); | ||
|
|
||
| visualSuite('Tessellation', function() { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
-
As you said that Visual test not showing the bug
Should I use the exacttextToContours()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. -
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
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().Modified
_tesselateShape()insrc/webgl/ShapeBuilder.js:1e-6(epsilon) of each other are normalized to use the exact same valueAdded visual test in
test/unit/visual/cases/webgl.js: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:
After (with this fix):
PR Checklist
npm run lintpasses