Issues with get_texture_unit functions

[MoritzBrueckner]

As mentioned on the Discord server today, there are some issues with kinc_g4_pipeline_get_texture_unit() and kinc_compute_shader_get_texture_unit() when using array uniforms, at least on D3D11.

• Currently, both these functions expect the index passed in the name parameter to have only one digit:

  // This works...
  kinc_compute_shader_get_texture_unit(&shader, "myUniform[9]");
  
  // ...this doesn't
  kinc_compute_shader_get_texture_unit(&shader, "myUniform[10]");

  I guess it's just a matter of finding the opening bracket with strchr(), replace the closing one with a zero byte and then use atoi(). However, as discussed on Discord, it might make more sense to change the API a little bit, for example there could be texture unit functions specifically for arrays with an additional int index parameter. In Kha for example, it could work to have just one function with an optional index parameter that defaults to 0 or -1, but I'm not sure whether that really makes sense.

• Depending on what array uniforms are used in the shader, the uniform names in the generated .d3d11 file sometimes contain array brackets and sometimes they don't. However, both functions mentioned above currently don't expect any brackets in those names, and thus the hash comparison between the name stored in the d3d11 file and the name parameter will fail (square brackets are always removed from the name parameter before comparison) in some cases.

  Example shader:

  #version 450
  
  in vec2 texcoord;
  
  layout(rgba32f, binding = 0) uniform readonly image2D teximage[2];
  uniform sampler2D texsampler[2];
  
  out vec4 fragColor;
  
  void main() {
  	vec4 color = texture(texsampler[0], texcoord) + imageLoad(teximage[0], ivec2(0, 0));
  	fragColor = vec4(color.r, color.g, color.b, 1.0);
  }

  If you comment out the teximage stuff, there are no square brackets for the texcoord sampler in the d3d11 file [1]. If you use the shader as it is written above, both teximage and texsampler will have square brackets in the d3d11 file [2]. If you use only an image2D array there are brackets as well.

  [1] sampler2D array only:
  

  [2] both sampler2D and image2D array
  

  The screenshots are from HxD, "Dekodierter Text" translates to "decoded text" and refers to the utf8 decoding of the binary data.

• The d3d11 file contains all sampled indices from the shader. The following line for example

  vec4 color = texture(texsampler[0], texcoord) + texture(texsampler[1], texcoord) + imageLoad(teximage[0], ivec2(0, 0));

  results in the following output:
  

  This isn't a problem per se, but the uniforms will be added multiple times to shader->impl.textures in kinc_g4_shader_init() and in its compute equivalent, which might not be needed and perhaps could even cause hard to locate issues down the line. But I haven't tested it, it's just an observation.

• Since you mentioned on Discord that it may make sense to redesign the API a little: I'm wondering whether returning NULL if no texture unit is found would be better than just a warning. I understand that this may trick users into thinking that they need to check this value at runtime even if the shader and the uniform name are fixed in most cases and checking for the correct uniform is a "develop-time" issue and not a runtime one (= an assertion in the user code would be sufficient), but currently there is no way to programmatically check in Kha whether the texture unit was actually found. In case of Krom projects, people often use the pre-built release binaries that don't display any warnings and instead will just crash with a mini dump that doesn't give much information. Again, it's just an observation, but not really an important issue.

Huh, that was quite a wall of text. Thanks for looking into it :)