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

Add is_conformal method to Basis and Transform2D #79523

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Jul 15, 2023

This PR adds a new method is_conformal to Basis and Transform2D.

This method returns true if the basis is conformal, meaning it preserves angles and distance ratios, and may only composed of rotation or uniform scale. Returns false if the basis has non-uniform scale or shear/skew.

This can be used to validate if the basis is non-distorted, which is important for physics and other use cases. I have wanted a method like this for a long time but I did not know what to call it before now.

This PR also includes documentation and test cases. Most of the added lines are test cases.

I plan to use this method in the engine in #79364, but I wanted to open this PR to add the method first.

Copy link
Contributor

@nlupugla nlupugla left a comment

Choose a reason for hiding this comment

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

Nice work :)

@Calinou Calinou added this to the 4.x milestone Jul 16, 2023
@aaronfranke aaronfranke force-pushed the is-conformal branch 2 times, most recently from 09681ac to d85f55b Compare August 1, 2023 18:52
@aaronfranke aaronfranke force-pushed the is-conformal branch 2 times, most recently from 1ae6529 to 38c7639 Compare August 19, 2023 05:47
@aaronfranke
Copy link
Member Author

Updated the PR to have an optimized 3D implementation suggested by @SlugFiller.

doc/classes/Basis.xml Outdated Show resolved Hide resolved
doc/classes/Transform2D.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@RevoluPowered RevoluPowered left a comment

Choose a reason for hiding this comment

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

minor issue nothing scary

core/math/basis.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@RevoluPowered RevoluPowered left a comment

Choose a reason for hiding this comment

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

LGTM - probably @akien-mga should give it a check for merge

@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the is-conformal branch September 26, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants