Fixes for Skybox and All Examples (#4780)

CHANGELOG.md

@@ -44,6 +44,10 @@ Bottom level categories:
 
 This adds a way to allow a Vulkan driver which is non-compliant per VK_KHR_driver_properties to be enumerated. This is intended for testing new Vulkan drivers which are not Vulkan compliant yet.
 
+### `DeviceExt::create_texture_with_data` Allows Mip-Major Data
+
+Previously, `DeviceExt::create_texture_with_data` only allowed data to be provided in layer major order. There is now a `order` parameter which allows you to specify if the data is in layer major or mip major order.
+
 ### New Features
 
 #### General

Cargo.lock

@@ -935,18 +935,6 @@ version = "2.5.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7e962a19be5cfc3f3bf6dd8f61eb50107f356ad6270fbb3ed41476571db78be5"
 
-[[package]]
-name = "ddsfile"
-version = "0.5.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "479dfe1e6737aa9e96c6ac7b69689dc4c32da8383f2c12744739d76afa8b66c4"
-dependencies = [
- "bitflags 2.4.1",
- "byteorder",
- "enum-primitive-derive",
- "num-traits",
-]
-
 [[package]]
 name = "debugid"
 version = "0.8.0"
@@ -1209,17 +1197,6 @@ dependencies = [
  "cfg-if",
 ]
 
-[[package]]
-name = "enum-primitive-derive"
-version = "0.2.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c375b9c5eadb68d0a6efee2999fef292f45854c3444c86f09d8ab086ba942b0e"
-dependencies = [
- "num-traits",
- "quote",
- "syn 1.0.109",
-]
-
 [[package]]
 name = "env_logger"
 version = "0.10.1"
@@ -1914,6 +1891,15 @@ version = "3.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e2db585e1d738fc771bf08a151420d3ed193d9d895a36df7f6f8a9456b911ddc"
 
+[[package]]
+name = "ktx2"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "87d65e08a9ec02e409d27a0139eaa6b9756b4d81fe7cde71f6941a83730ce838"
+dependencies = [
+ "bitflags 1.3.2",
+]
+
 [[package]]
 name = "lazy-regex"
 version = "3.1.0"
@@ -4119,7 +4105,6 @@ dependencies = [
  "cfg-if",
  "console_error_panic_hook",
  "console_log",
- "ddsfile",
  "encase",
  "env_logger",
  "fern",
@@ -4127,6 +4112,7 @@ dependencies = [
  "getrandom",
  "glam",
  "js-sys",
+ "ktx2",
  "log",
  "nanorand",
  "noise",

Cargo.toml

@@ -77,7 +77,6 @@ cfg_aliases = "0.1"
 cfg-if = "1"
 codespan-reporting = "0.11"
 ctor = "0.2"
-ddsfile = "0.5.2"
 encase = "0.6"
 env_logger = "0.10"
 fern = "0.6"
@@ -88,6 +87,7 @@ getrandom = "0.2"
 glam = "0.24.2"
 heck = "0.4.0"
 image = { version = "0.24", default-features = false, features = ["png"] }
+ktx2 = "0.3"
 # libloading 0.8 switches from `winapi` to `windows-sys`; permit either
 libloading = ">=0.7, <0.9"
 libc = "0.2"

examples/Cargo.toml

@@ -22,11 +22,11 @@ test = false
 [dependencies]
 bytemuck.workspace = true
 cfg-if.workspace = true
-ddsfile.workspace = true
 encase = { workspace = true, features = ["glam"] }
 flume.workspace = true
 getrandom.workspace = true
 glam.workspace = true
+ktx2.workspace = true
 log.workspace = true
 nanorand.workspace = true
 noise.workspace = true

examples/src/framework.rs

@@ -4,7 +4,7 @@ use wgpu::{Instance, Surface};
 use winit::{
     dpi::PhysicalSize,
     event::{Event, KeyEvent, StartCause, WindowEvent},
-    event_loop::{ControlFlow, EventLoop, EventLoopWindowTarget},
+    event_loop::{EventLoop, EventLoopWindowTarget},
     keyboard::{Key, NamedKey},
     window::Window,
 };
@@ -218,7 +218,17 @@ impl SurfaceWrapper {
 
         match surface.get_current_texture() {
             Ok(frame) => frame,
-            Err(_) => {
+            // If we timed out, just try again
+            Err(wgpu::SurfaceError::Timeout) => surface
+                .get_current_texture()
+                .expect("Failed to acquire next surface texture!"),
+            Err(
+                // If the surface is outdated, or was lost, reconfigure it.
+                wgpu::SurfaceError::Outdated
+                | wgpu::SurfaceError::Lost
+                // If OutOfMemory happens, reconfiguring may not help, but we might as well try
+                | wgpu::SurfaceError::OutOfMemory,
+            ) => {
                 surface.configure(&context.device, self.config());
                 surface
                     .get_current_texture()
@@ -380,9 +390,6 @@ async fn start<E: Example>(title: &str) {
     let _ = (event_loop_function)(
         window_loop.event_loop,
         move |event: Event<()>, target: &EventLoopWindowTarget<()>| {
-            // We set to refresh as fast as possible.
-            target.set_control_flow(ControlFlow::Poll);
-
             match event {
                 ref e if SurfaceWrapper::start_condition(e) => {
                     surface.resume(&context, window_loop.window.clone(), E::SRGB);

examples/src/skybox/images/generation.bash

@@ -0,0 +1,45 @@
+# Needs montage from ImageMagick in PATH
+# Needs compressonatorcli.exe from https://github.com/GPUOpen-Tools/compressonator in PATH
+# Needs PVRTexToolCLI.exe from https://developer.imaginationtech.com/pvrtextool/ in PATH
+
+# Generate a skybox image from 6 jpeg in the folder in first argument.
+# The images must be named right.jpg, left.jpg, top.jpg, bottom.jpg, back.jpg, front.jpg
+#
+# Must be called from the root of the project.
+#
+# bash examples/src/skybox/images/generation.bash ./path/to/images/folder
+
+SCRIPT_DIRECTORY=examples/src/skybox/images
+CHUNK_SIZE="256x256"
+
+set -e
+
+# ensure the script is called from the root of the project
+if [ ! -f "$SCRIPT_DIRECTORY/generation.bash" ]; then
+    echo "The script must be called from the root of the project!"
+    exit 1
+fi
+
+# ensure an argument is passed
+if [ $# -eq 0 ]; then
+    echo "No arguments supplied!"
+    echo
+    echo "Usage: bash examples/src/skybox/images/generation.bash ./path/to/images/folder"
+    exit 1
+fi
+
+TEMP=examples/src/skybox/images/tmp
+
+mkdir -p $TEMP
+# resize images to 256x256
+magick mogrify -path $TEMP -resize 256x256 -format png $1/*.jpg
+# create an uncompressed ktx2 cubemap file
+PVRTexToolCLI.exe -i $TEMP/right.png,$TEMP/left.png,$TEMP/top.png,$TEMP/bottom.png,$TEMP/front.png,$TEMP/back.png -ics SRGB -cube -m -f r8g8b8a8,UBN,SRGB -o $SCRIPT_DIRECTORY/rgba8.ktx2
+# create the bc7 compressed ktx2 cubemap files using compressonator
+compressonatorcli.exe -fd BC7 $SCRIPT_DIRECTORY/rgba8.ktx2 $SCRIPT_DIRECTORY/bc7.ktx2
+# create the etc2 and astc compressed ktx2 cubemap file using PVRTexTool
+#
+# compressonator has support for etc2, but the result looks terrible.
+PVRTexToolCLI.exe -i $SCRIPT_DIRECTORY/rgba8.ktx2 -ics srgb -m -f ETC2_RGB_A1,UBN,SRGB -q etcslow -o $SCRIPT_DIRECTORY/etc2.ktx2
+PVRTexToolCLI.exe -i $SCRIPT_DIRECTORY/rgba8.ktx2 -ics srgb -m -f ASTC_4X4,UBN,SRGB -q astcexhaustive -o $SCRIPT_DIRECTORY/astc.ktx2
+rm -r $TEMP

examples/src/skybox/mod.rs

@@ -2,7 +2,7 @@ use bytemuck::{Pod, Zeroable};
 use std::{borrow::Cow, f32::consts};
 use wgpu::{util::DeviceExt, AstcBlock, AstcChannel};
 
-const IMAGE_SIZE: u32 = 128;
+const IMAGE_SIZE: u32 = 256;
 
 #[derive(Clone, Copy, Pod, Zeroable)]
 #[repr(C)]
@@ -266,20 +266,20 @@ impl crate::framework::Example for Example {
         let device_features = device.features();
 
         let skybox_format = if device_features.contains(wgpu::Features::TEXTURE_COMPRESSION_ASTC) {
-            log::info!("Using ASTC");
+            log::info!("Using astc");
             wgpu::TextureFormat::Astc {
                 block: AstcBlock::B4x4,
                 channel: AstcChannel::UnormSrgb,
             }
         } else if device_features.contains(wgpu::Features::TEXTURE_COMPRESSION_ETC2) {
-            log::info!("Using ETC2");
-            wgpu::TextureFormat::Etc2Rgb8UnormSrgb
+            log::info!("Using etc2");
+            wgpu::TextureFormat::Etc2Rgb8A1UnormSrgb
         } else if device_features.contains(wgpu::Features::TEXTURE_COMPRESSION_BC) {
-            log::info!("Using BC");
-            wgpu::TextureFormat::Bc1RgbaUnormSrgb
+            log::info!("Using bc7");
+            wgpu::TextureFormat::Bc7RgbaUnormSrgb
         } else {
-            log::info!("Using plain");
-            wgpu::TextureFormat::Bgra8UnormSrgb
+            log::info!("Using rgba8");
+            wgpu::TextureFormat::Rgba8UnormSrgb
         };
 
         let size = wgpu::Extent3d {
@@ -306,20 +306,26 @@ impl crate::framework::Example for Example {
             wgpu::TextureFormat::Astc {
                 block: AstcBlock::B4x4,
                 channel: AstcChannel::UnormSrgb,
-            } => &include_bytes!("images/astc.dds")[..],
-            wgpu::TextureFormat::Etc2Rgb8UnormSrgb => &include_bytes!("images/etc2.dds")[..],
-            wgpu::TextureFormat::Bc1RgbaUnormSrgb => &include_bytes!("images/bc1.dds")[..],
-            wgpu::TextureFormat::Bgra8UnormSrgb => &include_bytes!("images/bgra.dds")[..],
+            } => &include_bytes!("images/astc.ktx2")[..],
+            wgpu::TextureFormat::Etc2Rgb8A1UnormSrgb => &include_bytes!("images/etc2.ktx2")[..],
+            wgpu::TextureFormat::Bc7RgbaUnormSrgb => &include_bytes!("images/bc7.ktx2")[..],
+            wgpu::TextureFormat::Rgba8UnormSrgb => &include_bytes!("images/rgba8.ktx2")[..],
             _ => unreachable!(),
         };
 
-        let image = ddsfile::Dds::read(&mut std::io::Cursor::new(&bytes)).unwrap();
+        let reader = ktx2::Reader::new(bytes).unwrap();
+        let header = reader.header();
+
+        let mut image = Vec::with_capacity(reader.data().len());
+        for level in reader.levels() {
+            image.extend_from_slice(level);
+        }
 
         let texture = device.create_texture_with_data(
             queue,
             &wgpu::TextureDescriptor {
                 size,
-                mip_level_count: max_mips,
+                mip_level_count: header.level_count,
                 sample_count: 1,
                 dimension: wgpu::TextureDimension::D2,
                 format: skybox_format,
@@ -327,7 +333,9 @@ impl crate::framework::Example for Example {
                 label: None,
                 view_formats: &[],
             },
-            &image.data,
+            // KTX2 stores mip levels in mip major order.
+            wgpu::util::TextureDataOrder::MipMajor,
+            &image,
         );
 
         let texture_view = texture.create_view(&wgpu::TextureViewDescriptor {
@@ -477,8 +485,8 @@ static TEST: crate::framework::ExampleTestParams = crate::framework::ExampleTest
 #[cfg(test)]
 #[wgpu_test::gpu_test]
 static TEST_BCN: crate::framework::ExampleTestParams = crate::framework::ExampleTestParams {
-    name: "skybox-bc1",
-    image_path: "/examples/src/skybox/screenshot_bc1.png",
+    name: "skybox-bc7",
+    image_path: "/examples/src/skybox/screenshot_bc7.png",
     width: 1024,
     height: 768,
     optional_features: wgpu::Features::TEXTURE_COMPRESSION_BC,

tests/tests/mem_leaks.rs

@@ -148,6 +148,7 @@ fn draw_test_with_reports(
             usage: wgpu::TextureUsages::RENDER_ATTACHMENT | wgpu::TextureUsages::COPY_DST,
             view_formats: &[],
         },
+        wgpu::util::TextureDataOrder::LayerMajor,
         &[0, 0, 0, 1],
     );
     let texture_view = texture.create_view(&wgpu::TextureViewDescriptor::default());

tests/tests/shader_view_format/mod.rs

@@ -76,6 +76,7 @@ fn reinterpret(
             sample_count: 1,
             view_formats: &[reinterpret_to],
         },
+        wgpu::util::TextureDataOrder::LayerMajor,
         bytemuck::cast_slice(src_data),
     );
     let tv = tex.create_view(&wgpu::TextureViewDescriptor {

tests/tests/vertex_indices/mod.rs

@@ -316,6 +316,7 @@ fn vertex_index_common(ctx: TestingContext) {
                 usage: wgpu::TextureUsages::RENDER_ATTACHMENT | wgpu::TextureUsages::COPY_DST,
                 view_formats: &[],
             },
+            wgpu::util::TextureDataOrder::LayerMajor,
             &[0, 0, 0, 1],
         )
         .create_view(&wgpu::TextureViewDescriptor::default());

wgpu/src/util/device.rs

@@ -10,6 +10,34 @@ pub struct BufferInitDescriptor<'a> {
     pub usage: crate::BufferUsages,
 }
 
+/// Order in which TextureData is laid out in memory.
+#[derive(Clone, Copy, Default, Debug, PartialEq, Eq, Hash)]
+pub enum TextureDataOrder {
+    /// The texture is laid out densely in memory as:
+    ///
+    /// ```text
+    /// Layer0Mip0 Layer0Mip1 Layer0Mip2
+    /// Layer1Mip0 Layer1Mip1 Layer1Mip2
+    /// Layer2Mip0 Layer2Mip1 Layer2Mip2
+    /// ````
+    ///
+    /// This is the layout used by dds files.
+    ///
+    /// This was the previous behavior of [`DeviceExt::create_texture_with_data`].
+    #[default]
+    LayerMajor,
+    /// The texture is laid out densely in memory as:
+    ///
+    /// ```text
+    /// Layer0Mip0 Layer1Mip0 Layer2Mip0
+    /// Layer0Mip1 Layer1Mip1 Layer2Mip1
+    /// Layer0Mip2 Layer1Mip2 Layer2Mip2
+    /// ```
+    ///
+    /// This is the layout used by ktx and ktx2 files.
+    MipMajor,
+}
+
 /// Utility methods not meant to be in the main API.
 pub trait DeviceExt {
     /// Creates a [Buffer](crate::Buffer) with data to initialize it.
@@ -19,11 +47,7 @@ pub trait DeviceExt {
     ///
     /// Expects all mipmaps to be tightly packed in the data buffer.
     ///
-    /// If the texture is a 2DArray texture, uploads each layer in order, expecting
-    /// each layer and its mips to be tightly packed.
-    ///
-    /// Example:
-    /// Layer0Mip0 Layer0Mip1 Layer0Mip2 ... Layer1Mip0 Layer1Mip1 Layer1Mip2 ...
+    /// See [`TextureDataOrder`] for the order in which the data is laid out in memory.
     ///
     /// Implicitly adds the `COPY_DST` usage if it is not present in the descriptor,
     /// as it is required to be able to upload the data to the gpu.
@@ -31,6 +55,7 @@ pub trait DeviceExt {
         &self,
         queue: &crate::Queue,
         desc: &crate::TextureDescriptor<'_>,
+        order: TextureDataOrder,
         data: &[u8],
     ) -> crate::Texture;
 }
@@ -78,6 +103,7 @@ impl DeviceExt for crate::Device {
         &self,
         queue: &crate::Queue,
         desc: &crate::TextureDescriptor<'_>,
+        order: TextureDataOrder,
         data: &[u8],
     ) -> crate::Texture {
         // Implicitly add the COPY_DST usage
@@ -92,9 +118,27 @@ impl DeviceExt for crate::Device {
         let (block_width, block_height) = desc.format.block_dimensions();
         let layer_iterations = desc.array_layer_count();
 
+        let outer_iteration;
+        let inner_iteration;
+        match order {
+            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 layer in 0..layer_iterations {
-            for mip in 0..desc.mip_level_count {
+        for outer in 0..outer_iteration {
+            for inner in 0..inner_iteration {
+                let (layer, mip) = match order {
+                    TextureDataOrder::LayerMajor => (outer, inner),
+                    TextureDataOrder::MipMajor => (inner, outer),
+                };
+
                 let mut mip_size = desc.mip_level_size(mip).unwrap();
                 // copying layers separately
                 if desc.dimension != wgt::TextureDimension::D3 {

wgpu/src/util/mod.rs

@@ -16,7 +16,7 @@ use std::{
 };
 
 pub use belt::StagingBelt;
-pub use device::{BufferInitDescriptor, DeviceExt};
+pub use device::{BufferInitDescriptor, DeviceExt, TextureDataOrder};
 pub use encoder::RenderEncoder;
 pub use init::*;
 pub use wgt::{math::*, DispatchIndirectArgs, DrawIndexedIndirectArgs, DrawIndirectArgs};