Closed GoogleCodeExporter closed 8 years ago
Merged. Committed in revision 1346. Will be released in version 2.1.5.
Found a bug in LineSegment.GetIntersectionWith( LineSegment other ) ...
Here is simple test to find it:
[Row( 0, 0, 6, 0, 5, 1, 5, 5, 5, 0, IntersectionType.SegmentA )]
For this two segments there is not intesection at all. However, here is what
you do. You call result = GetIntersectionWith( other.line ), so you find
intersection between a segment and a line. Then you check if the intesection is
on the first segment. But you never check the second segment – you simply
exteneded it to line without checking if the result belongs to it.
Since you trasnfer the call to LineSegment.GetIntersectionWith( Line other ),
the check is actually useless, since same check is done in that method also. So
in the segment-segment version you better check second segment, but not repeat
check for the first segment again.
Some comments:
1) The difference between parallel Line-Line intersection with other variants
is a bit awkward. Why two parallel lines cannot have NULL as intersection
point? Just not sure about the idea behind ... if parallel line-line
intersection, then it is not valid and we throw exception. But if parallel
segment-line, line-segment or segment-segment, then it is OK, just no
intersection.
2) In my opinion it is a bit unusual to call intersection the case, when two
collinear segments have only one common point. Not really sure it is an
intersection from geometry point of view. In my opinion all intersecting
lines/segment should have different slope. Such implementation may be useful in
some cases, but to me it looks like a hack.
What are going to do with #163? Just close it?
Original comment by andrew.k...@gmail.com
on 16 Nov 2010 at 4:41
I've put the Pending status just for any further discussion/patch. Will not
revert changes at this point. Just would like to clarify things, if possible.
Original comment by andrew.k...@gmail.com
on 16 Nov 2010 at 4:42
> [Bug]
*headscratch*
Ah. Yes. And I see you've fixed it too; thanks.
1) Mostly because I did that before thinking too hard about intersections
involving line segments. It wouldn't be hard to make
Line.GetIntersectionWith(Line) return null for parallel and throw on identical.
(I've gone ahead and attached a patch.)
2) I'm not sure. It may not be an "intersection", but it's definitely the
single common point. I believe Wolfram
(http://mathworld.wolfram.com/Intersection.html) says the intersection is the
set of all common points; we're restricting that to the single common point. In
the absence of a use case that says one way or another, I went for (what I
think is) geometrically accurate.
> [Bug 163]
That was what I was thinking, maybe with a note that what was
Point.DistanceToSegment exists here now?
Original comment by dales...@gmail.com
on 23 Nov 2010 at 7:15
Attachments:
Done, merged. I think that is all about it.
Original comment by andrew.k...@gmail.com
on 25 Nov 2010 at 2:02
Original comment by andrew.k...@gmail.com
on 12 Jan 2011 at 11:45
Original issue reported on code.google.com by
dales...@gmail.com
on 16 Nov 2010 at 3:58Attachments: