Java swizzle and constant fixes #972
Conversation
| * | ||
| * @param inputSwizzle The swizzle | ||
| */ | ||
| public void setInputSwizzle(char[] inputSwizzle) { | ||
| char defaultSwizzle[] = new char[4]; |
There was a problem hiding this comment.
Does Java initialize new arrays to all 0? Otherwise initialization is needed.
There was a problem hiding this comment.
Yes, all array components are initialized to be 0.
https://docs.oracle.com/javase/specs/jls/se23/html/jls-4.html#jls-4.12.3 :
Array components are unnamed variables that are created and initialized to default values
(link->) For type char, the default value is the null character, that is,'\u0000'.
| throw new IllegalArgumentException("The inputSwizzle may only consist of 'rgba01', but contains " + c); | ||
| } | ||
| } | ||
| char defaultSwizzle[] = new char[4]; |
There was a problem hiding this comment.
Same question as for the AstcParams case.
There was a problem hiding this comment.
Same answer as above.
With the addendum: I also didn't like the duplication, and considered to pull out this block into a function, so that the 'setter' could boil down to something like
this.inputSwizzle = Swizzling.validate(inputSwizzle);
This would also allow to avoid the duplication of the unit tests (which may be a stronger case to make).
I somehow hesitate(d) to create such a class Swizzling just for this purpose, but will have another look: There might be other duplicated blocks that could justify moving this into some class Utils that does some of the parameter checks.
There was a problem hiding this comment.
So I think that, in this case, it makes sense to move this to a common function. So I just did that, and there now is a KtxUtilities.validateSwizzle function. This avoids the duplication in the BasisParams/AstcParams themself, and it avoids the duplication of the unit tests.
There was a problem hiding this comment.
This avoids the duplication in the BasisParams/AstcParams themself, and it avoids the duplication of the unit tests.
Sounds good. I'll be away the next 3 days. I'll review again on my return.
Fixes #969 (see the issue for details).
The fix for the
inputSwizzle(for both theKtxAstcParamsandKtxBasisParams) is implemented by checking whether the input equals the default, which is achar[] { 0, 0, 0, 0 }array. If this is the case, then this value will be accepted, passed to the native layer, and handled there accordingly. Only when the input is not that default, the elements will be checked to be inrgba01.