Reworked Segment types into their cartesian forms #17404
Conversation
Jondolf
added
C-Usability
Co-authored-by: Alice Cecile <[email protected]>
| let segment = Segment2d::new(point1, point2).centered(); | ||
| let center = (point1 + point2) / 2.; |
There was a problem hiding this comment.
Isn't this centering the line segment at the origin and then translating it back to the original position? I'm pretty sure this should just compute the bounding circle of the actual line segment, passing in the isometry, but not centering anything or applying stuff like Isometry2d::from_translation(center).
There was a problem hiding this comment.
I tried the simple approach but it failed in the example and the tests.
I believe it has to do with the old implementation automatically "centering" the segment even if the given points were not centered, but the center returned from from_points was not centered itself and was just the middle of the given points (if any of this makes sense).
This weird behavior is what caused me to do this weird code to reproduce the original behavior as close as I could.
This could be a case of something being messed up somewhere else, or just me not understanding the math properly or something else idk.
| @@ -359,22 +359,25 @@ impl Primitive3d for Line3d {} | |||
| reflect(Serialize, Deserialize) | |||
| )] | |||
| pub struct Segment3d { | |||
There was a problem hiding this comment.
I won't copy all of my comments from the 2D version, but most of them also apply here in 3D.
There was a problem hiding this comment.
For the things you havn't specificially mentionned for the Segment3d, I have done the translation and rotation changes, tell me if I missed anything else
Co-authored-by: Joona Aalto <[email protected]>
…into cartesian-segments
Co-authored-by: Joona Aalto <[email protected]>
Co-authored-by: Joona Aalto <[email protected]>
Co-authored-by: Joona Aalto <[email protected]>
…into cartesian-segments
Co-authored-by: Joona Aalto <[email protected]>
Co-authored-by: Joona Aalto <[email protected]>
…into cartesian-segments
Co-authored-by: Joona Aalto <[email protected]>
Co-authored-by: Joona Aalto <[email protected]>
|
If I didn't miss anything (unlikely) I should have addressed everything except the stuff I left questions about. |
Objective
Segment2d and Segment3d are currently hard to work with because unlike many other primary shapes, they are bound to the origin.
The objective of this PR is to allow these segments to exist anywhere in cartesian space, making them much more useful in a variety of contexts.
Solution
Reworking the existing segment type's internal fields and methods to allow them to exist anywhere in cartesian space.
I have done both reworks for 2d and 3d segments but I was unsure if I should just have it all here or not so feel free to tell me how I should proceed, for now I have only pushed Segment2d changes.
As I am not a very seasoned contributor, this first implementation is very likely sloppy and will need some additional work from my end, I am open to all criticisms and willing to work to get this to bevy's standards.
Testing
I am not very familiar with the standards of testing. Of course my changes had to pass the thorough existing tests for primitive shapes.
I also checked the gizmo 2d shapes intersection example and everything looked fine.
I did add a few utility methods to the types that have no tests yet. I am willing to implement some if it is deemed necessary
Migration Guide
The segment type constructors changed so if someone previously created a Segment2d with a direction and length they would now need to use the
from_directionconstructor