gfx-rs / gfx

[maintenance mode] A low-overhead Vulkan-like GPU API for Rust.
http://gfx-rs.github.io/
Apache License 2.0
5.35k stars 549 forks source link

samplerFilterMinmax enabled by default #3677

Closed brannegan closed 3 years ago

brannegan commented 3 years ago

Short info header:

kvark commented 3 years ago

Oh snap, thank you for filing! So Vulkan 1.2 includes the extension in the core, but there is still a flag to check, which we missed. Would you want to make a PR to fix this? That would be wonderful!

tangmi commented 3 years ago

What happens when sampler_filter_minmax is not supported? Since it (and imagless_framebuffer) aren't in hal::Features, would a user of gfx be able to tell if the support is available?

kvark commented 3 years ago

@tangmi I was thinking that we'd just not expose ImageFeature::SAMPLED_MINMAX in this case in any format.

tangmi commented 3 years ago

@kvark I assume the image format feature flags we get from Vulkan just won't have SAMPLED_IMAGE_FILTER_MINMAX if the device feature is disabled.

This was definitely my mistake from the device creation refactor, I think the PR I opened addresses it.

kvark commented 3 years ago

@tangmi I'm afraid this may be a bit trickier than this. Vulkan clients are expected to ignore the flags they aren't supposed to know about. So if the client (which is gfx-backend-vulkan in our case) doesn't request any sampler reduction feature/extension, they can still get the format feature bits, but they are supposed to ignore them :/ This means even with the PR you made, gfx may return the SAMPLED_IMAGE_FILTER_MINMAX feature on some formats, erroneously.

tangmi commented 3 years ago

@kvark Aight I'll make a follow-up PR soon to plumb supports_sampler_filter_minmax through to the image features functions