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

Likely bug in Quaternion.copyRotationInto #324

Open
robertmuth opened this issue May 31, 2024 · 3 comments
Open

Likely bug in Quaternion.copyRotationInto #324

robertmuth opened this issue May 31, 2024 · 3 comments

Comments

@robertmuth
Copy link

robertmuth commented May 31, 2024

For this code:

import 'package:vector_math/vector_math.dart' as VM;

void TestMe(VM.Quaternion dir) {
  print("quad: ${dir}");
  VM.Matrix3 rot = VM.Matrix3.identity();
  VM.Vector3 x = VM.Vector3.zero();
  x.setValues(0.0, 1.0, 0.0);
  dir.rotate(x);
  print("Direction from quad: ${x}");
  //
  dir.copyRotationInto(rot);
  x.setValues(0.0, 1.0, 0.0);
  rot.transform(x);
  print("Direction from rot: ${x}");
}

I would expect the last two print statments to mostly agree but they do not, e.g.

quad: -0.3830223083496094, -0.3213938772678375, -0.6634140610694885 @ -0.5566705465316772
Direction from quad: [0.9848082065582275,-0.1736481785774231,-4.0803353584806246e-8]
Direction from rot: [-0.49240386486053467,-0.17364810407161713,0.8528685569763184]

There also seems to be no test for this in the test suite

@robertmuth
Copy link
Author

It could be that I am merely misunderstanding the API instead of

dir.rotate(x);

using

x.applyQuaternion(dir);

seems to work.

If that is indeed the case, maybe the documentation could be updated.

I also have a related feature request: quaternion multiplication always forces a allocation.
It would be nice tio have a version that multiplies into a given (empty) quaternion.

@notakshitgoyal
Copy link

notakshitgoyal commented Dec 1, 2024

void TestMe(VM.Quaternion dir) {
  print("quad: ${dir}");
  VM.Matrix3 rot = VM.Matrix3.identity();

  VM.Vector3 x = VM.Vector3.zero();
  x.setValues(0.0, 1.0, 0.0);
  dir.rotate(x);
  x.normalize();  // Normalize after quaternion rotation
  print("Direction from quad: ${x}");

  dir.copyRotationInto(rot);
  x.setValues(0.0, 1.0, 0.0);
  rot.transform(x);
  x.normalize();  // Normalize after matrix transformation
  print("Direction from rot: ${x}");
}

@robertmuth
Copy link
Author

Could you add a little more "prose" to the answer - people that look at this in a couple of years will be thankful.

E.g.:

Why do you have to normalize? Is this a well known math fact? If so, provide a link.
Or maybe this is only necessary for a certain class of Quaternions. If so, how can one tell?

Also, what is the recommended course of action: do nothing, add the normalization step to the library, add a warning to the library documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants