[GL] Texture data upload errors

[kvark]

reported by @Diggsey Problem with GL is that the texture creation actually depends on the channel type hint, and that's the reason for this hint to be a parameter in the first place... When the user passes None for this hint, the backend chooses unsigned integer by default. This was intentional to detect errors, as opposed to defaulting to a normalized format, which is used more often.

However, such error detection is not yet in place, or at least incomplete. When the user tries to update the texture contents with a normalized format data, GL backend encounters a GL error. We should intercept this earlier and return a proper error before it reaches GL.

It would be terrific to figure out a better way to handle GL texture formats in general, since the current hint passing is an unfortunate hack.

[Bastacyclop]

What if views and targets were specified beforehand ?

let (texture, view, target) = TextureBuilder::new(kind, usage)
    // TRANSFER_DST is automatically added for Usage::Dynamic ?
    .levels(levels)
    .transfer_src()
    .transfer_dst()
    // adds SHADER_RESOURCE and adds a builder output using generics
    .with_view(TextureViewInfo::new().levels((beg, end)).swizzle(swizzle))
    // adds RENDER_TARGET and adds a builder output using generics
    .with_target(TextureTargetInfo::new().level(level).layer(layer))
    // might have something to make a depth-stencil target too
    .build(factory)
    .unwrap();

I'm not sure how hard it would be to implement (generic-wise) but it would play well with #1225 and #1036. Creating views and targets afterwards may still be possible using the non-builder API and channel_hint.

[kvark]

@Bastacyclop what is the benefit of builder pattern versus using a struct (like we do, or D3D, or Vulkan) full of fields, for which you can use the ... pattern?

let (texture, view, target) = factory.create_texture(TextureInfo {
  levels: levels,
  ... TextureInfo::new_2d(usage, 100, 100)
});

[Bastacyclop]

Here is a small test around this idea: https://is.gd/7rDT6a A lot of stuff is missing but I think this can be done without too much trouble.

[Bastacyclop]

The .. pattern cannot: merge bind flags, create multiple views or none, and probably other things. It's just more flexible.

[kvark]

I agree - creating multiple views is indeed a good use case for the builder. Perhaps, it should live in the FactoryExt in order to not add too much API surface to the core?

[Bastacyclop]

Yes, render is a good place for the builders.

[Diggsey]

👍 I think this would improve the API even if it wasn't required to solve this problem.