davideberly / GeometricTools

A collection of source code for computing in the fields of mathematics, geometry, graphics, image analysis and physics.
Boost Software License 1.0
1.08k stars 202 forks source link

bug Arc2 Segment2 intersection #76

Closed owai1980 closed 9 months ago

owai1980 commented 9 months ago

Hello,

I'm investigating a problem in GTE IntrSegment2Arc2, using double precision. I know "double" floating point precision is not the best... but here the arc2 and the segment2 are so far away, there should be no intersection...

Capture d'écran 2023-09-28 140333

these are my 2 objects and your function returns 2 intersections!

image

I am still trying to understand where is the problem (and possibly my fault too...)

so far i found one strange thing in IntrSegment2Arc2.h, line 83: if (arc.Contains(scResult.point[i]))

this function is deprecated and it seems we should use the "epsilon" parameter... this would probably reject the 2 (wrong) intersections found.

but... IMO, the test at line 78 should be "false" and we should not even have to call "arc.Contains(...)" if (scResult.intersect)

I will tell you if i find something interesting...

Johan

owai1980 commented 9 months ago

i think i found why, but i need your advice to make a clean fix:

in IntrLine2Circle2.h, this function finds only 1 intersection (discr == 0). this is correct. but result.parameter[1] is undetermined (i added the selected blue line 114)

Capture d'écran 2023-09-28 163730

later, in "InterSegment2Circle2.h", we pass the result.parameter array without telling how many elements are valid. assuming the result.parameter[1] is a valid intersection. (see the red line)

Capture d'écran 2023-09-28 163816

davideberly commented 9 months ago

Yes, a bug. Great job diagnosing and fixing this! I posted the modified file with check-in comments and update history crediting you for doing this.

I also removed the deprecation comment from Arc2::Contains function. This function is still useful; I do not recall why I deprecated it in the first place.

owai1980 commented 9 months ago

Thank you!

About the deprecated function, I think you did it because the deprecated function is somehow misleading... It doesn't check if the distance (point, center) is equal to the radius. The function only tests the "angle".

Anyway, thank you for your fast reply!

Le jeu. 28 sept. 2023 à 18:16, David Eberly @.***> a écrit :

Yes, a bug. Great job diagnosing and fixing this! I posted the modified file with check-in comments and update history crediting you for doing this.

I also removed the deprecation comment from Arc2::Contains function. This function is still useful; I do not recall why I deprecated it in the first place.

— Reply to this email directly, view it on GitHub https://github.com/davideberly/GeometricTools/issues/76#issuecomment-1739635547, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3BAQPVTUHETPSKF3UCMCLX4WPG5ANCNFSM6AAAAAA5K33TMU . You are receiving this because you authored the thread.Message ID: @.***>