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

remove pixel size and try to calculate texture sizes more correctly #6788

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 2 additions & 8 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use bevy_render::{
render_resource::*,
renderer::{RenderDevice, RenderQueue},
texture::{
BevyDefault, DefaultImageSampler, GpuImage, Image, ImageSampler, TextureFormatPixelInfo,
BevyDefault, DefaultImageSampler, Extent3dDimensions, GpuImage, Image, ImageSampler,
},
view::{ComputedVisibility, ViewTarget, ViewUniform, ViewUniformOffset, ViewUniforms},
Extract, RenderApp, RenderStage,
Expand Down Expand Up @@ -446,7 +446,6 @@ impl FromWorld for MeshPipeline {
ImageSampler::Descriptor(descriptor) => render_device.create_sampler(&descriptor),
};

let format_size = image.texture_descriptor.format.pixel_size();
render_queue.write_texture(
ImageCopyTexture {
texture: &texture,
Expand All @@ -457,12 +456,7 @@ impl FromWorld for MeshPipeline {
&image.data,
ImageDataLayout {
offset: 0,
bytes_per_row: Some(
std::num::NonZeroU32::new(
image.texture_descriptor.size.width * format_size as u32,
)
.unwrap(),
),
bytes_per_row: Some(image.texture_descriptor.bytes_per_row()),
rows_per_image: None,
},
image.texture_descriptor.size,
Expand Down
10 changes: 3 additions & 7 deletions crates/bevy_render/src/texture/hdr_texture_loader.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::texture::{Image, TextureFormatPixelInfo};
use crate::texture::Image;
use anyhow::Result;
use bevy_asset::{AssetLoader, LoadContext, LoadedAsset};
use bevy_utils::BoxedFuture;
Expand All @@ -16,16 +16,12 @@ impl AssetLoader for HdrTextureLoader {
) -> BoxedFuture<'a, Result<()>> {
Box::pin(async move {
let format = TextureFormat::Rgba32Float;
debug_assert_eq!(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this debug assert is sort of meaningless since we're hard coded to Rgba32Float

format.pixel_size(),
4 * 4,
"Format should have 32bit x 4 size"
);

let decoder = image::codecs::hdr::HdrDecoder::new(bytes)?;
let info = decoder.metadata();
let rgb_data = decoder.read_image_hdr()?;
let mut rgba_data = Vec::with_capacity(rgb_data.len() * format.pixel_size());
let mut rgba_data =
Vec::with_capacity(rgb_data.len() * format.describe().block_size as usize);

for rgb in rgb_data {
let alpha = 1.0f32;
Expand Down
198 changes: 46 additions & 152 deletions crates/bevy_render/src/texture/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ use bevy_derive::{Deref, DerefMut};
use bevy_ecs::system::{lifetimeless::SRes, Resource, SystemParamItem};
use bevy_math::Vec2;
use bevy_reflect::{FromReflect, Reflect, TypeUuid};
use std::hash::Hash;
use std::{hash::Hash, num::NonZeroU32};
use thiserror::Error;
use wgpu::{
Extent3d, ImageCopyTexture, ImageDataLayout, Origin3d, TextureDimension, TextureFormat,
TextureViewDescriptor,
Extent3d, ImageCopyTexture, ImageDataLayout, Origin3d, TextureDescriptor, TextureDimension,
TextureFormat, TextureViewDescriptor,
};

pub const TEXTURE_ASSET_INDEX: u64 = 0;
Expand Down Expand Up @@ -172,7 +172,8 @@ pub struct DefaultImageSampler(pub(crate) Sampler);
impl Default for Image {
fn default() -> Self {
let format = wgpu::TextureFormat::bevy_default();
let data = vec![255; format.pixel_size()];
// TODO: check if this breaks if bevy_default is a compressed texture
let data = vec![255; format.describe().block_size as usize];
Image {
data,
texture_descriptor: wgpu::TextureDescriptor {
Expand Down Expand Up @@ -207,7 +208,7 @@ impl Image {
format: TextureFormat,
) -> Self {
debug_assert_eq!(
size.volume() * format.pixel_size(),
total_bytes(&size, &format) as usize,
data.len(),
"Pixel data, size and format have to match",
);
Expand Down Expand Up @@ -239,7 +240,7 @@ impl Image {
value.resize(size);

debug_assert_eq!(
pixel.len() % format.pixel_size(),
pixel.len() % format.describe().block_size as usize,
0,
"Must not have incomplete pixel data."
);
Expand Down Expand Up @@ -271,20 +272,19 @@ impl Image {
/// Does not properly resize the contents of the image, but only its internal `data` buffer.
pub fn resize(&mut self, size: Extent3d) {
self.texture_descriptor.size = size;
self.data.resize(
size.volume() * self.texture_descriptor.format.pixel_size(),
0,
);
self.data
.resize(self.texture_descriptor.total_bytes() as usize, 0);
}

/// Changes the `size`, asserting that the total number of data elements (pixels) remains the
/// same.
///
/// # Panics
/// Panics if the `new_size` does not have the same volume as to old one.
/// Panics if the `new_size` does not have the same number of bytes as the old one.
pub fn reinterpret_size(&mut self, new_size: Extent3d) {
assert!(
new_size.volume() == self.texture_descriptor.size.volume(),
total_bytes(&new_size, &self.texture_descriptor.format)
== self.texture_descriptor.total_bytes(),
"Incompatible sizes: old = {:?} new = {:?}",
self.texture_descriptor.size,
new_size
Expand Down Expand Up @@ -455,148 +455,48 @@ impl<'a> ImageType<'a> {
}
}

/// Used to calculate the volume of an item.
pub trait Volume {
fn volume(&self) -> usize;
/// Extends Extent3d with some convenience methods to calculate some useful values related to the dimensions of a texture
pub trait Extent3dDimensions {
/// Calculates the bytes per row in a texture
fn bytes_per_row(&self) -> NonZeroU32;
/// Calculates the rows in an image
fn rows_per_image(&self) -> u32;
/// Calculates the total bytes for a texture
fn total_bytes(&self) -> u32;
}

impl Volume for Extent3d {
/// Calculates the volume of the [`Extent3d`].
fn volume(&self) -> usize {
(self.width * self.height * self.depth_or_array_layers) as usize
impl<'a> Extent3dDimensions for TextureDescriptor<'a> {
fn bytes_per_row(&self) -> NonZeroU32 {
bytes_per_row(self.size.physical_size(self.format).width, &self.format)
}
}

/// 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,
}
fn rows_per_image(&self) -> u32 {
rows_per_image(self.size.physical_size(self.format).height, &self.format)
}

/// 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
fn total_bytes(&self) -> u32 {
total_bytes(&self.size, &self.format)
}
}

impl TextureFormatPixelInfo for TextureFormat {
#[allow(clippy::match_same_arms)]
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::R16Unorm
| TextureFormat::Rg16Uint
| TextureFormat::Rg16Sint
| TextureFormat::Rg16Unorm
| 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::Rgb9e5Ufloat => 4,
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"),
};
fn bytes_per_row(physical_width: u32, format: &TextureFormat) -> NonZeroU32 {
let info = format.describe();
NonZeroU32::new(physical_width * info.block_size as u32 / info.block_dimensions.0 as u32)
.unwrap()
}

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

TextureFormat::Rg8Unorm
| TextureFormat::Rg8Snorm
| TextureFormat::Rg8Uint
| TextureFormat::Rg8Sint
| TextureFormat::Rg16Uint
| TextureFormat::Rg16Sint
| TextureFormat::Rg16Unorm
| 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::Rgb9e5Ufloat
| 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"),
};
fn rows_per_image(physical_height: u32, format: &TextureFormat) -> u32 {
let info = format.describe();
physical_height / info.block_dimensions.1 as u32
}

PixelInfo {
type_size,
num_components: components,
}
}
/// Calculate the total bytes needed to hold an image with `size` and `format`. If you have a [`TextureDescriptor`]
/// consider using [`Extent3dDimensions::total_bytes`] instead.
pub fn total_bytes(size: &Extent3d, format: &TextureFormat) -> u32 {
let physical_size = size.physical_size(*format);
let bytes_per_row = u32::from(bytes_per_row(physical_size.width, format));
let rows_per_image = rows_per_image(physical_size.height, format);
bytes_per_row * rows_per_image * size.depth_or_array_layers
}

/// The GPU-representation of an [`Image`].
Expand Down Expand Up @@ -637,7 +537,6 @@ impl RenderAsset for Image {
)
} else {
let texture = render_device.create_texture(&image.texture_descriptor);
let format_size = image.texture_descriptor.format.pixel_size();
render_queue.write_texture(
ImageCopyTexture {
texture: &texture,
Expand All @@ -648,14 +547,9 @@ impl RenderAsset for Image {
&image.data,
ImageDataLayout {
offset: 0,
bytes_per_row: Some(
std::num::NonZeroU32::new(
image.texture_descriptor.size.width * format_size as u32,
)
.unwrap(),
),
bytes_per_row: Some(image.texture_descriptor.bytes_per_row()),
rows_per_image: if image.texture_descriptor.size.depth_or_array_layers > 1 {
std::num::NonZeroU32::new(image.texture_descriptor.size.height)
std::num::NonZeroU32::new(image.texture_descriptor.rows_per_image())
} else {
None
},
Expand Down
12 changes: 7 additions & 5 deletions crates/bevy_render/src/texture/image_texture_conversion.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::texture::{Image, TextureFormatPixelInfo};
use crate::texture::Image;
use anyhow::anyhow;
use image::{DynamicImage, ImageBuffer};
use wgpu::{Extent3d, TextureDimension, TextureFormat};
Expand Down Expand Up @@ -84,8 +84,9 @@ impl Image {
height = image.height();
format = TextureFormat::Rgba16Uint;

let mut local_data =
Vec::with_capacity(width as usize * height as usize * format.pixel_size());
let mut local_data = Vec::with_capacity(
width as usize * height as usize * format.describe().block_size as usize,
);

for pixel in image.into_raw().chunks_exact(3) {
// TODO: use the array_chunks method once stabilised
Expand Down Expand Up @@ -117,8 +118,9 @@ impl Image {
height = image.height();
format = TextureFormat::Rgba32Float;

let mut local_data =
Vec::with_capacity(width as usize * height as usize * format.pixel_size());
let mut local_data = Vec::with_capacity(
width as usize * height as usize * format.describe().block_size as usize,
);

for pixel in image.into_raw().chunks_exact(3) {
// TODO: use the array_chunks method once stabilised
Expand Down
18 changes: 16 additions & 2 deletions crates/bevy_sprite/src/dynamic_texture_atlas_builder.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::TextureAtlas;
use bevy_asset::Assets;
use bevy_math::{IVec2, Rect, Vec2};
use bevy_render::texture::{Image, TextureFormatPixelInfo};
use bevy_render::texture::Image;
use guillotiere::{size2, Allocation, AtlasAllocator};

pub struct DynamicTextureAtlasBuilder {
Expand Down Expand Up @@ -67,12 +67,26 @@ impl DynamicTextureAtlasBuilder {
allocation: Allocation,
texture: &Image,
) {
debug_assert_eq!(
atlas_texture
.texture_descriptor
.format
.describe()
.block_dimensions,
(1, 1),
"Compressed textures are unsupported"
);

let mut rect = allocation.rectangle;
rect.max.x -= self.padding;
rect.max.y -= self.padding;
let atlas_width = atlas_texture.texture_descriptor.size.width as usize;
let rect_width = rect.width() as usize;
let format_size = atlas_texture.texture_descriptor.format.pixel_size();
let format_size = atlas_texture
.texture_descriptor
.format
.describe()
.block_size as usize;

for (texture_y, bound_y) in (rect.min.y..rect.max.y).map(|i| i as usize).enumerate() {
let begin = (bound_y * atlas_width + rect.min.x as usize) * format_size;
Expand Down
Loading