Skip to content

Triangulation: Add static filter for 5D and 6D orientation predicate #8862

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

Draft
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

afabri
Copy link
Member

@afabri afabri commented Apr 23, 2025

Summary of Changes

This PR adds a static filter for the orientation test of 5D and 6D points. It is ~20% faster than just interval arithmetic.

Release Management

  • Affected package(s): Triangulation
  • License and copyright ownership: unchanged

@afabri afabri requested a review from mglisse April 23, 2025 13:28
@mglisse
Copy link
Member

mglisse commented Apr 23, 2025

So far it is slower than just interval arithmetic

I tried quickly running the benchmark (on a file generated with the new generator).
With master, I get 11s.
Branch with -DCGAL_NO_STATIC_FILTER_5 (so presumably just the clear/mark change): 10.3s
Branch (with static filter): 8.5s

That doesn't look slower to me 😕

  • The TDS change is exactly what I had in mind
  • orientationC5.h doesn't seem to be used (ah! it was probably used as input to FPG, then it makes sense to keep it somewhere indeed, with a comment to avoid confusing people like me 😉)
  • we could write the static filter directly with the internal interface instead of defining an adapter (I used the adapter approach for 2d and 3d because I wanted to reuse the static filters from Kernel_23 without modifying them), but if you prefer with the intermediate adapter, that's fine with me.

@afabri afabri changed the title Triangulation: Add static filter for 5D orientation predicate Triangulation: Add static filter for 5D and 6D orientation predicate Apr 25, 2025
@MaelRL MaelRL mentioned this pull request Apr 28, 2025
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants