Revert rotation order and document.#1514
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1514 +/- ##
=======================================
Coverage 92.41% 92.41%
=======================================
Files 35 35
Lines 6024 6024
=======================================
Hits 5567 5567
Misses 457 457 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
elalish
left a comment
There was a problem hiding this comment.
Thanks - guess I reviewed that last one a little too quickly.
| const vCloseTo = (a: Vec3, b: Vec3, margin = 1): boolean => | ||
| vdiff(a, b) <= margin; | ||
| const vContains = (haystack: Array<Vec3>, needle: Vec3): boolean => | ||
| !!haystack.find(x => vCloseTo(needle, x)); |
There was a problem hiding this comment.
I feel like I've seen this code before - is this fodder for a shared helper function somewhere?
There was a problem hiding this comment.
Yep. Put the distance function in lib/math.ts, and the more test specific stuff in test/util.ts.
| // Some quick vector math to check our results. | ||
| const vdiff = (a: Vec3, b: Vec3): number => | ||
| Math.sqrt(a[0] - b[0]) ** 2 + (a[1] - b[1]) ** 2 + (a[2] - b[2]) ** 2; | ||
| Math.sqrt((a[0] - b[0]) ** 2 + (a[1] - b[1]) ** 2 + (a[2] - b[2]) ** 2); |
There was a problem hiding this comment.
oops. Much as I enjoy JS, the utter lack of a decent vector library is annoying.
| * Additionally, more efficient code paths are used to update the manifold | ||
| * when the transforms only rotate by multiples of 90 degrees. This operation | ||
| * can be chained. Transforms are combined and applied lazily. | ||
| * Applies an Euler or Tait-Bryan angle rotation to the manifold. This |
There was a problem hiding this comment.
Who are Tait and Bryan? I've only ever heard these called Euler angles.
| const layFlat = new GLTFNode(); | ||
| layFlat.rotation = | ||
| [0, 90-(Math.atan(1 / Math.sqrt(2)) * 180 / Math.PI), 135]; | ||
| [45, -(Math.atan(1 / Math.sqrt(2)) * 180 / Math.PI), 120]; |
There was a problem hiding this comment.
Okay, so this is back to how it was originally?
There was a problem hiding this comment.
Yeah, yeah it is. 😭
| * Rotations are applied in `XYZ` order. | ||
| * That is roll first, then pitch and finally yaw. | ||
| * From the reference frame of the model being rotated, rotations are applied | ||
| * in *z-y'-x"* order. That is yaw first, then pitch and finally roll. |
There was a problem hiding this comment.
I don't quite follow this. I agree it's global X,Y,Z (following OpenSCAD's convention), but I don't see how that relates to roll, pitch, and yaw. Those are defined when rotating by the transformed axes, rather than the global axes.
f2a6de9 to
b3d5d2f
Compare
|
Oh awesome. I was gonna back out a little more of the documentation stuff -- Going back over it, I must have gotten a little over-excited. There's a lot of potential for ambiguity in how this stuff is described. Anyhow, Tait-Bryan angles are a subset of Euler angles where each rotation is about a different axis. E.g.: |
Fixes changes made in #1505, and clarifies rotation order. Also, adds tests to ensure manifold rotation matches GLTFNode rotation.
Mentioned in #1507.