gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.18k stars 896 forks source link

Add `.bits_per_pixel()` to the TextureFormat to simplify buffer size calculation for each texture format. #4054

Closed dtzxporter closed 1 year ago

dtzxporter commented 1 year ago

Is your feature request related to a problem? Please describe. It would make it easy to perform some calculations if each format had it's bits per pixel value.

Describe the solution you'd like A method on TextureFormat that returns the bits per pixel for the format: Bc1: 4, rgba8: 32, etc.

Describe alternatives you've considered N/A

Additional context N/A

Wumpf commented 1 year ago

Since you explicitly mention Bc1 as 4 bits per pixel, can you outline the usecase a bit?

There's already block_size which returns a texel's block size in bytes. So it would be trivial to implement a bit size per pixel based on that (since there's also block_dimensions, but I fear it could be confusing to put that in for anyone not familiar with the concept of texel blocks 🤔

dtzxporter commented 1 year ago

I feel like since wgpu is fairly low level, the point should be to have something like this, especially since you don't have to use it.

Using bits per pixel, its also very easy to calculate the size in bytes of a buffer, for instance, (bpp width height) / 8 = size of the buffer required for the texture.

I'm currently using wgpu to perform texture conversion, because there is no rust support for most bc compressed formats, and conversion between other non-compressed formats.

Libraries such as DirectXTex provide this as well.

While I have my own ImageFormat which closely maps DXGI_FORMAT as it has more options, I have to map into wgpu's TextureFormat for instances where it's used as a conversion backend, and it would be useful to have that information at the ready.

JMS55 commented 1 year ago

You also have to account for both mipmaps and depth/array layers as well. Bevy has some code for this already, but doesn't account for mips. Iirc there's also code that correctly accounts for all of this built into wgpu already, as it uses it to validate create_texture_with_data().

teoxoy commented 1 year ago

Bits per pixel can be derived from the existing texel block size and dimensions.

bits_per_pixel = (block_size * 8) / (block_dimensions.0 * block_dimensions.1)

Since the texture size has to be a multiple of the texel block dimensions, I'd imagine calculating the size in texel blocks and later multiplying it with the texel block size to get the size of the buffer required for the texture rather than calculating the bits per pixel.

I feel like there is a lot of variety in how users choose to calculate those values and adding a utility function for this specific implementation won't satisfy all use cases.

Thoughts?

cwfitzgerald commented 1 year ago

(I would note that the formula should be bits_per_pixel = (block_size * 8) / (block_dimensions.0 * block_dimension.1))

Because we have block encoded textures, bits per pixel isn't actually the core primitive here and we should encourage people to be thinking about these transforms in terms of blocks, despite blocks being more complicated. Additionally bits per pixel can be fractional (take astc 5x5: (16 * 8) / (5 * 5) = 5.12 bits/pixel) which isn't good for address math.

Maybe there is use in having helpers that automatically compute things like the buffer copy footprint (and how to issue memcpys to "remove" the padding needed for buffer rows) to make these things easier, but this is orthogonal.

teoxoy commented 1 year ago

(I would note that the formula should be bits_per_pixel = (block_size * 8) / (block_dimensions.0 * block_dimension.1))

Ah, indeed - I had it in my head, I double checked it with the calculator but still ended up writing it wrong 😅

I will edit the comment so the wrong formula doesn't spread.

Wumpf commented 1 year ago

I suppose we could expose a lot of the internal calculations that are used for validation to wgpu-utils, again this is orthogonal.

Still super skeptical about bits_per_pixel for the exact reasons @cwfitzgerald mentioned. Previously, I didn't even consider fractional return values here which makes such a method extra confusing or even dubious - 5.12 isn't even exactly representable in f32, meaning we'd need to return a fraction