Skip to content

Filtered_kernel: Do_intersect(Tetrahedron_3,Bbox_3)#5767

Merged
lrineau merged 33 commits intoCGAL:masterfrom
afabri:Filtered_kernel-Do_intersect_3-GF
Oct 29, 2021
Merged

Filtered_kernel: Do_intersect(Tetrahedron_3,Bbox_3)#5767
lrineau merged 33 commits intoCGAL:masterfrom
afabri:Filtered_kernel-Do_intersect_3-GF

Conversation

@afabri
Copy link
Member

@afabri afabri commented Jun 7, 2021

Summary of Changes

Currently the intersection test is directly done using interval arithmetic.
This PR does not really add a static filter but for the moment just tests if vertices of the tetrahedron are either all in the bbox or if some are inside and some outside to have an early result.
The need for improvement comes from an AABB tree with tetrahedra as query objects. @cportaneri @palliez

Release Management

return true;
}
}else{
return Base::operator()(t,b);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you want to check the next point before giving up? Or do you consider that if one point has ugly coordinates, it is likely that they all do?
(you could use a loop)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are maybe right. And thanks for reading. Before this commit the first block was slightly different. I also thought maybe a lambda.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand if for one point the coordinates did not fit into a double the chances are high that it is the same for the other points.

I was also wondering if it makes sense to check if a corner of the bbox is inside the tetrahedron.

@afabri afabri added the Speed label Jun 18, 2021
@afabri
Copy link
Member Author

afabri commented Jun 18, 2021

@sloriot you should maybe commit your benchmark concerning the bbox and the static filter, and report the numbers in the description of the PR.

@maxGimeno maxGimeno added this to the 5.4-beta milestone Jun 29, 2021
@maxGimeno maxGimeno self-assigned this Jun 29, 2021
@maxGimeno
Copy link
Contributor

I think this PR is responsible for all the red squares in https://cgal.geometryfactory.com/CGAL/testsuite/results-5.4-Ic-27.shtml.

@afabri
Copy link
Member Author

afabri commented Aug 12, 2021

I think this PR is responsible for all the red squares in https://cgal.geometryfactory.com/CGAL/testsuite/results-5.4-Ic-27.shtml.

I confirm that there is a problem. I can reproduce it locally.

@sloriot
Copy link
Member

sloriot commented Oct 7, 2021

@afabri if you do not make the changes requested please assign someone else so that we can integrate this PR.

@sloriot
Copy link
Member

sloriot commented Oct 28, 2021

Successfully tested in CGAL-5.4-Ic-82

@lrineau
Copy link
Member

lrineau commented Oct 29, 2021

@sloriot You added "Not yet approved", September 20. Is this PR still blocked?

@sloriot sloriot removed the Not yet approved The feature or pull-request has not yet been approved. label Oct 29, 2021
@sloriot
Copy link
Member

sloriot commented Oct 29, 2021

Mael fixed the issue since then.

@lrineau lrineau self-assigned this Oct 29, 2021
@lrineau lrineau added the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Oct 29, 2021
@lrineau lrineau merged commit 50f30df into CGAL:master Oct 29, 2021
@lrineau lrineau removed the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Nov 2, 2021
@lrineau lrineau deleted the Filtered_kernel-Do_intersect_3-GF branch November 2, 2021 10:04
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.

6 participants