Skip to content

Circle2D intersection with Line(Segment)2D - #237

Merged
jkalias merged 25 commits into
mathnet:masterfrom
f-frhs:Circle2D_Intersection
Dec 9, 2023
Merged

Circle2D intersection with Line(Segment)2D#237
jkalias merged 25 commits into
mathnet:masterfrom
f-frhs:Circle2D_Intersection

Conversation

@f-frhs

@f-frhsf-frhs commented Nov 25, 2023

Copy link
Copy Markdown
Contributor

I implemented the code to calculate the intersection of the circle with the line, specifically,

  • intersection of Circle2D with Line2D
  • intersection of Circle2D with LineSegment2D

It will be nice if we can discuss whether the testcases are sufficient or not.

this task is based on the comment in #186:

I think it would be useful to have additional methods, like

  • intersection of sphere with line/ray
  • intersection of sphere with plane
  • intersection of sphere with another sphere
  • functions which check if a point lies inside/on/outside of the circle

Originally posted by @jkalias in #186 (comment)

Comment threadsrc/Spatial/Euclidean/Circle2D.cs Outdated
Comment threadsrc/Spatial/Euclidean/Circle2D.cs
Comment threadsrc/Spatial/Euclidean/Circle2D.cs Outdated
Comment threadsrc/Spatial/Euclidean/Circle2D.cs Outdated
Comment threadsrc/Spatial/Euclidean/Line2D.cs Outdated
Comment threadsrc/Spatial/Euclidean/Line2D.cs Outdated
Comment threadsrc/Spatial/Extensions/DoubleExtensions.cs Outdated
Comment threadsrc/Spatial/Euclidean/Circle2D.cs Outdated
Comment threadsrc/Spatial/Euclidean/Circle2D.cs Outdated
Comment threadsrc/Spatial.Tests/Euclidean/Circle2DTests.cs Outdated
Comment threadsrc/Spatial.Tests/Euclidean/Circle2DTests.cs Outdated

@jkaliasjkalias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Naming

Comment threadsrc/Spatial/Euclidean/Circle2D.cs Outdated
var d = line.Direction;
var r = this.Radius;

var a = 1d;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

var a = 1.0;

@f-frhsf-frhsDec 3, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Could you tell me what the difference is between var a = 1.0 and var a = 1d?
If this repo has coding conventions, I failed to read the doc. I will read it and fix this.

Thanks for the comment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel that var a = 1d; is a bit like C of Fortran, that's all. I wouldn't reject a PR based on this.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks for the comment.
I respect your feeling, and I will fix this.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

done in the commit f-frhs@0e7eb69

Comment threadsrc/Spatial/Euclidean/Circle2D.cs Outdated
Comment threadsrc/Spatial.Tests/Euclidean/Circle2DTests.cs Outdated
Comment threadsrc/Spatial.Tests/Euclidean/Circle2DTests.cs Outdated
Comment threadsrc/Spatial.Tests/Euclidean/Circle2DTests.cs Outdated
Comment threadsrc/Spatial.Tests/Euclidean/Circle2DTests.cs Outdated
- parallel to X-axis
- parallel to Y-axis
- general cases (eg. x-y+c=0)
test: rename method's name
@f-frhs
f-frhsforce-pushed the Circle2D_Intersection branch from 4081b03 to f3815edCompareDecember 7, 2023 09:45
@f-frhs
f-frhs requested a review from jkaliasDecember 9, 2023 10:54
@f-frhs

Copy link
Copy Markdown
ContributorAuthor

Could you tell me which requrest remains?
I lost myself in this PR.

@jkalias
jkalias merged commit 9a40b9f into mathnet:masterDec 9, 2023
@jkalias

Copy link
Copy Markdown
Member

Thank you for your effort and time!

@f-frhs

Copy link
Copy Markdown
ContributorAuthor

Thank YOU for your patience.
It was a nice time for me.

@f-frhs
f-frhs deleted the Circle2D_Intersection branch December 25, 2023 15:47
Sign up for freeto 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

@f-frhs@jkalias