Skip to content

Commit

Permalink
use enum to define axis in mirror_matrix
Browse files Browse the repository at this point in the history
follow naming guidelines for parameters
  • Loading branch information
stf976 committed May 10, 2021
1 parent 54cd2c9 commit 321d016
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
6 changes: 4 additions & 2 deletions framework/include/math_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ namespace gvk
/// extract the translation part out of a matrix
extern glm::vec3 get_translation_from_matrix(const glm::mat4& m);

glm::mat4 cancel_translation_from_matrix(const glm::mat4& m);
glm::mat4 cancel_translation_from_matrix(const glm::mat4& aMatrix);

This comment has been minimized.

Copy link
@johannesugb

johannesugb May 11, 2021

Member

Please add /** */ style comments and make sure to briefly describe the function and all its @parameters as well as its @return value!


glm::mat4 mirror_matrix(const glm::mat4& m, const glm::length_t axis = 0);
enum principal_axis : uint32_t { x = 0u, y, z };

This comment has been minimized.

Copy link
@johannesugb

johannesugb May 10, 2021

Member

An "old" enum is dangerous here, because it drags the enum values into its own namespace (i.e. out of the enum). Furthermore, it is not typesafe in the sense that only type principal_axis could be passed to variables of that type.

=> use enum struct!

	enum struct principal_axis : uint32_t { x = 0u, y, z };

This comment has been minimized.

Copy link
@johannesugb

johannesugb May 11, 2021

Member

Please comment the enum struct to be!


glm::mat4 mirror_matrix(const glm::mat4& aMatrix, const glm::length_t aAxis = principal_axis::x);

This comment has been minimized.

Copy link
@johannesugb

johannesugb May 11, 2021

Member

Please add /** */ style comments and make sure to briefly describe the function and all its @parameters as well as its @return value!


/// <summary>
/// Solve a system of equations with 3 unknowns.
Expand Down
10 changes: 5 additions & 5 deletions framework/src/math_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ namespace gvk
return glm::vec3(m[3][0], m[3][1], m[3][2]);
}

glm::mat4 cancel_translation_from_matrix(const glm::mat4& m)
glm::mat4 cancel_translation_from_matrix(const glm::mat4& aMatrix)
{
return glm::mat4(m[0], m[1], m[2], glm::vec4(0.0f, 0.0f, 0.0f, 1.0f));
return glm::mat4(aMatrix[0], aMatrix[1], aMatrix[2], glm::vec4(0.0f, 0.0f, 0.0f, 1.0f));
}

glm::mat4 mirror_matrix(const glm::mat4& m, const glm::length_t axis)
glm::mat4 mirror_matrix(const glm::mat4& aMatrix, const glm::length_t aAxis)
{
assert(0 <= axis && axis <= 2);
auto axis = static_cast<std::underlying_type<principal_axis>::type>(aAxis);

auto result(m);
auto result(aMatrix);
result[axis] *= -1.0f;
return result;
}
Expand Down

0 comments on commit 321d016

Please sign in to comment.