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

Quaternions #1796

Draft
wants to merge 50 commits into
base: master
Choose a base branch
from
Draft

Quaternions #1796

wants to merge 50 commits into from

Conversation

lhog
Copy link
Collaborator

@lhog lhog commented Dec 3, 2024

No description provided.

@saurtron
Copy link
Collaborator

saurtron commented Dec 3, 2024

Guess you already checked this and not the most critical thing atm, but since I looked it up, so it can serve as reference for later on:

Some possible mathematical simplifications for 4x4 det: blender code (not sure where their method comes from but seems to do less operations). Also for reference in case it can be useful, their quaternion code.

Also, from wikipedia:

$det\pmatrix{A & B \\ C & D} = det(D)det(A-BD^{-1}C))$

Being A B C D the 2x2 blocks. this would also possibly result in less operations.

There's lot of literature on the subject, specially re simd optimizations, but anyways those seem straighforward simplifications.

Looking great, maybe some tests would be in order?

@lhog
Copy link
Collaborator Author

lhog commented Dec 3, 2024

Looking great, maybe some tests would be in order?

I'm going to switch most of the engine internals to quaternions, so there will be lots of manual tests :)

@lhog lhog force-pushed the BAR-quat branch 5 times, most recently from 0bb4f3f to cb56e17 Compare December 15, 2024 01:05
Comment on lines 20 to 21
auto mi = std::min_element(std::begin(xyz), std::end(xyz), [](const auto& a, const auto& b) { return math::fabs(a) < math::fabs(b); });
auto Mi = std::max_element(std::begin(xyz), std::end(xyz), [](const auto& a, const auto& b) { return math::fabs(a) > math::fabs(b); });
Copy link
Collaborator

@sprunk sprunk Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the opposite comparator? Won't it this result in mi and Mi being the same? If there are two minimums then it will still return the same one in both. See https://godbolt.org/z/cdPd4h7Pe

If this really is just looking for min and max then consider auto [mi, Mi] = std::minmax_element(...) to look for both at the same time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh lol, you're right, nice catch!

lhog added 29 commits December 15, 2024 20:53
…del pieces.

Non-uniform scaling doesn't produce affine transformation matrices upon their multiplication,
nor could be represented by {rotation, translation, scaling} tuple,
shouldn't be a big deal as non-uniform scaling was never supported by the engine in the first place
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

Successfully merging this pull request may close these issues.

3 participants