Allowed creating uninitialized images (for use as storage textures) (#17760)

# Objective
https://github.com/bevyengine/bevy/issues/17746
## Solution
- Change `Image.data` from being a `Vec<u8>` to a `Option<Vec<u8>>`
- Added functions to help with creating images
## Testing

- Did you test these changes? If so, how?
All current tests pass
Tested a variety of existing examples to make sure they don't crash
(they don't)
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Linux x86 64-bit NixOS 
---
## Migration Guide
Code that directly access `Image` data will now need to use unwrap or
handle the case where no data is provided.
Behaviour of new_fill slightly changed, but not in a way that is likely
to affect anything. It no longer panics and will fill the whole texture
instead of leaving black pixels if the data provided is not a nice
factor of the size of the image.

---------

Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
This commit is contained in:
Lege19 2025-02-10 22:22:07 +00:00 committed by GitHub
parent db0356517e
commit 3978ba9783
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 187 additions and 129 deletions

View File

@ -465,7 +465,7 @@ pub fn lut_placeholder() -> Image {
let format = TextureFormat::Rgba8Unorm;
let data = vec![255, 0, 255, 255];
Image {
data,
data: Some(data),
texture_descriptor: TextureDescriptor {
size: Extent3d {
width: 1,

View File

@ -116,7 +116,7 @@ pub fn basis_buffer_to_image(
)))
}
};
image.data = transcoded;
image.data = Some(transcoded);
Ok(image)
}

View File

@ -11,6 +11,8 @@ pub struct CompressedImageSaver;
pub enum CompressedImageSaverError {
#[error(transparent)]
Io(#[from] std::io::Error),
#[error("Cannot compress an uninitialized image")]
UninitializedImage,
}
impl AssetSaver for CompressedImageSaver {
@ -42,7 +44,10 @@ impl AssetSaver for CompressedImageSaver {
let mut source_image = compressor_params.source_image_mut(0);
let size = image.size();
source_image.init(&image.data, size.x, size.y, 4);
let Some(ref data) = image.data else {
return Err(CompressedImageSaverError::UninitializedImage);
};
source_image.init(data, size.x, size.y, 4);
let mut compressor = basis_universal::Compressor::new(4);
#[expect(

View File

@ -82,7 +82,7 @@ pub fn dds_buffer_to_image(
..Default::default()
});
}
image.data = dds.data;
image.data = Some(dds.data);
Ok(image)
}
@ -373,7 +373,7 @@ mod test {
let r = dds_buffer_to_image("".into(), &buffer, CompressedImageFormats::BC, true);
assert!(r.is_ok());
if let Ok(r) = r {
fake_wgpu_create_texture_with_data(&r.texture_descriptor, &r.data);
fake_wgpu_create_texture_with_data(&r.texture_descriptor, r.data.as_ref().unwrap());
}
}
}

View File

@ -2,6 +2,20 @@ use crate::{Image, TextureAtlasLayout, TextureFormatPixelInfo as _};
use bevy_asset::RenderAssetUsages;
use bevy_math::{URect, UVec2};
use guillotiere::{size2, Allocation, AtlasAllocator};
use thiserror::Error;
use tracing::error;
#[derive(Debug, Error)]
pub enum DynamicTextureAtlasBuilderError {
#[error("Couldn't allocate space to add the image requested")]
FailedToAllocateSpace,
/// Attempted to add a texture to an uninitialzied atlas
#[error("cannot add texture to uninitialized atlas texture")]
UninitializedAtlas,
/// Attempted to add an uninitialized texture to an atlas
#[error("cannot add uninitialized texture to atlas")]
UninitializedSourceTexture,
}
/// Helper utility to update [`TextureAtlasLayout`] on the fly.
///
@ -42,7 +56,7 @@ impl DynamicTextureAtlasBuilder {
atlas_layout: &mut TextureAtlasLayout,
texture: &Image,
atlas_texture: &mut Image,
) -> Option<usize> {
) -> Result<usize, DynamicTextureAtlasBuilderError> {
let allocation = self.atlas_allocator.allocate(size2(
(texture.width() + self.padding).try_into().unwrap(),
(texture.height() + self.padding).try_into().unwrap(),
@ -53,12 +67,12 @@ impl DynamicTextureAtlasBuilder {
"The atlas_texture image must have the RenderAssetUsages::MAIN_WORLD usage flag set"
);
self.place_texture(atlas_texture, allocation, texture);
self.place_texture(atlas_texture, allocation, texture)?;
let mut rect: URect = to_rect(allocation.rectangle);
rect.max = rect.max.saturating_sub(UVec2::splat(self.padding));
Some(atlas_layout.add_texture(rect))
Ok(atlas_layout.add_texture(rect))
} else {
None
Err(DynamicTextureAtlasBuilderError::FailedToAllocateSpace)
}
}
@ -67,7 +81,7 @@ impl DynamicTextureAtlasBuilder {
atlas_texture: &mut Image,
allocation: Allocation,
texture: &Image,
) {
) -> Result<(), DynamicTextureAtlasBuilderError> {
let mut rect = allocation.rectangle;
rect.max.x -= self.padding as i32;
rect.max.y -= self.padding as i32;
@ -75,14 +89,20 @@ impl DynamicTextureAtlasBuilder {
let rect_width = rect.width() as usize;
let format_size = atlas_texture.texture_descriptor.format.pixel_size();
let Some(ref mut atlas_data) = atlas_texture.data else {
return Err(DynamicTextureAtlasBuilderError::UninitializedAtlas);
};
let Some(ref data) = texture.data else {
return Err(DynamicTextureAtlasBuilderError::UninitializedSourceTexture);
};
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;
let end = begin + rect_width * format_size;
let texture_begin = texture_y * rect_width * format_size;
let texture_end = texture_begin + rect_width * format_size;
atlas_texture.data[begin..end]
.copy_from_slice(&texture.data[texture_begin..texture_end]);
atlas_data[begin..end].copy_from_slice(&data[texture_begin..texture_end]);
}
Ok(())
}
}

View File

@ -13,6 +13,7 @@ use bevy_math::{AspectRatio, UVec2, UVec3, Vec2};
use core::hash::Hash;
use serde::{Deserialize, Serialize};
use thiserror::Error;
use tracing::warn;
use wgpu_types::{
AddressMode, CompareFunction, Extent3d, Features, FilterMode, SamplerBorderColor,
SamplerDescriptor, TextureDescriptor, TextureDimension, TextureFormat, TextureUsages,
@ -338,7 +339,11 @@ impl ImageFormat {
reflect(opaque, Default, Debug)
)]
pub struct Image {
pub data: Vec<u8>,
/// Raw pixel data.
/// If the image is being used as a storage texture which doesn't need to be initialized by the
/// CPU, then this should be `None`
/// Otherwise, it should always be `Some`
pub data: Option<Vec<u8>>,
// TODO: this nesting makes accessing Image metadata verbose. Either flatten out descriptor or add accessors
pub texture_descriptor: TextureDescriptor<Option<&'static str>, &'static [TextureFormat]>,
/// The [`ImageSampler`] to use during rendering.
@ -691,28 +696,9 @@ impl From<SamplerDescriptor<Option<&str>>> for ImageSamplerDescriptor {
impl Default for Image {
/// default is a 1x1x1 all '1.0' texture
fn default() -> Self {
let format = TextureFormat::bevy_default();
let data = vec![255; format.pixel_size()];
Image {
data,
texture_descriptor: TextureDescriptor {
size: Extent3d {
width: 1,
height: 1,
depth_or_array_layers: 1,
},
format,
dimension: TextureDimension::D2,
label: None,
mip_level_count: 1,
sample_count: 1,
usage: TextureUsages::TEXTURE_BINDING | TextureUsages::COPY_DST,
view_formats: &[],
},
sampler: ImageSampler::Default,
texture_view_descriptor: None,
asset_usage: RenderAssetUsages::default(),
}
let mut image = Image::default_uninit();
image.data = Some(vec![255; image.texture_descriptor.format.pixel_size()]);
image
}
}
@ -734,17 +720,36 @@ impl Image {
data.len(),
"Pixel data, size and format have to match",
);
let mut image = Self {
data,
..Default::default()
};
image.texture_descriptor.dimension = dimension;
image.texture_descriptor.size = size;
image.texture_descriptor.format = format;
image.asset_usage = asset_usage;
let mut image = Image::new_uninit(size, dimension, format, asset_usage);
image.data = Some(data);
image
}
/// Exactly the same as [`Image::new`], but doesn't initialize the image
pub fn new_uninit(
size: Extent3d,
dimension: TextureDimension,
format: TextureFormat,
asset_usage: RenderAssetUsages,
) -> Self {
Image {
data: None,
texture_descriptor: TextureDescriptor {
size,
format,
dimension,
label: None,
mip_level_count: 1,
sample_count: 1,
usage: TextureUsages::TEXTURE_BINDING | TextureUsages::COPY_DST,
view_formats: &[],
},
sampler: ImageSampler::Default,
texture_view_descriptor: None,
asset_usage,
}
}
/// A transparent white 1x1x1 image.
///
/// Contrast to [`Image::default`], which is opaque.
@ -755,26 +760,30 @@ impl Image {
let format = TextureFormat::bevy_default();
debug_assert!(format.pixel_size() == 4);
let data = vec![255, 255, 255, 0];
Image {
data,
texture_descriptor: TextureDescriptor {
size: Extent3d {
width: 1,
height: 1,
depth_or_array_layers: 1,
},
format,
dimension: TextureDimension::D2,
label: None,
mip_level_count: 1,
sample_count: 1,
usage: TextureUsages::TEXTURE_BINDING | TextureUsages::COPY_DST,
view_formats: &[],
Image::new(
Extent3d {
width: 1,
height: 1,
depth_or_array_layers: 1,
},
sampler: ImageSampler::Default,
texture_view_descriptor: None,
asset_usage: RenderAssetUsages::default(),
}
TextureDimension::D2,
data,
format,
RenderAssetUsages::default(),
)
}
/// Creates a new uninitialized 1x1x1 image
pub fn default_uninit() -> Image {
Image::new_uninit(
Extent3d {
width: 1,
height: 1,
depth_or_array_layers: 1,
},
TextureDimension::D2,
TextureFormat::bevy_default(),
RenderAssetUsages::default(),
)
}
/// Creates a new image from raw binary data and the corresponding metadata, by filling
@ -789,12 +798,7 @@ impl Image {
format: TextureFormat,
asset_usage: RenderAssetUsages,
) -> Self {
let mut value = Image::default();
value.texture_descriptor.format = format;
value.texture_descriptor.dimension = dimension;
value.asset_usage = asset_usage;
value.resize(size);
let byte_len = format.pixel_size() * size.volume();
debug_assert_eq!(
pixel.len() % format.pixel_size(),
0,
@ -802,15 +806,12 @@ impl Image {
format.pixel_size(),
);
debug_assert!(
pixel.len() <= value.data.len(),
pixel.len() <= byte_len,
"Fill data must fit within pixel buffer (expected {}B).",
value.data.len(),
byte_len,
);
for current_pixel in value.data.chunks_exact_mut(pixel.len()) {
current_pixel.copy_from_slice(pixel);
}
value
let data = pixel.iter().copied().cycle().take(byte_len).collect();
Image::new(size, dimension, data, format, asset_usage)
}
/// Returns the width of a 2D image.
@ -849,10 +850,14 @@ 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,
);
if let Some(ref mut data) = self.data {
data.resize(
size.volume() * self.texture_descriptor.format.pixel_size(),
0,
);
} else {
warn!("Resized an uninitialized image. Directly modify image.texture_descriptor.size instead");
}
}
/// Changes the `size`, asserting that the total number of data elements (pixels) remains the
@ -1035,16 +1040,18 @@ impl Image {
#[inline(always)]
pub fn pixel_bytes(&self, coords: UVec3) -> Option<&[u8]> {
let len = self.texture_descriptor.format.pixel_size();
let data = self.data.as_ref()?;
self.pixel_data_offset(coords)
.map(|start| &self.data[start..(start + len)])
.map(|start| &data[start..(start + len)])
}
/// Get a mutable reference to the data bytes where a specific pixel's value is stored
#[inline(always)]
pub fn pixel_bytes_mut(&mut self, coords: UVec3) -> Option<&mut [u8]> {
let len = self.texture_descriptor.format.pixel_size();
self.pixel_data_offset(coords)
.map(|start| &mut self.data[start..(start + len)])
let offset = self.pixel_data_offset(coords);
let data = self.data.as_mut()?;
offset.map(|start| &mut data[start..(start + len)])
}
/// Read the color of a specific pixel (1D texture).

View File

@ -170,22 +170,26 @@ impl Image {
///
/// To convert [`Image`] to a different format see: [`Image::convert`].
pub fn try_into_dynamic(self) -> Result<DynamicImage, IntoDynamicImageError> {
let width = self.width();
let height = self.height();
let Some(data) = self.data else {
return Err(IntoDynamicImageError::UninitializedImage);
};
match self.texture_descriptor.format {
TextureFormat::R8Unorm => ImageBuffer::from_raw(self.width(), self.height(), self.data)
.map(DynamicImage::ImageLuma8),
TextureFormat::R8Unorm => {
ImageBuffer::from_raw(width, height, data).map(DynamicImage::ImageLuma8)
}
TextureFormat::Rg8Unorm => {
ImageBuffer::from_raw(self.width(), self.height(), self.data)
.map(DynamicImage::ImageLumaA8)
ImageBuffer::from_raw(width, height, data).map(DynamicImage::ImageLumaA8)
}
TextureFormat::Rgba8UnormSrgb => {
ImageBuffer::from_raw(self.width(), self.height(), self.data)
.map(DynamicImage::ImageRgba8)
ImageBuffer::from_raw(width, height, data).map(DynamicImage::ImageRgba8)
}
// This format is commonly used as the format for the swapchain texture
// This conversion is added here to support screenshots
TextureFormat::Bgra8UnormSrgb | TextureFormat::Bgra8Unorm => {
ImageBuffer::from_raw(self.width(), self.height(), {
let mut data = self.data;
ImageBuffer::from_raw(width, height, {
let mut data = data;
for bgra in data.chunks_exact_mut(4) {
bgra.swap(0, 2);
}
@ -213,6 +217,10 @@ pub enum IntoDynamicImageError {
/// Encountered an unknown error during conversion.
#[error("Failed to convert into {0:?}.")]
UnknownConversionError(TextureFormat),
/// Tried to convert an image that has no texture data
#[error("Image has no texture data")]
UninitializedImage,
}
#[cfg(test)]

View File

@ -266,7 +266,7 @@ pub fn ktx2_buffer_to_image(
// error cases have been handled
let mut image = Image::default();
image.texture_descriptor.format = texture_format;
image.data = wgpu_data.into_iter().flatten().collect::<Vec<_>>();
image.data = Some(wgpu_data.into_iter().flatten().collect::<Vec<_>>());
image.texture_descriptor.size = Extent3d {
width,
height,

View File

@ -18,6 +18,12 @@ pub enum TextureAtlasBuilderError {
NotEnoughSpace,
#[error("added a texture with the wrong format in an atlas")]
WrongFormat,
/// Attempted to add a texture to an uninitialzied atlas
#[error("cannot add texture to uninitialized atlas texture")]
UninitializedAtlas,
/// Attempted to add an uninitialized texture to an atlas
#[error("cannot add uninitialized texture to atlas")]
UninitializedSourceTexture,
}
#[derive(Debug)]
@ -105,7 +111,7 @@ impl<'a> TextureAtlasBuilder<'a> {
texture: &Image,
packed_location: &PackedLocation,
padding: UVec2,
) {
) -> TextureAtlasBuilderResult<()> {
let rect_width = (packed_location.width() - padding.x) as usize;
let rect_height = (packed_location.height() - padding.y) as usize;
let rect_x = packed_location.x() as usize;
@ -113,14 +119,20 @@ impl<'a> TextureAtlasBuilder<'a> {
let atlas_width = atlas_texture.width() as usize;
let format_size = atlas_texture.texture_descriptor.format.pixel_size();
let Some(ref mut atlas_data) = atlas_texture.data else {
return Err(TextureAtlasBuilderError::UninitializedAtlas);
};
let Some(ref data) = texture.data else {
return Err(TextureAtlasBuilderError::UninitializedSourceTexture);
};
for (texture_y, bound_y) in (rect_y..rect_y + rect_height).enumerate() {
let begin = (bound_y * atlas_width + rect_x) * format_size;
let end = begin + rect_width * format_size;
let texture_begin = texture_y * rect_width * format_size;
let texture_end = texture_begin + rect_width * format_size;
atlas_texture.data[begin..end]
.copy_from_slice(&texture.data[texture_begin..texture_end]);
atlas_data[begin..end].copy_from_slice(&data[texture_begin..texture_end]);
}
Ok(())
}
fn copy_converted_texture(
@ -128,9 +140,9 @@ impl<'a> TextureAtlasBuilder<'a> {
atlas_texture: &mut Image,
texture: &Image,
packed_location: &PackedLocation,
) {
) -> TextureAtlasBuilderResult<()> {
if self.format == texture.texture_descriptor.format {
Self::copy_texture_to_atlas(atlas_texture, texture, packed_location, self.padding);
Self::copy_texture_to_atlas(atlas_texture, texture, packed_location, self.padding)?;
} else if let Some(converted_texture) = texture.convert(self.format) {
debug!(
"Converting texture from '{:?}' to '{:?}'",
@ -141,13 +153,14 @@ impl<'a> TextureAtlasBuilder<'a> {
&converted_texture,
packed_location,
self.padding,
);
)?;
} else {
error!(
"Error converting texture from '{:?}' to '{:?}', ignoring",
texture.texture_descriptor.format, self.format
);
}
Ok(())
}
/// Consumes the builder, and returns the newly created texture atlas and
@ -274,7 +287,7 @@ impl<'a> TextureAtlasBuilder<'a> {
);
return Err(TextureAtlasBuilderError::WrongFormat);
}
self.copy_converted_texture(&mut atlas_texture, texture, packed_location);
self.copy_converted_texture(&mut atlas_texture, texture, packed_location)?;
}
Ok((

View File

@ -1732,7 +1732,7 @@ impl FromWorld for MeshPipeline {
let format_size = image.texture_descriptor.format.pixel_size();
render_queue.write_texture(
texture.as_image_copy(),
&image.data,
image.data.as_ref().expect("Image was created without data"),
TexelCopyBufferLayout {
offset: 0,
bytes_per_row: Some(image.width() * format_size as u32),

View File

@ -98,7 +98,7 @@ fn fallback_image_new(
RenderAssetUsages::RENDER_WORLD,
)
} else {
let mut image = Image::default();
let mut image = Image::default_uninit();
image.texture_descriptor.dimension = TextureDimension::D2;
image.texture_descriptor.size = extents;
image.texture_descriptor.format = format;
@ -114,7 +114,7 @@ fn fallback_image_new(
render_queue,
&image.texture_descriptor,
TextureDataOrder::default(),
&image.data,
&image.data.expect("Image has no data"),
)
} else {
render_device.create_texture(&image.texture_descriptor)

View File

@ -36,7 +36,7 @@ impl RenderAsset for GpuImage {
#[inline]
fn byte_len(image: &Self::SourceAsset) -> Option<usize> {
Some(image.data.len())
image.data.as_ref().map(Vec::len)
}
/// Converts the extracted image into a [`GpuImage`].
@ -45,13 +45,17 @@ impl RenderAsset for GpuImage {
_: AssetId<Self::SourceAsset>,
(render_device, render_queue, default_sampler): &mut SystemParamItem<Self::Param>,
) -> Result<Self, PrepareAssetError<Self::SourceAsset>> {
let texture = render_device.create_texture_with_data(
render_queue,
&image.texture_descriptor,
// TODO: Is this correct? Do we need to use `MipMajor` if it's a ktx2 file?
wgpu::util::TextureDataOrder::default(),
&image.data,
);
let texture = if let Some(ref data) = image.data {
render_device.create_texture_with_data(
render_queue,
&image.texture_descriptor,
// TODO: Is this correct? Do we need to use `MipMajor` if it's a ktx2 file?
wgpu::util::TextureDataOrder::default(),
data,
)
} else {
render_device.create_texture(&image.texture_descriptor)
};
let texture_view = texture.create_view(
image

View File

@ -359,7 +359,7 @@ impl FromWorld for Mesh2dPipeline {
let format_size = image.texture_descriptor.format.pixel_size();
render_queue.write_texture(
texture.as_image_copy(),
&image.data,
image.data.as_ref().expect("Image has no data"),
TexelCopyBufferLayout {
offset: 0,
bytes_per_row: Some(image.width() * format_size as u32),

View File

@ -102,7 +102,7 @@ impl FromWorld for SpritePipeline {
let format_size = image.texture_descriptor.format.pixel_size();
render_queue.write_texture(
texture.as_image_copy(),
&image.data,
image.data.as_ref().expect("Image has no data"),
TexelCopyBufferLayout {
offset: 0,
bytes_per_row: Some(image.width() * format_size as u32),

View File

@ -97,7 +97,7 @@ impl FontAtlas {
let atlas_layout = atlas_layouts.get_mut(&self.texture_atlas).unwrap();
let atlas_texture = textures.get_mut(&self.texture).unwrap();
if let Some(glyph_index) =
if let Ok(glyph_index) =
self.dynamic_texture_atlas_builder
.add_texture(atlas_layout, texture, atlas_texture)
{

View File

@ -145,18 +145,16 @@ pub(crate) fn extract_rgba_pixels(image: &Image) -> Option<Vec<u8>> {
| TextureFormat::Rgba8UnormSrgb
| TextureFormat::Rgba8Snorm
| TextureFormat::Rgba8Uint
| TextureFormat::Rgba8Sint => Some(image.data.clone()),
TextureFormat::Rgba32Float => Some(
image
.data
.chunks(4)
| TextureFormat::Rgba8Sint => Some(image.data.clone()?),
TextureFormat::Rgba32Float => image.data.as_ref().map(|data| {
data.chunks(4)
.map(|chunk| {
let chunk = chunk.try_into().unwrap();
let num = bytemuck::cast_ref::<[u8; 4], f32>(chunk);
ops::round(num.clamp(0.0, 1.0) * 255.0) as u8
})
.collect(),
),
.collect()
}),
_ => None,
}
}

View File

@ -499,15 +499,17 @@ fn update(
* img_bytes.texture_descriptor.format.pixel_size();
let aligned_row_bytes = RenderDevice::align_copy_bytes_per_row(row_bytes);
if row_bytes == aligned_row_bytes {
img_bytes.data.clone_from(&image_data);
img_bytes.data.as_mut().unwrap().clone_from(&image_data);
} else {
// shrink data to original image size
img_bytes.data = image_data
.chunks(aligned_row_bytes)
.take(img_bytes.height() as usize)
.flat_map(|row| &row[..row_bytes.min(row.len())])
.cloned()
.collect();
img_bytes.data = Some(
image_data
.chunks(aligned_row_bytes)
.take(img_bytes.height() as usize)
.flat_map(|row| &row[..row_bytes.min(row.len())])
.cloned()
.collect(),
);
}
// Create RGBA Image Buffer

View File

@ -127,7 +127,7 @@ fn alter_asset(mut images: ResMut<Assets<Image>>, left_bird: Single<&Sprite, Wit
return;
};
for pixel in &mut image.data {
for pixel in image.data.as_mut().unwrap() {
// Directly modify the asset data, which will affect all users of this asset. By
// contrast, mutating the handle (as we did above) affects only one copy. In this case,
// we'll just invert the colors, by way of demonstration. Notice that both uses of the

View File

@ -86,10 +86,11 @@ fn setup(
height: 1,
..default()
};
let mut image = Image::new_fill(
// We create an uninitialized image since this texture will only be used for getting data out
// of the compute shader, not getting data in, so there's no reason for it to exist on the CPU
let mut image = Image::new_uninit(
size,
TextureDimension::D2,
&[0, 0, 0, 0],
TextureFormat::R32Uint,
RenderAssetUsages::RENDER_WORLD,
);