Skip to content
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 StorageFormat::Bgra8Unorm #2542

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Add StorageFormat::Bgra8Unorm #2542

merged 1 commit into from
Oct 9, 2023

Conversation

nical
Copy link
Contributor

@nical nical commented Oct 6, 2023

Fixes #2195
Blocks: gfx-rs/wgpu#3634

I'm not sure what the best course is for the wgsl backend, so I defaulted to putting the "rgba8" swizzle which is the closest available format. I suppose that the backend would have to insert some extra code to convert when interacting with the storage texture (probably that's already done in other places?).

@nical
Copy link
Contributor Author

nical commented Oct 6, 2023

Out of curiosity I looked at tint's solution and they do some substitution between bgra and rgba in their IR, see https://dawn.googlesource.com/tint/+/refs/heads/main/src/tint/lang/core/ir/transform/bgra8unorm_polyfill.cc

@teoxoy
Copy link
Member

teoxoy commented Oct 6, 2023

Out of curiosity I looked at tint's solution and they do some substitution between bgra and rgba in their IR, see https://dawn.googlesource.com/tint/+/refs/heads/main/src/tint/lang/core/ir/transform/bgra8unorm_polyfill.cc

That transform only runs for SPIR-V. The bgra8 -> rgba8 format replacement is not valid, they should use Unknown because SPIR-V's rgba8 format is not compatible with the bgra8 one that we'll use on the API side.

The swizzle I think we probably need but we should probably test first.

I'm not sure what the best course is for the wgsl backend, so I defaulted to putting the "rgba8" swizzle which is the closest available format. I suppose that the backend would have to insert some extra code to convert when interacting with the storage texture (probably that's already done in other places?).

I assume you mean the GLSL backend, we should probably fail with an unimplemented/unsupported error.

src/back/spv/instructions.rs Show resolved Hide resolved
src/back/glsl/mod.rs Outdated Show resolved Hide resolved
@nical nical force-pushed the bgrau-storage branch 2 times, most recently from 328d047 to e5c727b Compare October 9, 2023 08:01
src/back/spv/writer.rs Outdated Show resolved Hide resolved
@teoxoy teoxoy merged commit 3c7dbc4 into gfx-rs:master Oct 9, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support bgra8unorm texel format
2 participants