Skip to content

Commit

Permalink
Fixed a bug where skybox ddsfile would crash from wgpu (#12894)
Browse files Browse the repository at this point in the history
Fixed a bug where skybox ddsfile would crash from wgpu while trying to
read past the file buffer.
Added a unit-test to prevent regression.
Bumped ddsfile dependency version to 0.5.2

# Objective

Prevents a crash when loading dds skybox.

## Solution

ddsfile already automatically sets array layers to be 6 for skyboxes.
Removed bevy's extra *= 6 multiplication.

---

This is a copy of
[#12598](#12598) ... I made that
one off of main and wasn't able to make more pull requests without
making a new branch.

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
  • Loading branch information
Hexorg and mockersf committed Apr 8, 2024
1 parent 5570315 commit b9a2329
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 8 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_render/Cargo.toml
Expand Up @@ -86,7 +86,7 @@ thread_local = "1.1"
thiserror = "1.0"
futures-lite = "2.0.1"
hexasphere = "10.0"
ddsfile = { version = "0.5.0", optional = true }
ddsfile = { version = "0.5.2", optional = true }
ktx2 = { version = "0.3.0", optional = true }
# For ktx2 supercompression
flate2 = { version = "1.0.22", optional = true }
Expand Down
106 changes: 99 additions & 7 deletions crates/bevy_render/src/texture/dds.rs
Expand Up @@ -24,23 +24,22 @@ pub fn dds_buffer_to_image(
}
let mut image = Image::default();
let is_cubemap = dds.header.caps2.contains(Caps2::CUBEMAP);
let mut depth_or_array_layers = if dds.get_num_array_layers() > 1 {
let depth_or_array_layers = if dds.get_num_array_layers() > 1 {
dds.get_num_array_layers()
} else {
dds.get_depth()
};
if is_cubemap {
if !dds.header.caps2.contains(
if is_cubemap
&& !dds.header.caps2.contains(
Caps2::CUBEMAP_NEGATIVEX
| Caps2::CUBEMAP_NEGATIVEY
| Caps2::CUBEMAP_NEGATIVEZ
| Caps2::CUBEMAP_POSITIVEX
| Caps2::CUBEMAP_POSITIVEY
| Caps2::CUBEMAP_POSITIVEZ,
) {
return Err(TextureError::IncompleteCubemap);
}
depth_or_array_layers *= 6;
)
{
return Err(TextureError::IncompleteCubemap);
}
image.texture_descriptor.size = Extent3d {
width: dds.get_width(),
Expand Down Expand Up @@ -277,3 +276,96 @@ pub fn dds_format_to_texture_format(
));
})
}

#[cfg(test)]
mod test {
use wgpu::{util::TextureDataOrder, TextureDescriptor, TextureDimension};

use crate::texture::CompressedImageFormats;

use super::dds_buffer_to_image;

/// `wgpu::create_texture_with_data` that reads from data structure but doesn't actually talk to your GPU
fn fake_wgpu_create_texture_with_data(desc: &TextureDescriptor<'_>, data: &[u8]) {
// Will return None only if it's a combined depth-stencil format
// If so, default to 4, validation will fail later anyway since the depth or stencil
// aspect needs to be written to individually
let block_size = desc.format.block_copy_size(None).unwrap_or(4);
let (block_width, block_height) = desc.format.block_dimensions();
let layer_iterations = desc.array_layer_count();

let outer_iteration;
let inner_iteration;
match TextureDataOrder::default() {
TextureDataOrder::LayerMajor => {
outer_iteration = layer_iterations;
inner_iteration = desc.mip_level_count;
}
TextureDataOrder::MipMajor => {
outer_iteration = desc.mip_level_count;
inner_iteration = layer_iterations;
}
}

let mut binary_offset = 0;
for outer in 0..outer_iteration {
for inner in 0..inner_iteration {
let (_layer, mip) = match TextureDataOrder::default() {
TextureDataOrder::LayerMajor => (outer, inner),
TextureDataOrder::MipMajor => (inner, outer),
};

let mut mip_size = desc.mip_level_size(mip).unwrap();
// copying layers separately
if desc.dimension != TextureDimension::D3 {
mip_size.depth_or_array_layers = 1;
}

// When uploading mips of compressed textures and the mip is supposed to be
// a size that isn't a multiple of the block size, the mip needs to be uploaded
// as its "physical size" which is the size rounded up to the nearest block size.
let mip_physical = mip_size.physical_size(desc.format);

// All these calculations are performed on the physical size as that's the
// data that exists in the buffer.
let width_blocks = mip_physical.width / block_width;
let height_blocks = mip_physical.height / block_height;

let bytes_per_row = width_blocks * block_size;
let data_size = bytes_per_row * height_blocks * mip_size.depth_or_array_layers;

let end_offset = binary_offset + data_size as usize;

assert!(binary_offset < data.len());
assert!(end_offset <= data.len());
// those asserts match how the data will be accessed by wgpu:
// data[binary_offset..end_offset])

binary_offset = end_offset;
}
}
}

#[test]
fn dds_skybox() {
let buffer: [u8; 224] = [
0x44, 0x44, 0x53, 0x20, 0x7c, 0, 0, 0, 7, 0x10, 0x08, 0, 4, 0, 0, 0, 4, 0, 0, 0, 0x10,
0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0x47, 0x49, 0x4d, 0x50, 0x2d, 0x44, 0x44, 0x53, 0x5c,
0x09, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0x20, 0, 0, 0, 4, 0, 0, 0, 0x44, 0x58, 0x54, 0x35, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x08, 0x10, 0, 0, 0, 0xfe, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0x49, 0x92, 0x24, 0x49, 0x92, 0x24, 0xda, 0xd6,
0x2f, 0x5b, 0x8a, 0, 0xff, 0x55, 0xff, 0xff, 0x49, 0x92, 0x24, 0x49, 0x92, 0x24, 0xd5,
0x84, 0x8e, 0x3a, 0xb7, 0, 0xaa, 0x55, 0xff, 0xff, 0x49, 0x92, 0x24, 0x49, 0x92, 0x24,
0xf5, 0x94, 0x6f, 0x32, 0x57, 0xb7, 0x8b, 0, 0xff, 0xff, 0x49, 0x92, 0x24, 0x49, 0x92,
0x24, 0x2c, 0x3a, 0x49, 0x19, 0x28, 0xf7, 0xd7, 0xbe, 0xff, 0xff, 0x49, 0x92, 0x24,
0x49, 0x92, 0x24, 0x16, 0x95, 0xae, 0x42, 0xfc, 0, 0xaa, 0x55, 0xff, 0xff, 0x49, 0x92,
0x24, 0x49, 0x92, 0x24, 0xd8, 0xad, 0xae, 0x42, 0xaf, 0x0a, 0xaa, 0x55,
];
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);
}
}
}

0 comments on commit b9a2329

Please sign in to comment.