Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix #10656 - PolygonClipping: CLIPLINE produces NaN when line to clip is parallel to the maxX of the clip window #10657

Merged
merged 4 commits into from
Aug 14, 2024
Merged

Fix #10656 - PolygonClipping: CLIPLINE produces NaN when line to clip is parallel to the maxX of the clip window #10657

merged 4 commits into from
Aug 14, 2024

Commits on Aug 12, 2024

  1. Expose CLIPLINE and write a simple unit test that shows the NaN occur… 

    …ring
    @jmarrec
    jmarrec committed Aug 12, 2024
    Configuration menu
    Copy the full SHA
    21527bb View commit details
    Browse the repository at this point in the history

  2. Complete unit test for CLIPLINE

    @jmarrec
    jmarrec committed Aug 12, 2024
    4 Configuration menu
    Copy the full SHA
    55edc57 View commit details
    Browse the repository at this point in the history

  3. Fix #10656 - Rewrite the CLIPLINE algorithm 

    Use the Liang-Barsky algorithm

* Loosely adapted from https://en.wikipedia.org/wiki/Liang%E2%80%93Barsky_algorithm
* Rewrote it until performance is optimal

I used microbenchmarking: https://github.com/jmarrec/CppBenchmarks/blob/94f0e1594ff17f9833a0547f0725d68c561311a1/bench_clipline.cpp

BM_CLIPLINE is the current E+ algorithm. BM_liang_barsky_clipper3 is my final version that I used here, and which is 15% faster than the original while being actually correct. in the edge cases

```
-------------------------------------------------------------------
Benchmark                         Time             CPU   Iterations
-------------------------------------------------------------------
BM_CLIPLINE                     567 ns          566 ns      1231826
BM_Clip2                        703 ns          703 ns       991672
BM_liang_barsky_clipper         588 ns          588 ns      1199750
BM_liang_barsky_clipper2        490 ns          490 ns      1430600
BM_liang_barsky_clipper3        482 ns          482 ns      145236
```
    @jmarrec
    jmarrec committed Aug 12, 2024
    4 Configuration menu
    Copy the full SHA
    251ea56 View commit details
    Browse the repository at this point in the history

  4. Forgot to handle the rev case when I removed it as a param 

    Note: in the regular usage, we do nothing is visible is false, and we pass copies into the CLIPLINE method anyways, so there's no point reversing it back
    @jmarrec
    jmarrec committed Aug 12, 2024
    5 Configuration menu
    Copy the full SHA
    955463a View commit details
    Browse the repository at this point in the history