Skip to content

Commit 26419e8

Browse files
k3DWk3DW
authored
Merge pull request #72 from PartStackerCommunity/radians
Fix radians/degrees bug
2 parents ebf7e42 + 225227d commit 26419e8

File tree

10 files changed

+127
-52
lines changed

10 files changed

+127
-52
lines changed

‎src/pstack/calc/rotations.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,18 @@ namespace pstack::calc {
66
const std::array<geo::matrix3<float>, 32> arbitrary_rotations = [] {
77
std::array<geo::matrix3<float>, 32> out;
88
out[0] = geo::eye3<float>;
9-
out[1] = geo::rot3<float>({ 1, 1, 1 }, 120);
10-
out[2] = geo::rot3<float>({ 1, 1, 1 }, 240);
11-
out[3] = geo::rot3<float>({ 1, 0, 0 }, 180);
12-
out[4] = geo::rot3<float>({ 0, 1, 0 }, 180);
13-
out[5] = geo::rot3<float>({ 0, 0, 1 }, 180);
9+
out[1] = geo::rot3<float>({ 1, 1, 1 }, geo::degrees{120});
10+
out[2] = geo::rot3<float>({ 1, 1, 1 }, geo::degrees{240});
11+
out[3] = geo::rot3<float>({ 1, 0, 0 }, geo::degrees{180});
12+
out[4] = geo::rot3<float>({ 0, 1, 0 }, geo::degrees{180});
13+
out[5] = geo::rot3<float>({ 0, 0, 1 }, geo::degrees{180});
1414
std::random_device rd;
1515
std::mt19937 gen(rd());
1616
std::uniform_real_distribution<float> dis(0, 2 * geo::pi);
1717
for (std::size_t i = 6; i != 32; ++i) {
18-
out[i] = geo::rot3_z(dis(gen)) * geo::rot3_y(dis(gen)) * geo::rot3_x(dis(gen));
18+
out[i] = geo::rot3_z(geo::radians{dis(gen)})
19+
* geo::rot3_y(geo::radians{dis(gen)})
20+
* geo::rot3_x(geo::radians{dis(gen)});
1921
}
2022
return out;
2123
}();

‎src/pstack/calc/rotations.hpp‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,29 @@ inline constexpr std::array no_rotations = {
1313

1414
inline constexpr std::array cubic_rotations = {
1515
geo::eye3<float>,
16-
geo::rot3<float>({ 1, 0, 0 }, 90),
17-
geo::rot3<float>({ 1, 0, 0 }, 180),
18-
geo::rot3<float>({ 1, 0, 0 }, 270),
19-
geo::rot3<float>({ 0, 1, 0 }, 90),
20-
geo::rot3<float>({ 0, 1, 0 }, 180),
21-
geo::rot3<float>({ 0, 1, 0 }, 270),
22-
geo::rot3<float>({ 0, 0, 1 }, 90),
23-
geo::rot3<float>({ 0, 0, 1 }, 180),
24-
geo::rot3<float>({ 0, 0, 1 }, 270),
25-
geo::rot3<float>({ 1, 1, 0 }, 180),
26-
geo::rot3<float>({ 1, -1, 0 }, 180),
27-
geo::rot3<float>({ 0, 1, 1 }, 180),
28-
geo::rot3<float>({ 0, -1, 1 }, 180),
29-
geo::rot3<float>({ 1, 0, 1 }, 180),
30-
geo::rot3<float>({ 1, 0, -1 }, 180),
31-
geo::rot3<float>({ 1, 1, 1 }, 120),
32-
geo::rot3<float>({ 1, 1, 1 }, 240),
33-
geo::rot3<float>({ -1, 1, 1 }, 120),
34-
geo::rot3<float>({ -1, 1, 1 }, 240),
35-
geo::rot3<float>({ 1, -1, 1 }, 120),
36-
geo::rot3<float>({ 1, -1, 1 }, 240),
37-
geo::rot3<float>({ 1, 1, -1 }, 120),
38-
geo::rot3<float>({ 1, 1, -1 }, 240),
16+
geo::rot3<float>({ 1, 0, 0 }, geo::degrees{90}),
17+
geo::rot3<float>({ 1, 0, 0 }, geo::degrees{180}),
18+
geo::rot3<float>({ 1, 0, 0 }, geo::degrees{270}),
19+
geo::rot3<float>({ 0, 1, 0 }, geo::degrees{90}),
20+
geo::rot3<float>({ 0, 1, 0 }, geo::degrees{180}),
21+
geo::rot3<float>({ 0, 1, 0 }, geo::degrees{270}),
22+
geo::rot3<float>({ 0, 0, 1 }, geo::degrees{90}),
23+
geo::rot3<float>({ 0, 0, 1 }, geo::degrees{180}),
24+
geo::rot3<float>({ 0, 0, 1 }, geo::degrees{270}),
25+
geo::rot3<float>({ 1, 1, 0 }, geo::degrees{180}),
26+
geo::rot3<float>({ 1, -1, 0 }, geo::degrees{180}),
27+
geo::rot3<float>({ 0, 1, 1 }, geo::degrees{180}),
28+
geo::rot3<float>({ 0, -1, 1 }, geo::degrees{180}),
29+
geo::rot3<float>({ 1, 0, 1 }, geo::degrees{180}),
30+
geo::rot3<float>({ 1, 0, -1 }, geo::degrees{180}),
31+
geo::rot3<float>({ 1, 1, 1 }, geo::degrees{120}),
32+
geo::rot3<float>({ 1, 1, 1 }, geo::degrees{240}),
33+
geo::rot3<float>({ -1, 1, 1 }, geo::degrees{120}),
34+
geo::rot3<float>({ -1, 1, 1 }, geo::degrees{240}),
35+
geo::rot3<float>({ 1, -1, 1 }, geo::degrees{120}),
36+
geo::rot3<float>({ 1, -1, 1 }, geo::degrees{240}),
37+
geo::rot3<float>({ 1, 1, -1 }, geo::degrees{120}),
38+
geo::rot3<float>({ 1, 1, -1 }, geo::degrees{240}),
3939
};
4040

4141
extern const std::array<geo::matrix3<float>, 32> arbitrary_rotations;

‎src/pstack/calc/stacker.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,8 @@ std::optional<stack_result> stack_impl(const stack_parameters& params, const std
158158
static constexpr std::size_t sections = 20;
159159
static constexpr double angle_diff = 2 * geo::pi / sections;
160160

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

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

182-
base_rotation = geo::rot3_y<float>(best_y) * geo::rot3_x<float>(best_x);
182+
base_rotation = geo::rot3_y<float>(geo::radians{best_y}) * geo::rot3_x<float>(geo::radians{best_x});
183183
}
184184

185185
// Set up array of parts

‎src/pstack/geo/functions.hpp‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#define PSTACK_GEO_FUNCTIONS_HPP
33

44
#include <bit>
5+
#include <cmath>
56
#include <concepts>
67
#include <cstdint>
78
#include <numbers>
@@ -11,8 +12,27 @@ namespace pstack::geo {
1112
static_assert(sizeof(double) == sizeof(std::uint64_t));
1213

1314
using std::numbers::pi;
14-
15-
constexpr double sin(double x) {
15+
using std::numbers::sqrt2;
16+
17+
struct radians;
18+
struct degrees;
19+
20+
struct radians {
21+
explicit radians(double val) : value(val) {}
22+
radians(degrees d);
23+
double value;
24+
};
25+
struct degrees {
26+
explicit degrees(double val) : value(val) {}
27+
degrees(radians r);
28+
double value;
29+
};
30+
31+
inline radians::radians(degrees d) : value(d.value * (pi / 180)) {}
32+
inline degrees::degrees(radians r) : value(r.value * (180 / pi)) {}
33+
34+
constexpr double sin(radians r) {
35+
double x = r.value;
1636
while (x > pi) {
1737
x -= 2 * pi;
1838
}
@@ -43,7 +63,8 @@ constexpr double sin(double x) {
4363
return result * sign;
4464
}
4565

46-
constexpr double cos(double x) {
66+
constexpr double cos(radians r) {
67+
double x = r.value;
4768
while (x > pi) {
4869
x -= 2 * pi;
4970
}

‎src/pstack/geo/matrix3.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ constexpr matrix3<T> operator*(const std::type_identity_t<T>& lhs, const matrix3
6565
}
6666

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

7979
template <class T>
80-
constexpr matrix3<T> rot3_x(const T theta) {
80+
constexpr matrix3<T> rot3_x(const radians theta) {
8181
const T c = static_cast<T>(cos(theta));
8282
const T s = static_cast<T>(sin(theta));
8383
return { 1, 0, 0,
@@ -86,7 +86,7 @@ constexpr matrix3<T> rot3_x(const T theta) {
8686
}
8787

8888
template <class T>
89-
constexpr matrix3<T> rot3_y(const T theta) {
89+
constexpr matrix3<T> rot3_y(const radians theta) {
9090
const T c = static_cast<T>(cos(theta));
9191
const T s = static_cast<T>(sin(theta));
9292
return { c, 0, s,
@@ -95,7 +95,7 @@ constexpr matrix3<T> rot3_y(const T theta) {
9595
}
9696

9797
template <class T>
98-
constexpr matrix3<T> rot3_z(const T theta) {
98+
constexpr matrix3<T> rot3_z(const radians theta) {
9999
const T c = static_cast<T>(cos(theta));
100100
const T s = static_cast<T>(sin(theta));
101101
return { c, -s, 0,

‎src/pstack/geo/matrix4.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ inline constexpr matrix4<T> eye4 = { 1, 0, 0, 0,
4646
0, 0, 0, 1 };
4747

4848
template <class T>
49-
constexpr matrix4<T> rot4_x(const T theta) {
49+
constexpr matrix4<T> rot4_x(const radians theta) {
5050
const T c = static_cast<T>(cos(theta));
5151
const T s = static_cast<T>(sin(theta));
5252
return { 1, 0, 0, 0,
@@ -56,7 +56,7 @@ constexpr matrix4<T> rot4_x(const T theta) {
5656
}
5757

5858
template <class T>
59-
constexpr matrix4<T> rot4_y(const T theta) {
59+
constexpr matrix4<T> rot4_y(const radians theta) {
6060
const T c = static_cast<T>(cos(theta));
6161
const T s = static_cast<T>(sin(theta));
6262
return { c, 0, s, 0,
@@ -66,7 +66,7 @@ constexpr matrix4<T> rot4_y(const T theta) {
6666
}
6767

6868
template <class T>
69-
constexpr matrix4<T> rot4_z(const T theta) {
69+
constexpr matrix4<T> rot4_z(const radians theta) {
7070
const T c = static_cast<T>(cos(theta));
7171
const T s = static_cast<T>(sin(theta));
7272
return { c, -s, 0, 0,

‎src/pstack/geo/test/functions_ut.cpp‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,66 @@ TEST_CASE("pi", "[functions]") {
1616
STATIC_CHECK(pi == std::numbers::pi);
1717
}
1818

19+
TEST_CASE("sqrt2", "[functions]") {
20+
STATIC_CHECK(sqrt2 == std::numbers::sqrt2);
21+
}
22+
1923
TEST_CASE("sin", "[functions]") {
2024
const auto num = g.generate<double>();
21-
CHECK_THAT(sin(num), WithinAbs(std::sin(num), epsilon<double>));
25+
CHECK_THAT(sin(radians{num}), WithinAbs(std::sin(num), epsilon<double>));
26+
}
27+
28+
TEST_CASE("sin specific radian values", "[functions]") {
29+
CHECK_THAT(sin(radians{0}), WithinAbs(0, epsilon<double>));
30+
CHECK_THAT(sin(radians{pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
31+
CHECK_THAT(sin(radians{pi / 2}), WithinAbs(1, epsilon<double>));
32+
CHECK_THAT(sin(radians{3 * pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
33+
CHECK_THAT(sin(radians{pi}), WithinAbs(0, epsilon<double>));
34+
CHECK_THAT(sin(radians{5 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
35+
CHECK_THAT(sin(radians{3 * pi / 2}), WithinAbs(-1, epsilon<double>));
36+
CHECK_THAT(sin(radians{7 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
37+
CHECK_THAT(sin(radians{2 * pi}), WithinAbs(0, epsilon<double>));
38+
}
39+
40+
TEST_CASE("sin specific degree values", "[functions]") {
41+
CHECK_THAT(sin(degrees{0}), WithinAbs(0, epsilon<double>));
42+
CHECK_THAT(sin(degrees{45}), WithinAbs(sqrt2 / 2, epsilon<double>));
43+
CHECK_THAT(sin(degrees{90}), WithinAbs(1, epsilon<double>));
44+
CHECK_THAT(sin(degrees{135}), WithinAbs(sqrt2 / 2, epsilon<double>));
45+
CHECK_THAT(sin(degrees{180}), WithinAbs(0, epsilon<double>));
46+
CHECK_THAT(sin(degrees{225}), WithinAbs(-sqrt2 / 2, epsilon<double>));
47+
CHECK_THAT(sin(degrees{270}), WithinAbs(-1, epsilon<double>));
48+
CHECK_THAT(sin(degrees{315}), WithinAbs(-sqrt2 / 2, epsilon<double>));
49+
CHECK_THAT(sin(degrees{360}), WithinAbs(0, epsilon<double>));
2250
}
2351

2452
TEST_CASE("cos", "[functions]") {
2553
const auto num = g.generate<double>();
26-
CHECK_THAT(cos(num), WithinAbs(std::cos(num), epsilon<double>));
54+
CHECK_THAT(cos(radians{num}), WithinAbs(std::cos(num), epsilon<double>));
55+
}
56+
57+
TEST_CASE("cos specific radian values", "[functions]") {
58+
CHECK_THAT(cos(radians{0}), WithinAbs(1, epsilon<double>));
59+
CHECK_THAT(cos(radians{pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
60+
CHECK_THAT(cos(radians{pi / 2}), WithinAbs(0, epsilon<double>));
61+
CHECK_THAT(cos(radians{3 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
62+
CHECK_THAT(cos(radians{pi}), WithinAbs(-1, epsilon<double>));
63+
CHECK_THAT(cos(radians{5 * pi / 4}), WithinAbs(-sqrt2 / 2, epsilon<double>));
64+
CHECK_THAT(cos(radians{3 * pi / 2}), WithinAbs(0, epsilon<double>));
65+
CHECK_THAT(cos(radians{7 * pi / 4}), WithinAbs(sqrt2 / 2, epsilon<double>));
66+
CHECK_THAT(cos(radians{2 * pi}), WithinAbs(1, epsilon<double>));
67+
}
68+
69+
TEST_CASE("cos specific degree values", "[functions]") {
70+
CHECK_THAT(cos(degrees{0}), WithinAbs(1, epsilon<double>));
71+
CHECK_THAT(cos(degrees{45}), WithinAbs(sqrt2 / 2, epsilon<double>));
72+
CHECK_THAT(cos(degrees{90}), WithinAbs(0, epsilon<double>));
73+
CHECK_THAT(cos(degrees{135}), WithinAbs(-sqrt2 / 2, epsilon<double>));
74+
CHECK_THAT(cos(degrees{180}), WithinAbs(-1, epsilon<double>));
75+
CHECK_THAT(cos(degrees{225}), WithinAbs(-sqrt2 / 2, epsilon<double>));
76+
CHECK_THAT(cos(degrees{270}), WithinAbs(0, epsilon<double>));
77+
CHECK_THAT(cos(degrees{315}), WithinAbs(sqrt2 / 2, epsilon<double>));
78+
CHECK_THAT(cos(degrees{360}), WithinAbs(1, epsilon<double>));
2779
}
2880

2981
TEST_CASE("ceil fractional", "[functions]") {

‎src/pstack/geo/test/matrix3_ut.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ TEMPLATE_TEST_CASE("rot3()", "[matrix3]",
148148
const auto [axis, theta] = g.generate<vector3<T>, T>();
149149
const auto c = std::cos(theta);
150150
const auto s = std::sin(theta);
151-
const matrix3<T> actual = rot3(axis, theta);
151+
const matrix3<T> actual = rot3(axis, geo::radians{theta});
152152
const auto n = normalize(axis);
153153
const matrix3<T> K = { 0, -n.z, n.y,
154154
n.z, 0, -n.x,
@@ -172,7 +172,7 @@ TEMPLATE_TEST_CASE("rot3_x()", "[matrix3]",
172172
const auto theta = g.generate<T>();
173173
const auto c = std::cos(theta);
174174
const auto s = std::sin(theta);
175-
const matrix3<T> actual = rot3_x(theta);
175+
const matrix3<T> actual = rot3_x<T>(geo::radians{theta});
176176
CHECK(actual.xx == 1);
177177
CHECK(actual.xy == 0);
178178
CHECK(actual.xz == 0);
@@ -191,7 +191,7 @@ TEMPLATE_TEST_CASE("rot3_y()", "[matrix3]",
191191
const auto theta = g.generate<T>();
192192
const auto c = std::cos(theta);
193193
const auto s = std::sin(theta);
194-
const matrix3<T> actual = rot3_y(theta);
194+
const matrix3<T> actual = rot3_y<T>(geo::radians{theta});
195195
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
196196
CHECK(actual.xy == 0);
197197
CHECK_THAT(actual.xz, WithinAbs(s, epsilon<T>));
@@ -210,7 +210,7 @@ TEMPLATE_TEST_CASE("rot3_z()", "[matrix3]",
210210
const auto theta = g.generate<T>();
211211
const auto c = std::cos(theta);
212212
const auto s = std::sin(theta);
213-
const matrix3<T> actual = rot3_z(theta);
213+
const matrix3<T> actual = rot3_z<T>(geo::radians{theta});
214214
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
215215
CHECK_THAT(actual.xy, WithinAbs(-s, epsilon<T>));
216216
CHECK(actual.xz == 0);

‎src/pstack/geo/test/matrix4_ut.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ TEMPLATE_TEST_CASE("rot4_x()", "[matrix4]",
117117
const auto theta = g.generate<T>();
118118
const auto c = std::cos(theta);
119119
const auto s = std::sin(theta);
120-
const matrix4<T> actual = rot4_x(theta);
120+
const matrix4<T> actual = rot4_x<T>(geo::radians{theta});
121121
CHECK(actual.xx == 1);
122122
CHECK(actual.xy == 0);
123123
CHECK(actual.xz == 0);
@@ -143,7 +143,7 @@ TEMPLATE_TEST_CASE("rot4_y()", "[matrix4]",
143143
const auto theta = g.generate<T>();
144144
const auto c = std::cos(theta);
145145
const auto s = std::sin(theta);
146-
const matrix4<T> actual = rot4_y(theta);
146+
const matrix4<T> actual = rot4_y<T>(geo::radians{theta});
147147
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
148148
CHECK(actual.xy == 0);
149149
CHECK_THAT(actual.xz, WithinAbs(s, epsilon<T>));
@@ -169,7 +169,7 @@ TEMPLATE_TEST_CASE("rot4_z()", "[matrix4]",
169169
const auto theta = g.generate<T>();
170170
const auto c = std::cos(theta);
171171
const auto s = std::sin(theta);
172-
const matrix4<T> actual = rot4_z(theta);
172+
const matrix4<T> actual = rot4_z<T>(geo::radians{theta});
173173
CHECK_THAT(actual.xx, WithinAbs(c, epsilon<T>));
174174
CHECK_THAT(actual.xy, WithinAbs(-s, epsilon<T>));
175175
CHECK(actual.xz == 0);

‎src/pstack/gui/transformation.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class transformation {
1818
}
1919

2020
void rotate_by(float rx, float ry) {
21-
_orientation = geo::rot4_y(ry) * geo::rot4_x(rx) * _orientation;
21+
_orientation = geo::rot4_y<float>(geo::radians{ry}) * geo::rot4_x<float>(geo::radians{rx}) * _orientation;
2222
_recalculate();
2323
}
2424
void scale_mesh(float factor) {

0 commit comments

Comments
 (0)