Skip to content

Conversation

memononen
Copy link
Owner

tesedgeSign() does not evaluate to the same sign as tesedgeEval() when the x-coordinates are almost zero. In the test data we had values like -1.70990830e-08f, -8.38190317e-09f, -3.35276112e-10f.

The fix is to always use tesedgeEval() so that we have consistent signs.

This is slight perf degrade, but the code written in '94 tried really hard to avoid one floating point division, and I think we can handle with that today.

@memononen memononen requested a review from sfreilich April 24, 2025 19:07
@memononen
Copy link
Owner Author

This is the proper fix for #22.

I'm not sure why the test fail, though.

@sfreilich
Copy link
Collaborator

FAIL: //:libtess2_test (Segmentation fault)

Seems like that FloatOverflowQuad test-case is crashing. I merged #71, maybe rebase this and see if the tests are okay after that fix.

tesedgeSign() does not evaluate to the same sign as tesedgeEval() when the x-coordinates are almost zero. In the test data we had values like -1.70990830e-08f, -8.38190317e-09f, -3.35276112e-10f.

The fix is to always use tesedgeEval() so that we have consistent signs.

This is slight perf degrade, but the code written in '94 tried really hard to avoid one floating point division, and I think we can handle with that today.
@memononen memononen force-pushed the memononen-omstart22 branch from f2b3e24 to f3c2374 Compare April 24, 2025 21:51
@memononen
Copy link
Owner Author

How did you figure out which test was failing? (I'm github CI newbie)

Looks like rebasing did the trick, thanks!

@sfreilich
Copy link
Collaborator

The logs include the start and result of each test, those are printed in the Bazel output for failing tests (because of --test_output=errors). The logs here end with:

[ RUN      ] Libtess2Test.UnitQuad
[       OK ] Libtess2Test.UnitQuad (0 ms)
[ RUN      ] Libtess2Test.FloatOverflowQuad

That last test case doesn't print its result (OK or FAILED) because the test binary crashes before that. Googletest doesn't have good crash handling by default (probably to minimize dependencies), so it doesn't get a stacktrace on failure. I think there's a way to do that in the open-source build, but I wasn't able to figure it out quickly enough the last time I poked at it (probably requires an Abseil dependency and some build configuration).

@memononen memononen merged commit 15b8790 into master Apr 25, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants