Satin API

Satin API

3M Downloads

[API] Using UniformMat4 as a float array

lonefelidae16 opened this issue · 6 comments

commented

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

https://github.com/lonefelidae16/Blur/blob/09ab30c24b3c8544881df559b7f72b1896624514/src/main/java/com/tterrag/blur/Blur.java#L64-L66

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

commented

I went ahead and opened a PR based on your commit; would it do ?

commented

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.

commented

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.

commented

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!

commented

Oh no, actually I just realized I fucked up - default methods won't prevent clashes for other methods with the same name

commented

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.