Skip to content
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.

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

Quaternion: Extract yaw, pitch, and roll #38567

Open
Ramobo opened this issue Jun 29, 2020 · 10 comments
Open

Quaternion: Extract yaw, pitch, and roll #38567

Ramobo opened this issue Jun 29, 2020 · 10 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics
Milestone

Comments

@Ramobo
Copy link

Ramobo commented Jun 29, 2020

Background and Motivation

Essential for turning rotation outputs into non-black-wizard-mathematician-friendly counterparts.

Proposed API

namespace System.Numerics
{
    public class Quaternion : IEquatable<Quaternion>
    {
+        public void ExtractYawPitchRoll(out float yaw, out float pitch, out float roll);
    }
}

Since the inputs for Quaternion.CreateFromYawPitchRoll(...) are in radians, the outputs for the proposed method should also be in radians.

Usage Examples

Vector3 input = new Vector3(10f, 20f, 30f) * MathF.DegreesToRadians;

Quaternion quaternion = Quaternion.CreateFromYawPitchRoll(input.Y, input.X, input.Z);

quaternion.ExtractYawPitchRoll(out float yaw, out float pitch, out float roll);
Vector3 output = new Vector3(pitch, yaw, roll);

Uses a constant from #38566.

public Vector3 RotationRadians
{
    get =>
    {
        Rotation.ExtractYawPitchRoll(out float yaw, out float pitch, out float roll);

        return new Vector3(pitch, yaw, roll);
    }

    set => Rotation = Quaternion.CreateFromYawPitchRoll(value.Y, value.X, value.Z);
}

Alternative Designs

  • Return Vector3 instead. I heard that you can't since you have to be coordinate system-agnostic.

Risks

None.

@Ramobo Ramobo added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 29, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jun 29, 2020
@Ramobo
Copy link
Author

Ramobo commented Jun 29, 2020

Beep boop.

Tagging subscribers to this area: @tannergooding (according to the label assigned to #38566)
Notify danmosemsft if you want to be subscribed.

@ghost
Copy link

ghost commented Jun 29, 2020

Tagging subscribers to this area: @tannergooding
Notify danmosemsft if you want to be subscribed.

@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jun 29, 2020
@tannergooding tannergooding added this to the Future milestone Jun 29, 2020
@LEI-Hongfaan
Copy link

I prefer PitchYawRoll. Or add all their friends? (RollPitchYaw, PitchYawRoll, PitchYawPitch, YawRollYaw, ...)

@Ramobo
Copy link
Author

Ramobo commented Jun 30, 2020

There's no point in having that whole family. Although I would prefer +X right, +Y up, +Z forward, the order I chose keeps the convention of Quaternion.CreateFromYawPitchRoll(...). If you need it in another order, just initialize the vector as in the second example:

quaternion.ExtractYawPitchRoll(out float yaw, out float pitch, out float roll);
new Vector3(pitch, yaw, roll);

@LEI-Hongfaan
Copy link

LEI-Hongfaan commented Jul 7, 2020

@Ramobo Sorry for the late reply.

keeps the convention of Quaternion.CreateFromYawPitchRoll(...)

I see your point. It's great to have a counterpart of CreateFromYawPitchRoll.

If you need it in another order, just initialize the vector as in the second example

Due to a disagreement on terminology, it may be simply wrong.

In general,

(*Mathematica*)
EulerMatrix[{p, q, r}, {2, 1, 3}] != EulerMatrix[{q, p, r}, {1, 2, 3}]
EulerMatrix[{p, q, r}, {2, 1, 3}] != RollPitchYawMatrix[{p, q, r}, {2, 1, 3}]

CreateFromYawPitchRoll corresponds to EulerMatrix[(**), {2, 1, [3}]. [EulerMatrix] So if one means always 'yaw, pitch, roll' in EulerMatrix[(**), {2, 1, 3}] by 'yaw, pitch, roll', it could be fine. Otherwise, the order matters and the name may be ambiguous. (Taking coordinate handedness into account, we are going to have 48 variants in total.)

BTW, I think CreateFromRotationIntrinsicYXZ(float yaw, float pitch, float roll) is better in the first place because having 'yaw, pitch, roll' in both method name and parameter names seems redundant. And sadly, a remark like DirectXMath's is missing from the documentation.

@Ramobo
Copy link
Author

Ramobo commented Jul 7, 2020

...

Sorry, I don't understand. I'm not a matrix and quaternion black wizard or whatever, I just want to be done with core graphics already.
My point is: X, Y, and Z are ambiguous terms; yaw, pitch, and roll aren't. We should never assume coordinate systems in something this general-purpose, and that includes handedness. They're already failing at that by wanting to follow XNA conventions with very few options for anything else. Hell, it's Microsoft's framework, and it doesn't follow the conventions from Microsoft's own Direct3D.

And sadly, a remark like DirectXMath's is missing from the documentation.

Also sadly, everything is right-handed with no left-handed methods. I'm up for a proposal to add left-handed methods. I literally had to copy some methods from SharpDX to get a left-handed view and projection matrix, and the pitch was still counter-clockwise while the other two were clockwise. .NET math is neglected in general.

@LEI-Hongfaan
Copy link

Your Proposed API is good. But some part of Usage Examples could be misleading. They are mathematical black magic. You said that.

My key point is that the yaw in YawPitchRoll and the yaw in PitchYawRoll are not equal in general cases (if you take my terminology). The Vector3 in your example is merely a storage device. There is no point to change 'yaw, pitch, roll' from/to 'pitch, yaw, roll' in those lines and by this specific means. (The handedness is a negligible problem here.)

X, Y, and Z are definitely unambiguous because they are the field/component names of Vector3 and Quaternion and the most common axis names.

Of course, I can say it's Microsoft's fault. I just wondering why we did not provide a pair of methods (CompressToVector3/ExtractFromVector3) if we wanted a not-more-than one-Float-field-saved compacted form of Quaternion. YawPitchRoll is obviously more than a compacted form and has easy-to-see meanings.

My apologies. I just realize that what I really want to do is to protest the already-done approval of the poor-documented API CreateFromYawPitchRoll by dotnet team. Oh, it is a crime for killing (lame joke, haha) hours (per person) of time of some unlucky and compulsive developers to debug/test/verify whether or not they use this API correctly for importing/exporting model data or dong conversions. And here is a wrong place. This is my fault.

@Ramobo
Copy link
Author

Ramobo commented Jul 7, 2020

(The handedness is a negligible problem here.)

I agree that it is with what you're talking about, but it must absolutely be considered in general.

X, Y, and Z are definitely unambiguous because they are the field/component names of Vector3 and Quaternion and the most common axis names.

By ambiguous, I mean: If you say "X axis", what does that mean? Is that yaw, pitch, or roll? Yes, there are coordinate systems where X is yaw.

I agree with the rest.

@LEI-Hongfaan
Copy link

By ambiguous, I mean: If you say "X axis", what does that mean? Is that yaw, pitch, or roll?

"X axis" means the axis generated/specified by Vector3.UnitX. This definition does not depend on the graphics system.

Matrix4x4 m1 = Matrix4x4.CreateFromQuaternion(Quaternion.CreateFromYawPitchRoll(argle1, argle2, argle3);
Matrix4x4 m2 =
    Matrix4x4.CreateFromAxisAngle(Vector3.UnitZ, /*dotnet's roll*/argle3) *
    Matrix4x4.CreateFromAxisAngle(Vector3.UnitX, /*dotnet's pitch*/argle2) *
    Matrix4x4.CreateFromAxisAngle(Vector3.UnitY, /*dotnet's yaw*/argle1);

Noted that m1 and m2 are the same up to numeric errors. In fact, dotnet and the majority of its friends follow the same convention. So we can just define pitch/yaw/roll to be the amount of rotation around the X/Y/Z axis (right-handed). By this way, no specific coordinate system of graphics system is involved. We are purely working on the natural coordinate system, which comes from Vector3 itself. The downside is that not all libraries follow the convention. OpenTK is a notable one.

/*OpenTK*/Matrix4 m3 = Matrix4.CreateFromQuaternion(/*OpenTK*/Quaternion.FromEulerAngles(argle1, argle2, argle3));
Matrix4x4 m4 =
    Matrix4x4.CreateFromAxisAngle(Vector3.UnitX, /*OpenTK's roll*/argle3) *
    Matrix4x4.CreateFromAxisAngle(Vector3.UnitZ, /*OpenTK's pitch*/argle1) *
    Matrix4x4.CreateFromAxisAngle(Vector3.UnitY, /*OpenTK's yaw*/argle2);
// (What an odd re-ordering!) The matrices m3 and m4 should match.

For some one looking for an implementation

public static void CreateFromYawPitchRoll(Quaternion r, out float yaw, out float pitch, out float roll) {
    yaw = MathF.Atan2(2.0f * (r.Y * r.W + r.X * r.Z), 1.0f - 2.0f * (r.X * r.X + r.Y * r.Y));
    pitch = MathF.Asin(2.0f * (r.X * r.W - r.Y * r.Z));
    roll = MathF.Atan2(2.0f * (r.X * r.Y + r.Z * r.W), 1.0f - 2.0f * (r.X * r.X + r.Z * r.Z));
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics
Projects
None yet
Development

No branches or pull requests

5 participants