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.

Sign up for GitHub

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

Jump to bottom

Texture formats #4124

Closed
wants to merge 14 commits into from
Closed

Texture formats #4124

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8431c91
removed pixel info and replaced it with wrapper for wgpu describe
Mar 6, 2022
0b3b0f4
added create texture with data to render device
Mar 6, 2022
e1c50ca
Merge branch 'bevyengine:main' into texture-formats
kurtkuehnert Mar 28, 2022
bd1681f
updated onto main
Mar 28, 2022
ae8b563
Merge branch 'bevyengine:main' into texture-formats
kurtkuehnert May 4, 2022
cc00ca5
Merge branch 'bevyengine:main' into texture-formats
kurtkuehnert May 25, 2022
9cad2c8
Merge branch 'main' into texture-formats
kurtkuehnert Jun 6, 2022
affc4c5
Merge branch 'bevyengine:main' into texture-formats
kurtkuehnert Jun 22, 2022
640f352
Merge branch 'bevyengine:main' into texture-formats
kurtkuehnert Jun 28, 2022
d5d30ea
Merge branch 'bevyengine:main' into texture-formats
kurtkuehnert Jul 1, 2022
14b2e7e
added support for block formats pixel size
kurtkuehnert Jul 1, 2022
f3964ff
Merge remote-tracking branch 'origin/texture-formats' into texture-fo…
kurtkuehnert Jul 1, 2022
dbe0f09
Merge branch 'bevyengine:main' into texture-formats
kurtkuehnert Jul 4, 2022
adb47ba
removed pixel info in favor of texture size
kurtkuehnert Jul 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions crates/bevy_render/src/renderer/render_device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::render_resource::{
BindGroup, BindGroupLayout, Buffer, ComputePipeline, RawRenderPipelineDescriptor,
RenderPipeline, Sampler, Texture,
use crate::{
render_resource::{
BindGroup, BindGroupLayout, Buffer, ComputePipeline, RawRenderPipelineDescriptor,
RenderPipeline, Sampler, Texture,
},
renderer::RenderQueue,
};
use futures_lite::future;
use std::sync::Arc;
Expand Down Expand Up @@ -129,6 +132,19 @@ impl RenderDevice {
Texture::from(wgpu_texture)
}

/// Creates a new [`Texture`] and initializes it with the specified data.
///
/// `desc` specifies the general format of the texture.
pub fn create_texture_with_data(
kurtkuehnert marked this conversation as resolved.
Show resolved Hide resolved
&self,
queue: &RenderQueue,
desc: &wgpu::TextureDescriptor,
data: &[u8],
) -> Texture {
let wgpu_texture = self.device.create_texture_with_data(queue, desc, data);
Texture::from(wgpu_texture)
}

/// Creates a new [`Sampler`].
///
/// `desc` specifies the behavior of the sampler.
Expand Down
122 changes: 5 additions & 117 deletions crates/bevy_render/src/texture/image.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,128 +256,16 @@ impl Volume for Extent3d {
}
}

/// Information about the pixel size in bytes and the number of different components.
pub struct PixelInfo {
/// The size of a component of a pixel in bytes.
pub type_size: usize,
/// The amount of different components (color channels).
pub num_components: usize,
}

/// Extends the wgpu [`TextureFormat`] with information about the pixel.
pub trait TextureFormatPixelInfo {
/// Returns the pixel information of the format.
fn pixel_info(&self) -> PixelInfo;
/// Returns the size of a pixel of the format.
fn pixel_size(&self) -> usize {
let info = self.pixel_info();
info.type_size * info.num_components
}
/// Returns the size of a pixel of the format in bytes.
fn pixel_size(&self) -> usize;
}

impl TextureFormatPixelInfo for TextureFormat {
fn pixel_info(&self) -> PixelInfo {
let type_size = match self {
// 8bit
TextureFormat::R8Unorm
| TextureFormat::R8Snorm
| TextureFormat::R8Uint
| TextureFormat::R8Sint
| TextureFormat::Rg8Unorm
| TextureFormat::Rg8Snorm
| TextureFormat::Rg8Uint
| TextureFormat::Rg8Sint
| TextureFormat::Rgba8Unorm
| TextureFormat::Rgba8UnormSrgb
| TextureFormat::Rgba8Snorm
| TextureFormat::Rgba8Uint
| TextureFormat::Rgba8Sint
| TextureFormat::Bgra8Unorm
| TextureFormat::Bgra8UnormSrgb => 1,

// 16bit
TextureFormat::R16Uint
| TextureFormat::R16Sint
| TextureFormat::R16Float
| TextureFormat::Rg16Uint
| TextureFormat::Rg16Sint
| TextureFormat::Rg16Float
| TextureFormat::Rgba16Uint
| TextureFormat::Rgba16Sint
| TextureFormat::Rgba16Float => 2,

// 32bit
TextureFormat::R32Uint
| TextureFormat::R32Sint
| TextureFormat::R32Float
| TextureFormat::Rg32Uint
| TextureFormat::Rg32Sint
| TextureFormat::Rg32Float
| TextureFormat::Rgba32Uint
| TextureFormat::Rgba32Sint
| TextureFormat::Rgba32Float
| TextureFormat::Depth32Float => 4,

// special cases
TextureFormat::Rgb10a2Unorm => 4,
TextureFormat::Rg11b10Float => 4,
TextureFormat::Depth24Plus => 3, // FIXME is this correct?
TextureFormat::Depth24PlusStencil8 => 4,
// TODO: this is not good! this is a temporary step while porting bevy_render to direct wgpu usage
_ => panic!("cannot get pixel info for type"),
};

let components = match self {
TextureFormat::R8Unorm
| TextureFormat::R8Snorm
| TextureFormat::R8Uint
| TextureFormat::R8Sint
| TextureFormat::R16Uint
| TextureFormat::R16Sint
| TextureFormat::R16Float
| TextureFormat::R32Uint
| TextureFormat::R32Sint
| TextureFormat::R32Float => 1,

TextureFormat::Rg8Unorm
| TextureFormat::Rg8Snorm
| TextureFormat::Rg8Uint
| TextureFormat::Rg8Sint
| TextureFormat::Rg16Uint
| TextureFormat::Rg16Sint
| TextureFormat::Rg16Float
| TextureFormat::Rg32Uint
| TextureFormat::Rg32Sint
| TextureFormat::Rg32Float => 2,

TextureFormat::Rgba8Unorm
| TextureFormat::Rgba8UnormSrgb
| TextureFormat::Rgba8Snorm
| TextureFormat::Rgba8Uint
| TextureFormat::Rgba8Sint
| TextureFormat::Bgra8Unorm
| TextureFormat::Bgra8UnormSrgb
| TextureFormat::Rgba16Uint
| TextureFormat::Rgba16Sint
| TextureFormat::Rgba16Float
| TextureFormat::Rgba32Uint
| TextureFormat::Rgba32Sint
| TextureFormat::Rgba32Float => 4,

// special cases
TextureFormat::Rgb10a2Unorm
| TextureFormat::Rg11b10Float
| TextureFormat::Depth32Float
| TextureFormat::Depth24Plus
| TextureFormat::Depth24PlusStencil8 => 1,
// TODO: this is not good! this is a temporary step while porting bevy_render to direct wgpu usage
_ => panic!("cannot get pixel info for type"),
};

PixelInfo {
type_size,
num_components: components,
}
#[inline]
fn pixel_size(&self) -> usize {
self.describe().block_size as usize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct for non-block formats, but for block formats, it will be wrong. You kind of have to use both the block dimensions and the block byte size. It degrades to pixels as dimensions become 1x1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I am not familiar with compressed textures / block formats, so I am not sure how to proceed. But my "soon to be" terrain plugin depends on being able to use some of the unlocked formats.

Do you want to take over this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block formats encode MxN pixels in block_size bytes. So the pixel_size would be the mean if that, ie block_size / (block_dimensions.0 * block_dimensions.1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this information even useful to have? I could just remove the pixel size thing entirely.
It is used in some places in the engine though and I am not sure who else is using it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@superdump I have updated the PR now. Is the block size guaranteed to be a multiple of its area? If not than this change is not correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The block size is the size of a compressed block in bytes. So it accounts for all the data used to encode the block. I think I understand the intent of your question and the answer to that intent is yes. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhh, I think the current implementation of pixel size is wrong.
For example the TextureFormat::Astc5x4RgbaUnorm has got a blocksize of 16 bytes and dimensions of 5 by 4 pixels. Thus the average pixel size is 6.4 bit/px. If that number is stored as a usize it is casted to 0, which is wrong.
Prior to my PR only some of the uncompressed textures, where supported by the pixel size method, so this was not an issue.

This method is referenced in 14 places in the bevy code base and always used to calculate the size of the texture in bytes.
By replacing pixel_size with describe().block_size we would enable bevys Image to be compatible with all uncompressed texture formats (This is the issue I am personally blocked on).

However the same is not true for all compressed formats.
I am not familiar with how to calculate their size properly. Do you have to divide the texture size in pixels by the block dimensions and subsequently multiply that by the block size? How would you deal with rounding?
Does the texture size refer to the dimensions in pixels or in blocks? If the former: How to deal with sizes that are not a multiple of the block dimensions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Good catch. Yes, I suppose maybe the real API then should match wgpu's in the sense that it should expose block dimensions and block size in bytes.

However the same is not true for all compressed formats.
I am not familiar with how to calculate their size properly. Do you have to divide the texture size in pixels by the block dimensions and subsequently multiply that by the block size? How would you deal with rounding?
Does the texture size refer to the dimensions in pixels or in blocks? If the former: How to deal with sizes that are not a multiple of the block dimensions?

Yes, pretty much. For calculating the size of a texture, you have to round up to the next block. So if you have a block size of 4x4 and a texture that is 6x6 then that will be 2 blocks wide and 2 blocks high. I think elsewhere I have calculated this as image width in blocks = (image width in pixels + block width in pixels - 1) / block width in pixels and similar for height. And then use the image dimensions in blocks to calculate the image size. Note then that for non-compressed formats, where the block width and block height are both 1, image width in blocks will be equivalent to image width in pixels and the block size in bytes would just be the pixel size in bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the flawed pixel size method in favor of the new texture size method. This should be calculating the texture size in bytes properly now.

}
}

Expand Down