feynmanrules / aforge

Automatically exported from code.google.com/p/aforge
0 stars 0 forks source link

*Point.DistanceToLine method #163

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Pursuant to my solution to issue 162, I have added IntPoint.DistanceToLine and 
DoublePoint.DistanceToLine methods. A patch is attached.

Although my use case is to call this ~6000 times with only four different 
points, I'm not convinced that a PointsCloud method is appropriate, as my case 
would require something like:
int CountPointsNearLine( List<IntPoint> cloud, IntPoint linePoint1, IntPoint 
linePoint2, int range), which seems too specialized.

IEnumerable<IntPoint> GetPointsNearLine( List<IntPoint> cloud, IntPoint 
linePoint1, IntPoint linePoint2, int range);
would be better, but I'm still not certain this belongs in library code.

Even if more specialized versions are added, I think the low-level 
*Point.DistanceToLine methods should go in too, in case the specialized 
versions don't do what the user wants.

Original issue reported on code.google.com by dales...@gmail.com on 12 Oct 2010 at 5:20

Attachments:

GoogleCodeExporter commented 9 years ago
On further thought, what's more interesting to me is the distance to a line 
*segment*. Also, I decided that, since all the internal calculations are 
doubles anyway, I might as well upcast everything to DoublePoint as soon as 
possible. Also, this reduces both code duplication and the chances of 
unintentional integer math.

Here's a new patch; it supersedes the old one.

Original comment by dales...@gmail.com on 12 Oct 2010 at 9:15

Attachments:

GoogleCodeExporter commented 9 years ago
*rolls eyes* I swear I put that "public" in there. Multiple times.

Original comment by dales...@gmail.com on 12 Oct 2010 at 11:41

Attachments:

GoogleCodeExporter commented 9 years ago
I found more fat-fingering, plus I decided to take the plunge into writing 
tests.
The four new DistanceTo functions pass their corresponding tests, and the tests 
should be easily verified.

Original comment by dales...@gmail.com on 13 Oct 2010 at 4:48

Attachments:

GoogleCodeExporter commented 9 years ago
Although I’ve already merged the fix suggested in #166, I think we need to 
pause here and think a bit about some refactoring. What do you think? ;)

1) The previous fix was in AForge.Math.Geometry, which is fine to me, since it 
is related to lines. But this fix goes to AForge namespace. Seems like we are 
getting some mess – something is here, something is there.
2) We already have quite a bit of functions, related to lines …

What about introducing AForge.Math.Geometry.Line class ? It may have these two 
methods to find distance to point (DoublePoint). It will take also 
GetIntersectionPoint() from #166. Also the GetAngleBetweenLines() (was in 
GeometryTools for some time) may go there.

Here is a bonus a I see (apart from the bonus of grouping functionality) … 
You may create line with something like new Line( DoublePoint p1, DoublePoint 
p2) and pre-calculate things like “k” and “b” (from y=kx + b) in 
constructor. In this case we’ll not need to calculate them again and again as 
we do now in the case if these tool methods are used in some loops for 
processing many points.

As always … just a though. If we decide to skip Line class for now, then I am 
also not sure we need to put the suggested fix into AForge namespace, when we 
already have the rest in AForge.Math.Geometry namespace.

Should I start the Line class and then you will join? Or do you have time/will 
to try it on your own?

Original comment by andrew.k...@gmail.com on 10 Nov 2010 at 10:31

GoogleCodeExporter commented 9 years ago
Introducing Line class in AForge.Math.Geometry, which is supposed to keep 
lines' related methods. For now put there GetAngleBetweenLines() and 
GetIntersectionWith() with some helper properties.

Committed in revision 1336.

So all is left for now is to rewrite suggested in this ticket fix so it will be 
part of Line class, but not DoublePoint structure.

Original comment by andrew.k...@gmail.com on 11 Nov 2010 at 2:12

GoogleCodeExporter commented 9 years ago
The issue I see with the Line class is that it seems like a non-intuitive place 
for storing a function that calculates the distance from a point to a line 
segment.

I'm also voting for construction by factory methods, so we can have both 
Line.FromSlopeIntercept(double,double) and Line.FromRTheta(double,double). This 
makes the above issue especially obvious: we just don't *have* endpoints if the 
user constructed a line this way. (Also, 
Line.FromPointTheta(DoublePoint,double).)

The best solution I can come up with is
class Line { /* everything that deals with lines */ }
class LineSegment : Line { /* everything that deals with line segments */ }
This feels like abuse of the type system, but I can't really say why, and I 
can't come up with any other system that doesn't make the differentiation 
between lines and segments unnecessarily painful. (Either we create a Line 
independent of a Segment, and casts (whether implicit or explicit) get 
potentially expensive, or we create a Line that is sometimes a segment, but 
other times isn't, with no way for the compiler to check.)

I have, therefore, started refactoring to split lines from segments; should I 
submit that as a separate issue, or toss it in with this one?

Or should we consider a different solution?

Original comment by dales...@gmail.com on 11 Nov 2010 at 3:52

GoogleCodeExporter commented 9 years ago
Well, agree with the mess around segment .... Need to think about it.

We still can create Line.DistanceToPoint() method, which is the first part of 
this ticket. Then we need to think about segments.

Maybe we can go the way you suggested ... Have Line class and, let’s say, 
LineSegment. The Line has only K and B and it is endless. The LineSegment has 
start/end points in addition. A bit awkward … since segment is not an endless 
line, so inheritance here is confusing.

Let’s think :)

Original comment by andrew.k...@gmail.com on 11 Nov 2010 at 4:06

GoogleCodeExporter commented 9 years ago
I think I've got it for the Segment/Line relationship. A segment has-a line. 
(Logically, this is *lousy*, but it becomes a undocumented implementation 
detail, instead of something exposed to a user.) Conversions from Segment to 
Line become basically free because the object already exists, and Segment 
doesn't necessarily have to support GetIntersectionWith(), which, like 
DistanceToPoint(), is defined rather differently for Lines than for Segments. 
Also, caching of k and b arrives for free.

Anyway, here's Line.DistanceToPoint(), along with (the same) tests, some 
comment fixups in Line.cs, and a namespace fixup in LineTest.cs.

Original comment by dales...@gmail.com on 11 Nov 2010 at 6:34

Attachments:

GoogleCodeExporter commented 9 years ago
Well, I think we get closer to the idea. What about doing this ... We have a 
LineSegment class, which is independent from Line class. It may use internally 
if needs, but it is implementation detail. Now, if user want do get Line (and 
all the related methods) out of LineSegment, he just calls 
LineSegment.ToLine(), which return Line. It will be documented and it will 
explicit, so no one should be confused. If user needs to this in a loop, then 
he better call ToLine() before loop’s start and then enjoy line’s methods 
inside.

In this case we’ll need to move Start/End/Length properties to LineSegment 
and keep Line only with K and B – endless line. After that we may develop 
these classes further and provide R and Theta for line if needed (radian 
coordinates), etc., etc.

In the meanwhile will merge the DistanceToPoint() ...

Original comment by andrew.k...@gmail.com on 12 Nov 2010 at 10:32

GoogleCodeExporter commented 9 years ago
OK, DistanceToPoint() is there.

Original comment by andrew.k...@gmail.com on 12 Nov 2010 at 11:06

GoogleCodeExporter commented 9 years ago

Original comment by andrew.k...@gmail.com on 12 Nov 2010 at 11:07

GoogleCodeExporter commented 9 years ago
The idea of this ticket was re-thought which lead to new implementation. 
Related tickets are: 166, 174, 175.

Original comment by andrew.k...@gmail.com on 25 Nov 2010 at 2:06

GoogleCodeExporter commented 9 years ago

Original comment by andrew.k...@gmail.com on 12 Jan 2011 at 11:47