NovelRT
NovelRT copied to clipboard
Proposal: Modify GeoVector classes to be more feature complete
The GeoVector classes currently lack an out of the box solution to some commonly used functions
Squared length
GeoVectors already provide an API to get a vector's length. However, in some applications you can leave out the square root step at the end of the equation. For example, to determine if a point is in range of a vector. By comparing the squared distance between the vector and the point to the squared range you can tell if the point is within the range of a vector.
Example Implementation
// GeoVector2F.h
inline float getSquaredMagnitude() const noexcept
{
return glm::length2(*reinterpret_cast<const glm::vec2*>(this));
}
inline float getSquaredLength() const noexcept
{
return getSquaredMagnitude();
}
Usage of glm::length2
will require that glm/gtx/norm.hpp
is included, as it is currently
Dot product
The dot product is used in places like vector reflection, for example to bounce a ball against a wall, and calculating the angle between two vectors.
Example Implementation
// GeoVector2F.h
static inline float dot(GeoVector2F lhs, GeoVector2F rhs) noexcept
{
return glm::dot(*reinterpret_cast<const glm::vec2*>(&lhs), *reinterpret_cast<const glm::vec2*>(&rhs));
}
Cross product
A cross product allows for finding a vector that lies perpendicular on two other three-dimensional vectors.
Example Implementation
// GeoVector3F.h
static inline GeoVector3F cross(GeoVector3F lhs, GeoVector3F rhs)
{
return glm::cross(*reinterpret_cast<glm::vec3*>(&lhs), *reinterpret_cast<glm::vec3*>(&rhs));
}
Question:
- Should Vector2F also implement the cross product? Some codebases allow for calculating the cross product between two-dimensional vector by extending them to three-dimensional vectors with the Z component set to 0. As a consequence, the resulting vector would have its X and Y components set to 0
Besides adding new functions, some changes to existing code might be required as well.
Make rotateToAngleAroundPoint
use radians instead of degrees
As stated in https://github.com/novelrt/NovelRT/pull/483#discussion_r922731797, the current behavior of using degrees for rotation is not desired.
Example Implementation
// GeoVector2F.h
void rotateToAngleAroundPoint(float angleRotationValue, GeoVector2F point) noexcept
{
*reinterpret_cast<glm::vec2*>(this) =
glm::rotate((*reinterpret_cast<glm::vec2*>(this) = *reinterpret_cast<const glm::vec2*>(this) -
*reinterpret_cast<const glm::vec2*>(&point)),
angleRotationValue) +
*reinterpret_cast<const glm::vec2*>(&point);
}
As rotateToAngleAroundPoint
now expects angleRotationValue
to be in radians, a conversion is no longer required to use glm::rotate
Question:
- Should there be an API that allows for rotating in degrees alongside one that allows for radians? It would reduce the need for end users to do the conversion from degrees to radians if they are used to working with degrees.
Add missing initializers in GeoVector4F
The constructor GeoVector4F()
lacks any initializers and causes uninitialized member variable warnings on Visual Studio, this might extend to other compilers as well. Adding initializers would bring it in line with the other dimension vectors, as they provide zero initializers for its members in the zero argument constructor.
Example Implementation
GeoVector4F() noexcept : x(0.0f), y(0.0f), z(0.0f), w(0.0f)
{
}
General Question:
- Are there any other functions that would be worth including?
I think my 2c here is that the member methods should follow our new convention, and existing methods should have their naming updated accordingly from camelCase to PascalCase.
I can't think of any additional functions at the moment, but we can keep this ticket open just in case?
Marking this as approved and removing "Proposal" as... well, it's been approved :)
Eh, Didn't I already add the approved label?
The approved label makes no sense if we remove the proposals label either.
Fair enough - can we agree to remove "Proposal" from the title since the issues grown to be approved?
Yeah that's fine