Add a texture format filter.

[mbrla0]

General

This commit adds a structure called info::TextureFormatFilter and changes the code in info::query_all so that, when detecting OpenGL ES it can select a set of valid texture property combinations.

This provides PhysicalDevice::image_format_properties with a way of more accurately filtering out formats that aren't valid for textures and whose use would lead to a crash.

Fixes #3683 PR checklist:

• [x] make succeeds (on *nix)
• [ ] make reftests succeeds
• [x] tested examples with the following backends:
  1. OpenGL
  2. Vulkan

Remarks

One thing to keep in mind about this PR is that the current filter implementation overblocks formats. This is due to a limitation with the definition of PhysicalDevice::image_format_properties. Since the image creation function picks the underlying OpenGL function based on not just the format and usage, as image_format_properties assumes, but also based on the mipmap levels. There's no way for us to figure out for certain which function is gonna be picked for an image from image_format_properties. Thus, overblocking is the only viable solution I could think of that doesn't involve major changes to image_format_properties.

Other important thing to note is that this PR does not back port this change to hal-0.7, which I think is also important. I have already got that back port implemented in my own fork, but I'll leave it for another PR.

[kvark]

Other important thing to note is that this PR does not back port this change to hal-0.7, which I think is also important. I have already got that back port implemented in my own fork, but I'll leave it for another PR.

probably unnecessary at this point. GL is rough right now, it's fine to expect GL adopters to run on latest master.

[kvark]

bors r-

[bors[bot]]

Canceled.

[Gordon-F]

@DarkRyu550 Thank you for contributing! Could we also add EXT table for textures supported by extensions? For example GL_EXT_texture_format_BGRA8888

[Gordon-F]

Maybe we can reuse Requirement code here?

For example:

pub fn describe_format(format: Format) -> Option<FormatDescription> {
    use crate::native::VertexAttribFunction::*;
    use hal::format::Format::*;
    let _ = Double; //mark as used

    // TODO: Add more formats and error handling for `None`
    let format_desc = match format {
        R8Uint => FormatDescription::new(
            glow::R8UI,
            glow::RED_INTEGER,
            glow::UNSIGNED_BYTE,
            1,
            Integer,
            &[Es(1, 0)] // If None - Supports everywhere
        ),
        _ => return None,
    };

    if let Some(requirements) = format_desc.requirements {
        if crate::is_supported(requirements) {
            format_desc
        }
    }

    None
}

It also should save from code duplication problem (FormatDescription table and TextureFormatFilter table).

[mbrla0]

It also should save from code duplication problem (FormatDescription table and TextureFormatFilter table).

I'd originally thought about implementing the filter in that way, but ultimately decided against it for two main reasons: maintainability and modularity.

First, for maintainability. As far as I know, the only way to know whether or not a given set of texture parameters is supported or not in OpenGL ES is to take it right from the documentation. This goes for both deriving this list for the first time and for keeping it up to date. That list is, as of OpenGL ES 3.2, 76 entries long, and probably bound to grow in the future.

So, if you were to implement this filter together with the logic in describe_format, you'd have to add at least 76 checks integrated into the structure of the match and keep those up to date with the table in the documentation. Having it as a separate table of (u32, u32, u32) lets us keep a structure that is effectively identical to that of the original table, which should take much less effort to verify and update, whenever needed. Then, having a separate type built around it leaves the way these tables are actually specified as an implementation detal.

Second, for modularity. Because I planned for the whitelist to be expanded based on available extensions, it would only make the branching in describe_format more complicated if we were to also check for it there, rather than in a separate, composable set.

[kvark]

@Gordon-F does your concern still stand?

[Gordon-F]

I'm ok with the current implementation 👍

[kvark]

Ok, we can iterate on it. It's good to land at least for addressing the problem we have now. bors r+

[bors[bot]]

Build succeeded:

• Android Stable
• Dependency Check
• iOS Stable
• MacOS Nightly
• MacOS Stable
• Ubuntu Nightly
• Ubuntu Stable
• Web Assembly
• Windows Nightly
• Windows Stable