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

Fix MatrixHelper producing slightly different results from vanilla #2169

Merged
merged 2 commits into from
Nov 19, 2023

Conversation

embeddedt
Copy link
Contributor

Vanilla uses JOML's Vector4f#mul or Vector3f#mul methods to apply matrix transformations to vectors. The implementation of these methods uses calls like

dest.x = Math.fma(mat.m00(), x, Math.fma(mat.m10(), y, Math.fma(mat.m20(), z, mat.m30() * w)));

and the Math.fma wrappers have the effect of doing the final additions right-to-left, rather than left-to-right. Since Sodium's MatrixHelper uses left-to-right addition, the value it produces might differ very slightly from JOML's as a result of floating-point precision errors. This can cause Z-fighting.

The solution I used is to simply add brackets to force the additions to be performed in the same order as JOML.

Discord thread where the issue was reported: https://discord.com/channels/602796788608401408/1174666147828879400

The user and I both tested & confirmed this fixes their issue.

@jellysquid3
Copy link
Member

This bug description makes my bones hurt.

@jellysquid3
Copy link
Member

Jokes aside, the solution looks good, and it seems like our suspicions in Discord were correct as well. Thanks for the write-up.

@jellysquid3 jellysquid3 merged commit 0850b93 into CaffeineMC:dev Nov 19, 2023
@embeddedt embeddedt deleted the fix/matrixhelper-precision branch November 29, 2023 15:45
IMS212 pushed a commit to IMS212/sodium-fabric that referenced this pull request Aug 6, 2024
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.

2 participants