-
Notifications
You must be signed in to change notification settings - Fork 106
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
Optimize Vector Multiplication? #337
Comments
Thanks for suggesting! I've done some benchmarking to assess both the benefits as well as the possible performance degradation (from people actually using generic matrices with unknown properties in which case the multiple if checks checking for the various matrix properties will add unnecessary overhead) and I think the benefits likely outweigh the disadvantages. The next JOML 1.10.6-SNAPSHOT will have this performance optimization in place. In addition, the "generic" and specific multiplication methods will be exposed on the vector classes such that if people know what they are dealing with, they can directly use the most optimal method available. |
Thank you for the quick answer :) And that sounds like a good plan. Side topic. Is that normal? |
This is a very weird coincidence, and thanks for mentioning this: JOML actually has a serious bug in this regard. :D JOML/src/main/java/org/joml/Matrix4f.java Line 5317 in 8773210
But, of course, this should not be the case, and will be fixed with the next 1.10.6-SNAPSHOT. Thanks for mentioning it! :) |
Eh you missunderstood me. Matrix4f mat = new Matrix4f();
for(int i = 0;i<100;i++) {
mat.rotateX(22F); //Rotate by 22Degree a few hundred times, that includes quaternions or Eula
}
mat.getScale(new Vector3f()); The scale will be all 0 or in some cases 1 on one specific axis. And there was no scale applied at all, the scale information will be just lost when you rotate a matrix, in that sense that you can not know the scale itself at any point. |
rotate methods do not rotate by degrees. All angles in all methods are radians, not degrees. See: https://github.com/JOML-CI/JOML/wiki/Common-Pitfalls#angles-are-always-in-radians |
If you have a situation that results in |
This was just a example i came up with on the fly the "bug/rounding errors" test i did was a few days/weeks ago.
Ok let me test it.
Yeah if the rounding errors are small that is fine. The issue is that they are not that small then you think. This is the code example: public void test() {
Vector3f vector = new Vector3f();
Matrix4f jom = new Matrix4f();
for(int i = 0;i<100;i++) {
jom.rotate((float)Math.toRadians(12), 1F, 0F, 0F).getScale(vector);
System.out.println("Index="+i+", Scale="+vector);
}
} The output: ClickMe
Now this is the output if the code looks like this. (Changed 1F to 0.1F) public void test() {
Vector3f vector = new Vector3f();
Matrix4f jom = new Matrix4f();
for(int i = 0;i<100;i++) {
jom.rotate((float)Math.toRadians(12), 0.1F, 0F, 0F).getScale(vector);
System.out.println("Index="+i+", Scale="+vector);
}
} The output! ClickMe
As you can see the moment you don't provide 1F to the rotation axis it will just lose all scale. |
Yes, this is expected, by design and thoroughly documented as JavaDoc on the respective methods:
The axis vector in a rotate() method must be normalized/unit. So, you cannot rotate using the axis
No, the second, third and fourth parameter of https://javadoc.io/doc/org.joml/joml/latest/org/joml/Matrix4f.html#rotate(float,float,float,float) define the axis of rotation. Not the Euler angles around X, Y, and Z. If you want to rotate using Euler angles there are the methods |
So then why allow floats in the first place instead of boolean flags/enum? Also sorry if i ask to much. I am just trying to understand this ^^ |
You can specify the axis of the rotation using floats (or a 3d vector using an overload of the rotate() method that takes the axis as a vector). Obviously, you can use other axes other than the primary axes (1, 0, 0), (0, 1, 0) and (0, 0, 1) (for example |
Ok, so anything but the primary axis will mess with the scale. |
As I said above: The rotation axis needs to be unit (normalized - have a length of one). Any axis is fine, so long as its length is 1. If you use a
Yes, since none of these two axes have a length of 1. |
Ok yeah i feel really dumb..... Found also out that i need to fix the rounding issues in "MY implementation" since yours is still a lot more accurate but that is something i can work with :) Anyways, thank you for this I think algebra class? It gave me a greater understanding of stuff i didn't really understand before. (Clearly) Again thank you, and sorry for the wasted time i caused ^^" |
Sure, no problem. All fine. :) |
One might now ask, why So, it was a consideration between performance vs. least-surprise/astonishment in API design, in the favor of performance. This problem could probably have been solved better by providing two different methods (one which always does the normalization and one which expects the axis to be normalized in the first place). |
Ok, i was until now under the assumption this was the same in all implementations out there. |
…properties (#337) - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties - mulPosition() now takes into account identity, translation and affine - new method mulPositionTranslation() when assuming translation - new method mulPositionAffine() when assuming affine - new method mulPositionGeneric() when not assuming (and also not testing for) properties
…properties (#337) - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties - mulPosition() now takes into account identity, translation and affine - new method mulPositionTranslation() when assuming translation - new method mulPositionGeneric() when not assuming (and also not testing for) properties
…properties (#337) - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties - mulPosition() now takes into account identity, translation and affine - new method mulPositionTranslation() when assuming translation - new method mulPositionGeneric() when not assuming (and also not testing for) properties
@httpdigest you ran into a small pitfall yourself right now. A fix for that would be: (Yeah i have done this optimization already once) |
I don't know what you mean.
If by "GuesstimateProperties" you mean the Therefore, we first check for affinity (because that has the least restrictions). After determining affinity, we can then proceed to check if it is only a translation (more restrictions) or even identity (the most amount of restrictions). |
I was about to write a bunch of things. |
Alright, no problem. |
…properties (#337) - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties - mulPosition() now takes into account identity and translation - new method mulPositionTranslation() when assuming translation - new method mulPositionGeneric() when not assuming (and also not testing for) properties
…properties (#337) - mul() now takes into account identity, translation and affine - new method mulTranslation() when assuming translation - new method mulGeneric() when not assuming (and also not testing for) properties - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties
…properties (#337) - mul() now takes into account identity, translation and affine - new method mulTranslation() when assuming translation - new method mulGeneric() when not assuming (and also not testing for) properties - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties
…properties (#337) - mul() now takes into account identity, translation and affine - new method mulTranslation() when assuming translation - new method mulGeneric() when not assuming (and also not testing for) properties - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties - new method mulGenericTranspose() when not assuming (and also not testing for) properties
…properties (#337) - mul() now takes into account identity, translation and affine - new method mulTranslation() when assuming translation - new method mulGeneric() when not assuming (and also not testing for) properties - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties - new method mulGenericTranspose() when not assuming (and also not testing for) properties
…properties (#337) - mul() now takes into account identity, translation and affine - new method mulTranslation() when assuming translation - new method mulGeneric() when not assuming (and also not testing for) properties - mulProject() now takes into account identity, translation and affine - new method mulProjectTranslation() when assuming translation - new method mulProjectAffine() when assuming affine - new method mulProjectGeneric() when not assuming (and also not testing for) properties - new method mulGenericTranspose() when not assuming (and also not testing for) properties
JOML is really optimized, and I really enjoy using it and learning from it.
But i noticed one thing with Matrix Multiplications on vectors.
While the Matrix has so many properties, they are actually never used with Vectors.
JOML/src/main/java/org/joml/Vector4f.java
Line 940 in 8773210
Since the Matrix has Properties: Such as "Identity" and "Translation".
Why not make use of them:
If the matrix is a Identity, do nothing.
if the matrix is a translation only then only translate it.
This would reduce operations a LOT:
This would also optimize game engines a lot, minecraft too, since they do a TON of Matrix multiplications on the CPU in the first place, and having a ton of them reduced to identity or translation ONLY. That would be awesome!
Edit:
After reading more about the flags. Only Affine is really used, the others don't really indicate what they are.
The text was updated successfully, but these errors were encountered: