Skip to content

Fix radians/degrees bug #72

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

Merged
k3DW merged 2 commits into from
Oct 23, 2025
Merged

Fix radians/degrees bug #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5e5b42
Test specific values in sin and cos functions
k3DW Oct 23, 2025
225227d
Create strong types for degrees and radians, to fix the error in defi…
k3DW Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/pstack/calc/rotations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@ namespace pstack::calc {
const std::array<geo::matrix3<float>, 32> arbitrary_rotations = [] {
std::array<geo::matrix3<float>, 32> out;
out[0] = geo::eye3<float>;
out[1] = geo::rot3<float>({ 1, 1, 1 }, 120);
out[2] = geo::rot3<float>({ 1, 1, 1 }, 240);
out[3] = geo::rot3<float>({ 1, 0, 0 }, 180);
out[4] = geo::rot3<float>({ 0, 1, 0 }, 180);
out[5] = geo::rot3<float>({ 0, 0, 1 }, 180);
out[1] = geo::rot3<float>({ 1, 1, 1 }, geo::degrees{120});
out[2] = geo::rot3<float>({ 1, 1, 1 }, geo::degrees{240});
out[3] = geo::rot3<float>({ 1, 0, 0 }, geo::degrees{180});
out[4] = geo::rot3<float>({ 0, 1, 0 }, geo::degrees{180});
out[5] = geo::rot3<float>({ 0, 0, 1 }, geo::degrees{180});
std::random_device rd;
std::mt19937 gen(rd());
std::uniform_real_distribution<float> dis(0, 2 * geo::pi);
for (std::size_t i = 6; i != 32; ++i) {
out[i] = geo::rot3_z(dis(gen)) * geo::rot3_y(dis(gen)) * geo::rot3_x(dis(gen));
out[i] = geo::rot3_z(geo::radians{dis(gen)})
* geo::rot3_y(geo::radians{dis(gen)})
* geo::rot3_x(geo::radians{dis(gen)});
}
return out;
}();
Expand Down
46 changes: 23 additions & 23 deletions src/pstack/calc/rotations.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,29 @@ inline constexpr std::array no_rotations = {

inline constexpr std::array cubic_rotations = {
geo::eye3<float>,
geo::rot3<float>({ 1, 0, 0 }, 90),
geo::rot3<float>({ 1, 0, 0 }, 180),
geo::rot3<float>({ 1, 0, 0 }, 270),
geo::rot3<float>({ 0, 1, 0 }, 90),
geo::rot3<float>({ 0, 1, 0 }, 180),
geo::rot3<float>({ 0, 1, 0 }, 270),
geo::rot3<float>({ 0, 0, 1 }, 90),
geo::rot3<float>({ 0, 0, 1 }, 180),
geo::rot3<float>({ 0, 0, 1 }, 270),
geo::rot3<float>({ 1, 1, 0 }, 180),
geo::rot3<float>({ 1, -1, 0 }, 180),
geo::rot3<float>({ 0, 1, 1 }, 180),
geo::rot3<float>({ 0, -1, 1 }, 180),
geo::rot3<float>({ 1, 0, 1 }, 180),
geo::rot3<float>({ 1, 0, -1 }, 180),
geo::rot3<float>({ 1, 1, 1 }, 120),
geo::rot3<float>({ 1, 1, 1 }, 240),
geo::rot3<float>({ -1, 1, 1 }, 120),
geo::rot3<float>({ -1, 1, 1 }, 240),
geo::rot3<float>({ 1, -1, 1 }, 120),
geo::rot3<float>({ 1, -1, 1 }, 240),
geo::rot3<float>({ 1, 1, -1 }, 120),
geo::rot3<float>({ 1, 1, -1 }, 240),
geo::rot3<float>({ 1, 0, 0 }, geo::degrees{90}),
geo::rot3<float>({ 1, 0, 0 }, geo::degrees{180}),
geo::rot3<float>({ 1, 0, 0 }, geo::degrees{270}),
geo::rot3<float>({ 0, 1, 0 }, geo::degrees{90}),
geo::rot3<float>({ 0, 1, 0 }, geo::degrees{180}),
geo::rot3<float>({ 0, 1, 0 }, geo::degrees{270}),
geo::rot3<float>({ 0, 0, 1 }, geo::degrees{90}),
geo::rot3<float>({ 0, 0, 1 }, geo::degrees{180}),
geo::rot3<float>({ 0, 0, 1 }, geo::degrees{270}),
geo::rot3<float>({ 1, 1, 0 }, geo::degrees{180}),
geo::rot3<float>({ 1, -1, 0 }, geo::degrees{180}),
geo::rot3<float>({ 0, 1, 1 }, geo::degrees{180}),
geo::rot3<float>({ 0, -1, 1 }, geo::degrees{180}),
geo::rot3<float>({ 1, 0, 1 }, geo::degrees{180}),
geo::rot3<float>({ 1, 0, -1 }, geo::degrees{180}),
geo::rot3<float>({ 1, 1, 1 }, geo::degrees{120}),
geo::rot3<float>({ 1, 1, 1 }, geo::degrees{240}),
geo::rot3<float>({ -1, 1, 1 }, geo::degrees{120}),
geo::rot3<float>({ -1, 1, 1 }, geo::degrees{240}),
geo::rot3<float>({ 1, -1, 1 }, geo::degrees{120}),
geo::rot3<float>({ 1, -1, 1 }, geo::degrees{240}),
geo::rot3<float>({ 1, 1, -1 }, geo::degrees{120}),
geo::rot3<float>({ 1, 1, -1 }, geo::degrees{240}),
};

extern const std::array<geo::matrix3<float>, 32> arbitrary_rotations;
Expand Down
6 changes: 3 additions & 3 deletions src/pstack/calc/stacker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ std::optional<stack_result> stack_impl(const stack_parameters& params, const std
static constexpr std::size_t sections = 20;
static constexpr double angle_diff = 2 * geo::pi / sections;

static constexpr geo::matrix3 rot_x = geo::rot3_x<float>(angle_diff);
static constexpr geo::matrix3 rot_y = geo::rot3_y<float>(angle_diff);
static constexpr geo::matrix3 rot_x = geo::rot3_x<float>(geo::radians{angle_diff});
static constexpr geo::matrix3 rot_y = geo::rot3_y<float>(geo::radians{angle_diff});

int min_box_volume = std::numeric_limits<int>::max();
double best_x = 0;
Expand All @@ -179,7 +179,7 @@ std::optional<stack_result> stack_impl(const stack_parameters& params, const std
}
}

base_rotation = geo::rot3_y<float>(best_y) * geo::rot3_x<float>(best_x);
base_rotation = geo::rot3_y<float>(geo::radians{best_y}) * geo::rot3_x<float>(geo::radians{best_x});
}

// Set up array of parts
Expand Down
27 changes: 24 additions & 3 deletions src/pstack/geo/functions.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define PSTACK_GEO_FUNCTIONS_HPP

#include <bit>
#include <cmath>
#include <concepts>
#include <cstdint>
#include <numbers>
Expand All @@ -11,8 +12,27 @@ namespace pstack::geo {
static_assert(sizeof(double) == sizeof(std::uint64_t));

using std::numbers::pi;

constexpr double sin(double x) {
using std::numbers::sqrt2;

struct radians;
struct degrees;

struct radians {
explicit radians(double val) : value(val) {}
radians(degrees d);
double value;
};
struct degrees {
explicit degrees(double val) : value(val) {}
degrees(radians r);
double value;
};

inline radians::radians(degrees d) : value(d.value * (pi / 180)) {}
inline degrees::degrees(radians r) : value(r.value * (180 / pi)) {}

constexpr double sin(radians r) {
double x = r.value;
while (x > pi) {
x -= 2 * pi;
}
Expand Down Expand Up @@ -43,7 +63,8 @@ constexpr double sin(double x) {
return result * sign;
}

constexpr double cos(double x) {
constexpr double cos(radians r) {
double x = r.value;
while (x > pi) {
x -= 2 * pi;
}
Expand Down
8 changes: 4 additions & 4 deletions src/pstack/geo/matrix3.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ constexpr matrix3<T> operator*(const std::type_identity_t<T>& lhs, const matrix3
}

template <class T>
constexpr matrix3<T> rot3(const vector3<T>& axis, const std::type_identity_t<T> theta) {
constexpr matrix3<T> rot3(const vector3<T>& axis, const radians theta) {
const T c = static_cast<T>(cos(theta));
const T s = static_cast<T>(sin(theta));
const auto n = normalize(axis);
Expand All @@ -77,7 +77,7 @@ constexpr matrix3<T> rot3(const vector3<T>& axis, const std::type_identity_t<T>
}

template <class T>
constexpr matrix3<T> rot3_x(const T theta) {
constexpr matrix3<T> rot3_x(const radians theta) {
const T c = static_cast<T>(cos(theta));
const T s = static_cast<T>(sin(theta));
return { 1, 0, 0,
Expand All @@ -86,7 +86,7 @@ constexpr matrix3<T> rot3_x(const T theta) {
}

template <class T>
constexpr matrix3<T> rot3_y(const T theta) {
constexpr matrix3<T> rot3_y(const radians theta) {
const T c = static_cast<T>(cos(theta));
const T s = static_cast<T>(sin(theta));
return { c, 0, s,
Expand All @@ -95,7 +95,7 @@ constexpr matrix3<T> rot3_y(const T theta) {
}

template <class T>
constexpr matrix3<T> rot3_z(const T theta) {
constexpr matrix3<T> rot3_z(const radians theta) {
const T c = static_cast<T>(cos(theta));
const T s = static_cast<T>(sin(theta));
return { c, -s, 0,
Expand Down
6 changes: 3 additions & 3 deletions src/pstack/geo/matrix4.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ inline constexpr matrix4<T> eye4 = { 1, 0, 0, 0,
0, 0, 0, 1 };

template <class T>
constexpr matrix4<T> rot4_x(const T theta) {
constexpr matrix4<T> rot4_x(const radians theta) {
const T c = static_cast<T>(cos(theta));
const T s = static_cast<T>(sin(theta));
return { 1, 0, 0, 0,
Expand All @@ -56,7 +56,7 @@ constexpr matrix4<T> rot4_x(const T theta) {
}

template <class T>
constexpr matrix4<T> rot4_y(const T theta) {
constexpr matrix4<T> rot4_y(const radians theta) {
const T c = static_cast<T>(cos(theta));
const T s = static_cast<T>(sin(theta));
return { c, 0, s, 0,
Expand All @@ -66,7 +66,7 @@ constexpr matrix4<T> rot4_y(const T theta) {
}

template <class T>
constexpr matrix4<T> rot4_z(const T theta) {
constexpr matrix4<T> rot4_z(const radians theta) {
const T c = static_cast<T>(cos(theta));
const T s = static_cast<T>(sin(theta));
return { c, -s, 0, 0,
Expand Down
56 changes: 54 additions & 2 deletions src/pstack/geo/test/functions_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,66 @@ TEST_CASE("pi", "[functions]") {
STATIC_CHECK(pi == std::numbers::pi);
}

TEST_CASE("sqrt2", "[functions]") {
STATIC_CHECK(sqrt2 == std::numbers::sqrt2);
}

TEST_CASE("sin", "[functions]") {
const auto num = g.generate<double>();
CHECK_THAT(sin(num), WithinAbs(std::sin(num), epsilon<double>));
CHECK_THAT(sin(radians{num}), WithinAbs(std::sin(num), epsilon<double>));
}

TEST_CASE("sin specific radian values", "[functions]") {
CHECK_THAT(sin(radians{0}), WithinAbs(0, epsilon<double>));
CHECK_THAT(sin(radians{pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(radians{pi / 2}), WithinAbs(1, epsilon<double>));
CHECK_THAT(sin(radians{3 * pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(radians{pi}), WithinAbs(0, epsilon<double>));
CHECK_THAT(sin(radians{5 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(radians{3 * pi / 2}), WithinAbs(-1, epsilon<double>));
CHECK_THAT(sin(radians{7 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(radians{2 * pi}), WithinAbs(0, epsilon<double>));
}

TEST_CASE("sin specific degree values", "[functions]") {
CHECK_THAT(sin(degrees{0}), WithinAbs(0, epsilon<double>));
CHECK_THAT(sin(degrees{45}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(degrees{90}), WithinAbs(1, epsilon<double>));
CHECK_THAT(sin(degrees{135}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(degrees{180}), WithinAbs(0, epsilon<double>));
CHECK_THAT(sin(degrees{225}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(degrees{270}), WithinAbs(-1, epsilon<double>));
CHECK_THAT(sin(degrees{315}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(sin(degrees{360}), WithinAbs(0, epsilon<double>));
}

TEST_CASE("cos", "[functions]") {
const auto num = g.generate<double>();
CHECK_THAT(cos(num), WithinAbs(std::cos(num), epsilon<double>));
CHECK_THAT(cos(radians{num}), WithinAbs(std::cos(num), epsilon<double>));
}

TEST_CASE("cos specific radian values", "[functions]") {
CHECK_THAT(cos(radians{0}), WithinAbs(1, epsilon<double>));
CHECK_THAT(cos(radians{pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(radians{pi / 2}), WithinAbs(0, epsilon<double>));
CHECK_THAT(cos(radians{3 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(radians{pi}), WithinAbs(-1, epsilon<double>));
CHECK_THAT(cos(radians{5 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(radians{3 * pi / 2}), WithinAbs(0, epsilon<double>));
CHECK_THAT(cos(radians{7 * pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(radians{2 * pi}), WithinAbs(1, epsilon<double>));
}

TEST_CASE("cos specific degree values", "[functions]") {
CHECK_THAT(cos(degrees{0}), WithinAbs(1, epsilon<double>));
CHECK_THAT(cos(degrees{45}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(degrees{90}), WithinAbs(0, epsilon<double>));
CHECK_THAT(cos(degrees{135}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(degrees{180}), WithinAbs(-1, epsilon<double>));
CHECK_THAT(cos(degrees{225}), WithinAbs(-sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(degrees{270}), WithinAbs(0, epsilon<double>));
CHECK_THAT(cos(degrees{315}), WithinAbs(sqrt2 / 2, epsilon<double>));
CHECK_THAT(cos(degrees{360}), WithinAbs(1, epsilon<double>));
}

TEST_CASE("ceil fractional", "[functions]") {
Expand Down
8 changes: 4 additions & 4 deletions src/pstack/geo/test/matrix3_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ TEMPLATE_TEST_CASE("rot3()", "[matrix3]",
const auto [axis, theta] = g.generate<vector3<T>, T>();
const auto c = std::cos(theta);
const auto s = std::sin(theta);
const matrix3<T> actual = rot3(axis, theta);
const matrix3<T> actual = rot3(axis, geo::radians{theta});
const auto n = normalize(axis);
const matrix3<T> K = { 0, -n.z, n.y,
n.z, 0, -n.x,
Expand All @@ -172,7 +172,7 @@ TEMPLATE_TEST_CASE("rot3_x()", "[matrix3]",
const auto theta = g.generate<T>();
const auto c = std::cos(theta);
const auto s = std::sin(theta);
const matrix3<T> actual = rot3_x(theta);
const matrix3<T> actual = rot3_x<T>(geo::radians{theta});
CHECK(actual.xx == 1);
CHECK(actual.xy == 0);
CHECK(actual.xz == 0);
Expand All @@ -191,7 +191,7 @@ TEMPLATE_TEST_CASE("rot3_y()", "[matrix3]",
const auto theta = g.generate<T>();
const auto c = std::cos(theta);
const auto s = std::sin(theta);
const matrix3<T> actual = rot3_y(theta);
const matrix3<T> actual = rot3_y<T>(geo::radians{theta});
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
CHECK(actual.xy == 0);
CHECK_THAT(actual.xz, WithinAbs(s, epsilon<T>));
Expand All @@ -210,7 +210,7 @@ TEMPLATE_TEST_CASE("rot3_z()", "[matrix3]",
const auto theta = g.generate<T>();
const auto c = std::cos(theta);
const auto s = std::sin(theta);
const matrix3<T> actual = rot3_z(theta);
const matrix3<T> actual = rot3_z<T>(geo::radians{theta});
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
CHECK_THAT(actual.xy, WithinAbs(-s, epsilon<T>));
CHECK(actual.xz == 0);
Expand Down
6 changes: 3 additions & 3 deletions src/pstack/geo/test/matrix4_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ TEMPLATE_TEST_CASE("rot4_x()", "[matrix4]",
const auto theta = g.generate<T>();
const auto c = std::cos(theta);
const auto s = std::sin(theta);
const matrix4<T> actual = rot4_x(theta);
const matrix4<T> actual = rot4_x<T>(geo::radians{theta});
CHECK(actual.xx == 1);
CHECK(actual.xy == 0);
CHECK(actual.xz == 0);
Expand All @@ -143,7 +143,7 @@ TEMPLATE_TEST_CASE("rot4_y()", "[matrix4]",
const auto theta = g.generate<T>();
const auto c = std::cos(theta);
const auto s = std::sin(theta);
const matrix4<T> actual = rot4_y(theta);
const matrix4<T> actual = rot4_y<T>(geo::radians{theta});
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
CHECK(actual.xy == 0);
CHECK_THAT(actual.xz, WithinAbs(s, epsilon<T>));
Expand All @@ -169,7 +169,7 @@ TEMPLATE_TEST_CASE("rot4_z()", "[matrix4]",
const auto theta = g.generate<T>();
const auto c = std::cos(theta);
const auto s = std::sin(theta);
const matrix4<T> actual = rot4_z(theta);
const matrix4<T> actual = rot4_z<T>(geo::radians{theta});
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
CHECK_THAT(actual.xy, WithinAbs(-s, epsilon<T>));
CHECK(actual.xz == 0);
Expand Down
2 changes: 1 addition & 1 deletion src/pstack/gui/transformation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class transformation {
}

void rotate_by(float rx, float ry) {
_orientation = geo::rot4_y(ry) * geo::rot4_x(rx) * _orientation;
_orientation = geo::rot4_y<float>(geo::radians{ry}) * geo::rot4_x<float>(geo::radians{rx}) * _orientation;
_recalculate();
}
void scale_mesh(float factor) {
Expand Down