-
Notifications
You must be signed in to change notification settings - Fork 948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add BGRA8UNORM_STORAGE
extension
#3634
Conversation
36b15a0
to
9f3d024
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
I think we should wait for naga to add support for the texel format before we merge this so that we can test it.
phd, | ||
vk::Format::B8G8R8A8_UNORM, | ||
vk::ImageTiling::OPTIMAL, | ||
vk::FormatFeatureFlags::STORAGE_IMAGE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the shader side we need to use Unknown
since bgra8unorm
is not in this table.
So, we need to check for VK_FORMAT_FEATURE_2_STORAGE_WRITE_WITHOUT_FORMAT_BIT
instead.
See KhronosGroup/Vulkan-Docs#2027 (comment) for more background.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I don't know how to do the check of VK_FORMAT_FEATURE_2_STORAGE_WRITE_WITHOUT_FORMAT_BIT
. Check for FORMAT_FEATURE_2
requires obtaining FormatProperties3
first, but no relevant method was found in ash
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it this structure: https://docs.rs/ash/latest/ash/vk/struct.FormatProperties3.html ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but no way to get it is provided, onlyget_physical_device_format_properties
and get_physical_device_format_properties
to get FormatProperties
FormatProperties2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, how I think this should work is we call get_physical_device_format_properties2
and pass it a FormatProperties2
with a FormatProperties3
in it's pNext
chain.
This is quite convoluted as it requires extensions or specific vulkan versions.
VK_KHR_get_physical_device_properties2
or VK1.1 for the query function and FormatProperties2
structure +
VK_KHR_format_feature_flags2
or VK1.3 for FormatProperties3
the feature also needs to be added to https://github.com/gfx-rs/wgpu/blob/master/deno_webgpu/02_idl_types.js#L105-L152 |
9f3d024
to
5125578
Compare
Hey this got lost in the PR stack, could you rebase this? With naga not supporting it yet, how about we land all the infrastructure, but we keep the feature disabled. |
5125578
to
cc642e0
Compare
@cwfitzgerald Already rebased, but I'm unsure how to keep the feature disabled. |
Probably through wgpu-core removing the feature after it queries the features from the device. Can remove it and link back to the naga issue. |
67e0345
to
05d1ad0
Compare
There is still an unresolved discussion about the Vulkan impl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dismissing review request due to the above issue.
Heads up, I'm continuing the work on this branch: https://github.com/nical/wgpu/tree/bgra8unorm-storage |
I'm sorry I didn't deal with it for so long, since @nical is already done, it's more appropriate to open a new PR |
Checklist
cargo clippy
.RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown
if applicable.Connections
resolves #3359
Testing
Tested locally on metal, vulkan(--features=vulkan-portability) and dx12 backends: