[API] Using UniformMat4 as a float array
lonefelidae16 opened this issue · 6 comments
Describe your suggestion
Since I’m faced with a situation where I need to use UniformMat4f
as a float array, I suggest a new overloading method with float[]
argument. This may decrease memory overhead by removing the need to create a Matrix4f
instance.
Additional context
current API call:
UniformMat4 targetMat4;
float[] floatArrayThat16Sized;
Matrix4f mat4 = new Matrix4f();
mat4.set(floatArrayThat16Sized);
targetMat4.set(mat4);
after with this suggestion:
UniformMat4 targetMat4;
float[] floatArrayThat16Sized;
- Matrix4f mat4 = new Matrix4f();
- mat4.set(floatArrayThat16Sized);
- targetMat4.set(mat4);
+ targetMat4.set(floatArrayThat16Sized);
Ready to create a pull: lonefelidae16@90b56b0
Oh sorry, should have looked at this earlier. Couldn't this situation be easily solved by having your generateGauss
method return a Matrix4f ?
On the other hand, such a method may indeed be necessary if we want to avoid extra operations in more specific contexts. The issue I can see is that the array option drops compile-time safety regarding the count of values being set, but this can be mitigated with a specific name and runtime checks.
Thanks for replying!
Couldn't this situation be easily solved by having your
generateGauss
method return a Matrix4f ?
Yeah that’s right, storing them as Matrix4f instead of float[] is one of the easier solutions. but it’ll make easy to modify the values by calling its member method, so to avoid confusing others, I thought I want to indicate that it’s difficult to change the values of array. It doesn’t need to behave as Matrix4f.
was a public stance. I found a method in GlUniform that accepts float array, so I thought “Let’s patch it”, and it goes well xD
I thought I’d share it with you.
On the other hand, such a method may indeed be necessary if we want to avoid extra operations in more specific contexts. The issue I can see is that the array option drops compile-time safety regarding the count of values being set, but this can be mitigated with a specific name and runtime checks.
Ah... I’m absolutely sure. In this case, if the size of the float array is 15 or lesser, a warning is mass-produced by GlUniform. Conversely, if a size 17 or greater is passed, an IndexOutOfBoundsException is thrown from java.util.Objects#checkFromIndexSize. A check mechanism to prevent this from happening may indeed be necessary.
Looks so good! and I could only think of was to implement a new method to add to ManagedUniform that only supports Mat4, I hadn’t thought of casting itself within a method with the default keyword. this would bring availability to other Uniforms facing the same situation.
Compiled and tested, it works well!
Oh no, actually I just realized I fucked up - default methods won't prevent clashes for other methods with the same name
Oh, can confirm, default methods seem to have such a limitation... adding the same method name to Uniform4f didn’t compile. Even if I make an interface into an abstract class to have partial instance methods, Java doesn’t allow to extend multiple abstract classes.
I tested with the commit ce809ca and all is good! It seems that a compile-time check for shader assets has been added. and having a count field in ManagedUniform solves the default method issue.