Add more constructors and math helpers for primitive shapes #10632
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jondolf
added
A-Math
Jondolf
commented
Nov 18, 2023
Comment on lines
76
to
79
| /// Get the area of the ellipse | ||
| pub fn area(&self) -> f32 { | ||
| PI * self.half_width * self.half_height | ||
| } |
There was a problem hiding this comment.
Note: I would've also added a perimeter method, but there is no trivial method of computing it for ellipses outside of complex approximations
Jondolf
commented
Nov 18, 2023
Comment on lines
266
to
272
| /// Create a new `Rectangle` from a given half-size | ||
| pub fn from_half_size(half_size: Vec2) -> Self { | ||
| Self { | ||
| half_width: size.x / 2., | ||
| half_height: size.y / 2., | ||
| half_width: half_size.x, | ||
| half_height: half_size.y, | ||
| } | ||
| } |
There was a problem hiding this comment.
Cuboid stores a half_size vector, but Rectangle stores it with separate properties. Should Rectangle store a half_size as well? It would make this constructor and the half_size method obsolete and make the shape representations more consistent.
Jondolf
commented
Nov 18, 2023
| /// | ||
| /// This is the angle formed by two adjacent sides with points | ||
| /// within the angle being in the interior of the polygon | ||
| pub fn internal_angle_degrees(&self) -> f32 { |
There was a problem hiding this comment.
Is it fine to have separate _degrees and _radians versions, or should there be just one, possibly with degrees? Using separate versions makes the computation slightly more efficient since the user doesn't need to convert the units.
Jondolf
commented
Nov 18, 2023
41 tasks
Compiler should optimize these
|
I ended up splitting the |
Jondolf
changed the title
Jondolf
commented
Nov 19, 2023
IQuick143
reviewed
Nov 21, 2023
IQuick143
reviewed
Nov 21, 2023
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
IQuick143
reviewed
Nov 28, 2023
IQuick143
approved these changes
Nov 28, 2023
2 tasks
Jondolf
force-pushed
the
primitive-helpers
branch
from
8b69a81 to
5965561
Compare
NiseVoid
approved these changes
Jan 28, 2024
Jondolf
added
the
S-Ready-For-Final-Review
alice-i-cecile
approved these changes
Jan 29, 2024
tjamaan
pushed a commit
to tjamaan/bevy
that referenced
this pull request
Feb 6, 2024
…ne#10632) # Objective Working towards finishing a part of bevyengine#10572, this PR adds a ton of math helpers and useful constructors for primitive shapes. I also tried fixing some naming inconsistencies. ## Solution - Add mathematical helpers like `area`, `volume`, `perimeter`, `RegularPolygon::inradius` and so on, trying to cover all core mathematical properties of each shape - Add some constructors like `Rectangle::from_corners`, `Cuboid::from_corners` and `Plane3d::from_points` I also derived `PartialEq` for the shapes where it's trivial. Primitives like `Line2d` and `Segment2d` are not trivial because you could argue that they would be equal if they had an opposite direction. All mathematical methods have tests with reference values computed by hand or with external tools. ## Todo - [x] Add tests to verify that the values from mathematical helpers are correct --------- Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Working towards finishing a part of #10572, this PR adds a ton of math helpers and useful constructors for primitive shapes. I also tried fixing some naming inconsistencies.
Solution
area,volume,perimeter,RegularPolygon::inradiusand so on, trying to cover all core mathematical properties of each shapeRectangle::from_corners,Cuboid::from_cornersandPlane3d::from_pointsI also derived
PartialEqfor the shapes where it's trivial. Primitives likeLine2dandSegment2dare not trivial because you could argue that they would be equal if they had an opposite direction.All mathematical methods have tests with reference values computed by hand or with external tools.
Todo