Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overlap between Line2D and LineSegment2D #242

Open
labsin opened this issue Oct 14, 2024 · 6 comments
Open

Overlap between Line2D and LineSegment2D #242

labsin opened this issue Oct 14, 2024 · 6 comments

Comments

@labsin
Copy link

labsin commented Oct 14, 2024

As a new user, the current state is a bit confusing.

LineSegment2D/3D is clear:
It is the segment between two points

Line2D/3D is unclear:
the class documentation is exactly the same as LineString, with the added "projections to" operations

Looking at Line2D, it has:

  • Start
  • End
  • Length
  • Direction
    And equality is checked by start an.d end
    So it's also a line segment

But IntersectWith, LineTo and ClosestPointTo (optionally) work as infinite lines.

As LineSegment2D is already split off, would it be better to also split off an infinite line and deprecate the Line2D/3D?

@labsin
Copy link
Author

labsin commented Oct 14, 2024

Possibly related to #35

@jkalias
Copy link
Member

jkalias commented Oct 26, 2024

I agree with your comment, that this is a bit messed up.

For my understanding there are cases when one wants to consider a line segment (having a specific start and end), where I would say a line is infinite and independent of direction (eg. if you switch the direction vector of the line, it's still the same line).

I think the core functionality should be part of Line2D and LineSegment2D should be more specific, something along these...lines (pun intended :))

Line2D

  • DistanceFrom(point)
  • Direction
  • Rotate(aroundPoint)
  • Translate(byOffset)
  • Contains(otherPoint)

LineSegment (inherit from Line2D, but have a given Start and End)

  • Length
  • Contains(otherPoint) -> only if otherPoint falls within the segment

@Jones-Adam
Copy link
Contributor

A small bit of history, originally there was only Line2D/3D and they implemented the concept of a line from 1 point to another point, which was inaccurate, so LineSegment2D/3D was born to handle the point to point case and Line2D/3D was intended to be deprecated and eventually replaced with a model for an infinite line to allow the library to work with more pure geometry cases. However for various reasons this never happened.

@jkalias
Copy link
Member

jkalias commented Oct 30, 2024

A small bit of history, originally there was only Line2D/3D and they implemented the concept of a line from 1 point to another point, which was inaccurate, so LineSegment2D/3D was born to handle the point to point case and Line2D/3D was intended to be deprecated and eventually replaced with a model for an infinite line to allow the library to work with more pure geometry cases. However for various reasons this never happened.

Thanks for the info. So if I read this correctly, you would also agree if LineXD covered the infinite case and LineSegmentXD the “start-end” scenario, correct?

@labsin
Copy link
Author

labsin commented Oct 30, 2024

I already thought it was halfway a replacement. I was mostly wondering why it wasn't deprecated cause it took me (a new user) some time to figure out the current state of things and the difference between Line2D and LineSegment2D

I'd definitely not make these classes inherit from each other as I don't think a segment should ever be treated as an infinite line or the other way around. Maybe a LineSegment.AsRay() => Ray(StartPoint, Direction) would be enough for most use cases.

Apparently there is already an infinite 3d line called Ray3D, that you get from intersecting planes, which is a ThroughPoint and a Direction. Apparently there is also no UnitVector2D to do the same for a Ray2D 🤔 The equal checks if both the ThroughPoint and Direction are the same, which might also not be the expected behavior as #35 even implies that the direction orientation shouldn't even matter, let alone the ThroughPoint. Although calling it Ray might imply that the direction matters. But then what is the orientation of the Ray when you do an intersection of Planes? Is it then that doing A.IntersectionWith(B) = C that C.Direction = A.Normal x B.Normal?

Intersection methods are also a bit inconsistent. Some use the bool TryXx(, out result) (LineSegment2D), some a nullable return values (Line2D) and others like Plane.IntersectionWith throw an exception (which I found out the hard way 😆). But that's another issue altogether.

As I'm right now working on cropping and slicing functionality for triangulation, that heavily use the intersections, infinite lines, line segments and projections between 2D and 3D, I'm thinking about forking this library and doing a lot of breaking changes.

@jkalias
Copy link
Member

jkalias commented Nov 2, 2024

As I'm right now working on cropping and slicing functionality for triangulation, that heavily use the intersections, infinite lines, line segments and projections between 2D and 3D, I'm thinking about forking this library and doing a lot of breaking changes.

That might indeed be the best option you have at the moment.

Once I was accepted as a member of the team, I tried to clean up things a bit and keep evolving the library, but it faced considerable backlash from the community (#229)

We could create a separate branch from main, where all these changes are implemented, so anybody has access to them in the future, without having to redo the work, but I need to find the time and primarily the motivation to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants