Closed GoogleCodeExporter closed 8 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:
*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:
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:
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
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
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
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
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:
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
OK, DistanceToPoint() is there.
Original comment by andrew.k...@gmail.com
on 12 Nov 2010 at 11:06
Original comment by andrew.k...@gmail.com
on 12 Nov 2010 at 11:07
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
Original comment by andrew.k...@gmail.com
on 12 Jan 2011 at 11:47
Original issue reported on code.google.com by
dales...@gmail.com
on 12 Oct 2010 at 5:20Attachments: