Skip to content

Scalable primitives #20094

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Scalable primitives #20094

wants to merge 2 commits into from

Conversation

lynn-lumen
Copy link
Contributor

Objective

Solution

  • Added 3 new traits
    • ScaleUniform which allows scaling 2D and 3D primitives uniformly by some scale: f32
    • ScaleNonUniform2d which allows scaling 2D primitives non-uniformly by specifying a scale: Vec2 and an additional associated Output type. This may be useful when scaling i.e. a circle as it will become an ellipse.
    • ScaleNonUniform3d - the 3D analogue of ScaleNonUniform2d
  • Added implementations of these traits for Circle, Rectangle, Sphere and Cuboid

Additional Information

  • I am not sure whether these traits are named optimally. If you have any suggestions for better naming please do suggest them :)
  • There are a lot of other implementations that would have to be added. Most of them are fairly easy however so it should not be that much of a maintenance burden.

@tim-blackbird tim-blackbird added C-Feature A new feature, making something new possible A-Math Fundamental domain-agnostic mathematical operations S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 11, 2025
IQuick143
IQuick143 approved these changes Jul 14, 2025
View reviewed changes
Copy link
Contributor

@IQuick143 IQuick143 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally not a fan of nonuniform scaling, but if there's demand for it, I don't see a problem with this design.

mogambro
mogambro approved these changes Jul 28, 2025
View reviewed changes
Copy link

@mogambro mogambro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good and simple enough, still useful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
2 more reviewers

@IQuick143 IQuick143 IQuick143 approved these changes

@mogambro mogambro mogambro approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lynn-lumen @IQuick143 @mogambro @tim-blackbird