Fix issues with quaternion slerp

[jure]

Thanks for this wonderful library!

When using quaternion slerping I found some issues/erratic rotation when x,y,z,w change signs with the current slerp implementation. I then tried the Three.js slerp implementation and that works without issues - perhaps we could simply replace the current one?

I'm sure the issue is minimal in the current implementation, but also I'm not keen on debugging it if a working version is readily available.

I think a copy of the Three.js MIT license would have to be added to that piece of code, though.

[jure]

Ah, interesting! The tests are failing - maybe that makes the debugging easier then. From what I can see the new code works without glitches in the actual app.

FAIL test/lerp.test.js
  ● quatSlerp 2

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0.5

      44 |   const qb = { x: 0.5, y: 0.5, z: 0.5, w: 0.5 }
      45 |   const q = quatSlerp(qa, qb, 0.5)
    > 46 |   expect(q.w).toBe(1)
         |               ^
      47 | })
      48 | 
      49 | test('quatSlerp 3', () => {

      at Object.<anonymous> (test/lerp.test.js:46:15)

  ● quatSlerp 3

    expect(received).toBe(expected) // Object.is equality

    Expected: 0.999999995
    Received: 1

      51 |   const qb = { x: 0, y: 1, z: 0, w: 0 }
      52 |   const q = quatSlerp(qa, qb, 0.5)
    > 53 |   expect(q.y).toBe(0.999999995)
         |               ^
      54 | })
      55 | 

[jure]

One of the issues is actually mentioned here http://www.euclideanspace.com/maths/algebra/realNormedAlgebra/quaternions/slerp/

This is missing:

if (cosHalfTheta < 0) {
  qb.w = -qb.w; qb.x = -qb.x; qb.y = -qb.y; qb.z = qb.z;
  cosHalfTheta = -cosHalfTheta;
}

But even with this added, it has some glitches, so I think some normalization is additionally missing. Since this is orthogonal to what I'm trying to achieve I wouldn't like to spend more time fixing a slerp at this point :)

An alternative solution, one that doesn't require adding Three.js code, could be if the API is opened up just a bit to accept a custom interpolation function for quaternions.

[yandeu]

I think the test is only failing because I hardcoded the results.

I will try to test and compare both codes in a three.js app.

I'm pretty sure three's implementation is right.

[codecov-io]

Codecov Report

Merging #8 (b9562ab) into master (263abd5) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master        #8   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         5    +1     
  Lines          192       201    +9     
  Branches        47        51    +4     
=========================================
+ Hits           192       201    +9     

Impacted Files	Coverage Δ
src/lerp.ts	100.00% <ø> (ø)
src/slerp.ts	100.00% <100.00%> (ø)
src/snapshot-interpolation.ts	100.00% <100.00%> (ø)
src/vault.ts	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 263abd5...b9562ab. Read the comment docs.

[jure]

I've moved this slerp into a separate file so that it can have the Three.js MIT license specified at the top of the file, and there's no misunderstanding what code has what license. I believe that's the correct way to include this snippet.

I've also updated the tests and used a real quat example + cross-checked it with THREE.Quaternion.slerp to see that there is no difference between slerp and slerpFlat.

This now works great for me, but I understand if you have license considerations and want to fix your own slerp instead.

[yandeu]

I've moved this slerp into a separate file so that it can have the Three.js MIT license specified at the top of the file, and there's no misunderstanding what code has what license.

Perfect! I would have done it the same way. But I would use a different format. With the format below, the copyright will be copied to the bundle when using webpack.

/**
 * @author        three.js authors
 * @copyright     Copyright © 2010-2021 three.js authors
 * @license       {@link https://github.com/mrdoob/three.js/blob/dev/LICENSE|MIT}
 * @description   Copied and modified from: https://github.com/mrdoob/three.js/blob/464efc85ecfda5c03d786d15d8f8eff20d70f256/src/math/Quaternion.js
 */

[jure]

That's very neat! Fixed it, and added a final speed optimization (see https://github.com/mrdoob/three.js/pull/21183).

[yandeu]

Looks great! Thanks for contributing!

[jure]

Thank you for the kind words and the merge, @yandeu! I see it's been released as well, awesome.