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 sqr function to CinderMath.h #2175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IntellectualKitty
Copy link

No description provided.

@paulhoux
Copy link
Collaborator

What's the use case for this?

@IntellectualKitty
Copy link
Author

Using a squaring function makes for shorter, easier code. It also more closely resembles the mathematical equation the code is performing.

For example, this seems shorter, simpler, and easier to read:

float distance = cinder::math<float>::sqrt( cinder::math<float>::sqr( x2 - x1 ) + cinder::math<float>::sqr( y2 - y1 ) );

than this:

float deltaX = x2 - x1;
float deltaXSqr = deltaX  * deltaX;
float deltaY = y2 - y1;
float deltaYSqr = deltaY  * deltaY;
float distance = cinder::math<float>::sqrt( deltaXSqr + deltaYSqr );

@andrewfb
Copy link
Collaborator

I am up for this change if there are no objections

@totalgee
Copy link
Contributor

totalgee commented Sep 1, 2020

Just a minor point: what about calling it sq() or squared(), so there is no ambiguity about the sqr being "square root"? Or alternatively (better?), there is already a length2() in GLM (GLX) that does this -- squaring a scalar and doing dot(v,v) on vectors (which returns the squared length).

@paulhoux
Copy link
Collaborator

As a side note:

float deltaX = x2 - x1;
float deltaXSqr = deltaX  * deltaX;
float deltaY = y2 - y1;
float deltaYSqr = deltaY  * deltaY;
float distance = cinder::math<float>::sqrt( deltaXSqr + deltaYSqr );

can be simplified to:

auto delta = vec2( x2 - x1, y2 - y1 );
auto distance = glm::length( delta );

or:

auto distanceSquared = glm::dot( delta, delta );

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

Successfully merging this pull request may close these issues.

4 participants