This what the mouse rotation looks like on the surface3d.py example, before the patch (play the movie):

(you can see the figure sometimes rotates, when you'd think it should tilt; or it moves in the opposite direction as the mouse).
After the patch:

(Now the movement is much more consistent: the figure tilts up/down, left/right if you move the mouse up/down, left/right near the center; near the edge, the mouse rotates the figure).

I don't have any problem with the proposed change, but wonder if others would.

If we use a private Quaternion class, please name _Quaternion so we don't have to maintain it. Is numpy-quaternion actually a heavy dependency if you don't install numba or scipy? Or will it not install without those? One could imagine using quaternions elsewhere in the 3d code.

Is a "cardan" angle the same as an Euler angle? Why not use the more common name? So far as I can see in numpy-quaternion, they do not use the term "cardan".

Is a "cardan" angle the same as an Euler angle?

No, it is different. mplot3d's azimuth, elevation, roll angles are a kind of Tait-Bryan angles, not classic Euler angles; see also the wikipedia page on the subject. (It mentions Cardan angles too.) Furthermore, some signs (left or right handed) are different. So I considered that a different name was in order.

Why not use the more common name?

Because it is not the same thing. Euler angles are not simply a more common name, these are something else altogether.

So far as I can see in numpy-quaternion, they do not use the term "cardan".

They do have something to say about Euler and Tait-Bryan and other such angles, though - https://github.com/moble/quaternion/wiki/Euler-angles-are-horrible. The documentation of from_cardan_angles() does explain what the "cardan" angles are, in some detail, with this in mind.

Is numpy-quaternion actually a heavy dependency if you don't install numba or scipy?

I'd think so, considering that all that we really need is 1) quaternion multiplication, and 2) our funny angle<>quaternion conversion, which is not part of numpy-quaternion, so it would have to be added anyway.

One could imagine using quaternions elsewhere in the 3d code.

That would be a great idea, but it is for another PR.

This is really great, a much needed improvement. I agree with @jklymak that the quaternion class should be private, but wouldn't bother with adding an external dependency for a class less than 100 lines long.
As a minor point, I note that your quaternion class seems to be written to handle arrays of angles/of quaternions, but it may be simpler(?) to drop that and just work with scalars (and have a class with 4 scalar attributes w/x/y/z), which is the only case we need. (Not insisting of this, though.)

This is really great, a much needed improvement.

Thank you :)

I agree with @jklymak that the quaternion class should be private, but wouldn't bother with adding an external dependency for a class less than 100 lines long.

I'll try and stash it away then.

As a minor point, I note that your quaternion class seems to be written to handle arrays of angles/of quaternions,

Well it comes for free when using numpy, the only part where it is actually noticeable is in as_cardan_angles(), where it costs us three dots and an extra comma, if you will allow the indulgence then the class stays fully general.

but it may be simpler(?) to drop that and just work with scalars (and have a class with 4 scalar attributes w/x/y/z), which is the only case we need. (Not insisting of this, though.)

Simpler? You mean, you'd prefer your multiplication to be

    return Quaternion(
        (self.w * other.w) - (self.x * other.x) - (self.y * other.y) - (self.z * other.z),
        (self.w * other.x) + (self.x * other.w) + (self.y * other.z) - (self.z * other.y),
        (self.w * other.y) - (self.x * other.z) + (self.y * other.w) + (self.z * other.x),
        (self.w * other.z) + (self.x * other.y) - (self.y * other.x) + (self.z * other.w))

rather than

    return Quaternion(
        self.scalar*other.scalar - np.dot(self.vector, other.vector),
        self.scalar*other.vector + self.vector*other.scalar + np.cross(self.vector, other.vector))

Why would you consider the former simpler? A whole bunch of + and - signs that you have to take at face value (and indeed, some people refuse to, and define their quaternions with ijk=+1, "JPL convention" at NASA, apparently...)?
In contrast, describing a quaternion as a scalar together with a vector emphasizes meaning (the vector part naturally connects with rotations) rather than focusing on the representation: the cross product is not obscured by components x, y, z. And mistakes like ijk=+1 are avoided.

I would probably write

w0, x0, y0, z0 = self.w, self.x, self.y, self.z
w1, x1, y1, z1 = other.w, other.x, other.y, other.z

followed by formulas that directly match those at https://en.wikipedia.org/wiki/Quaternion#Hamilton_product, but again, I don't really mind. I can definitely see why you consider the scalar/vector representation more meaningful.

A side motivation for switching to scalars only is that numpy arrays tend to have enormous overheads (compared to python scalars, not to numpy scalars) for such small sizes, but performance hardly matters here.

Good to go now, hopefully...

I'd think so, considering that all that we really need is 1) quaternion multiplication, and 2) our funny angle<>quaternion conversion, which is not part of numpy-quaternion, so it would have to be added anyway.

Yes, I agree with this.

but it may be simpler(?) to drop that and just work with scalars (and have a class with 4 scalar attributes w/x/y/z), which is the only case we need. (Not insisting of this, though.)

I don't think this is so important as long as it's well documented. Could go either way, I think it's fine as-is and we can always change it later for a private class.

I would also consider adding two more methods that might be useful in the future, though maybe it doesn't make sense to add until/unless we need them:

• .inv() to invert quaternions
• .apply(v) to rotate a 3-vector v by the quaternion

I would also consider adding two more methods that might be useful in the future, though maybe it doesn't make sense to add until/unless we need them:

* `.inv()` to invert quaternions

* `.apply(v)` to rotate a 3-vector v by the quaternion

That's right - maybe later. Or if we would make the class public instead of private, then it might serve a purpose.

One additional suggestion I want to make is that I personally see the new behavior as strictly superior, but I can imagine some implementations where someone would want the old control scheme. I would suggest that we keep the old behavior as an option, accessible through an rcparam to allow users to switch. But I think this new mode is fine to make the default.

The idea has crossed my mind too. Presumably, it would be straightforward to implement. I already was adding more options (Holroyd's trackball, in addition to Shoemake's; also, you could consider the 'original' Shoemake's, at double the rotation rate; etc., etc.) But just because we can, does not mean we should. Maybe it is better to be brave, and make a choice (instead of getting lost in feeping creaturism).
We could see the PR in a different light: the trackball is not a enhancement - the gimbal lock is a bug that needs fixing.
But if I'm overlooking a compelling use case, maybe you can enlighten me.

+1 for later. If we don’t need it now, don’t bother with it now.

It should be good to go again (provided we can do without an rcparams for the trackball).

@MischaMegens2 Thanks for your contribution and the stamina to keep going through the somewhat lengthy review process. 🎉

It did get better :)

Looks good now. Anybody can merge after CI pass. - Please squash before merging.

Not sure about the CI - is there still an issue?

Got to play around with this finally, it feels good!

Without digging into the math to double check if the naming is right, it seems like this is matching up with "trackball" controls rather than "arcball" controls. See here for an example:

• https://threejs.org/examples/#misc_controls_trackball
• https://threejs.org/examples/#misc_controls_arcball

Personally I prefer the latter, since moving the mouse back the original point on the screen with arcball controls will return the view to the original view, whereas the trackball controls will seem to drift (ie, click and spin the mouse in a circle, and the view will precess in one direction or the other). But I know both are pretty common. @MischaMegens2 thoughts?

Got to play around with this finally, it feels good!

Glad that you liked it...

Without digging into the math to double check if the naming is right, it seems like this is matching up with "trackball" controls rather than "arcball" controls.

Well most certainly it is not matching up with the "trackball", the matplotlib control is indeed an "arcball". Just try the examples once more, paying attention to how it responds, rather than to how it looks superficially (the matplotlib arcball does not show the circles, like the threejs arcball):

• https://threejs.org/examples/#misc_controls_trackball
• https://threejs.org/examples/#misc_controls_arcball

In particular: drag the mouse tangentially near an edge or corner; the trackball will still change the 'tilt' (similar to what happens near the center of the figure), whereas an arcball will rotate the scene ('roll').

Personally I prefer the latter, since moving the mouse back the original point on the screen with arcball controls will return the view to the original view, whereas the trackball controls will seem to drift (ie, click and spin the mouse in a circle, and the view will precess in one direction or the other). But I know both are pretty common. @MischaMegens2 thoughts?

Well the philosophy is different. One might argue that that 'precession' of the trackball is a feature, rather than a bug - a feature that is much needed to be able to adjust the roll at all. It is just unfortunate that the figure then rotates in the opposite direction of the circular motion of the mouse...

As for the math of it: the 'arcball' as implemented does not strictly follow the arcball recipe of Ken Shoemake; Ken's arcball uses vectors on the (unit) sphere to directly define a rotation quaternion q = v1 v2', and then rotates the figure using v_rotated = q v_original q'. As a consequence, it rotates by twice the angle subtended on the sphere. (This is particurlarly noticeable when adjusting the 'roll', near the edge.) In contrast, the matplotlib arcball takes a square root: q = sqrt(v1 v2'), avoiding the double-speed rotation.

The behavior I was trying to get at isn't so much the tilt vs roll at the edge of the screen (doesn't matter so much IMO), but the property that during a single mouse click, returning to the original position on the screen will restore the original view. The threejs arcball has this property, but the matplotlib arcball doesn't right now and precesses:

The behavior I was trying to get at is [...] that during a single mouse click, returning to the original position on the screen will restore the original view. The threejs arcball has this property, but the matplotlib arcball doesn't right now and precesses.

Aha! I think this is an unfortunate consequence of the sqrt() that I threw in against the double-speed. I can see that this might not be desirable, and that Ken's original arcball has its advantages.
Shall I add an rcParams after all, then? And we can throw in a trackball option, and Holroyd's arcball as well, for good measure.
I think you have me convinced.
New issue#, new PR?

Think it's up to you if you want to change the default behavior or add rcparams! Since this feature hasn't been released in v3.10.0 yet, there's pretty wide latitude to make changes before that release. Yeah, new issue & PR I would think.