gpui_wgpu: Fix WGPU panic from unsupported atlas texture formats

Oleksiy Syvokon created

We were assuming that `Bgra8Unorm` always works, which was causing the panic on devices that don't ("wgpu-hal invariant was violated: Requested feature is not available on this device")

This change makes a proper fallback to Rgba8Unorm. In the worst case, if a device doesn't support that as well (highly unlikely), we return a proper error instead of a panic.

Change summary

crates/gpui_wgpu/src/wgpu_atlas.rs    | 77 ++++++++++++++++++++++----
crates/gpui_wgpu/src/wgpu_context.rs  | 83 +++++++++++++++++++++++++---
crates/gpui_wgpu/src/wgpu_renderer.rs | 13 +---
3 files changed, 140 insertions(+), 33 deletions(-)

Detailed changes

crates/gpui_wgpu/src/wgpu_atlas.rs 🔗

@@ -8,6 +8,8 @@ use gpui::{
 use parking_lot::Mutex;
 use std::{borrow::Cow, ops, sync::Arc};
 
+use crate::WgpuContext;
+
 fn device_size_to_etagere(size: Size<DevicePixels>) -> etagere::Size {
     size2(size.width.0, size.height.0)
 }
@@ -31,6 +33,7 @@ struct WgpuAtlasState {
     device: Arc<wgpu::Device>,
     queue: Arc<wgpu::Queue>,
     max_texture_size: u32,
+    color_texture_format: wgpu::TextureFormat,
     storage: WgpuAtlasStorage,
     tiles_by_key: FxHashMap<AtlasKey, AtlasTile>,
     pending_uploads: Vec<PendingUpload>,
@@ -41,18 +44,31 @@ pub struct WgpuTextureInfo {
 }
 
 impl WgpuAtlas {
-    pub fn new(device: Arc<wgpu::Device>, queue: Arc<wgpu::Queue>) -> Self {
+    pub fn new(
+        device: Arc<wgpu::Device>,
+        queue: Arc<wgpu::Queue>,
+        color_texture_format: wgpu::TextureFormat,
+    ) -> Self {
         let max_texture_size = device.limits().max_texture_dimension_2d;
         WgpuAtlas(Mutex::new(WgpuAtlasState {
             device,
             queue,
             max_texture_size,
+            color_texture_format,
             storage: WgpuAtlasStorage::default(),
             tiles_by_key: Default::default(),
             pending_uploads: Vec::new(),
         }))
     }
 
+    pub fn from_context(context: &WgpuContext) -> Self {
+        Self::new(
+            context.device.clone(),
+            context.queue.clone(),
+            context.color_texture_format(),
+        )
+    }
+
     pub fn before_frame(&self) {
         let mut lock = self.0.lock();
         lock.flush_uploads();
@@ -68,10 +84,11 @@ impl WgpuAtlas {
 
     /// Handles device lost by clearing all textures and cached tiles.
     /// The atlas will lazily recreate textures as needed on subsequent frames.
-    pub fn handle_device_lost(&self, device: Arc<wgpu::Device>, queue: Arc<wgpu::Queue>) {
+    pub fn handle_device_lost(&self, context: &WgpuContext) {
         let mut lock = self.0.lock();
-        lock.device = device;
-        lock.queue = queue;
+        lock.device = context.device.clone();
+        lock.queue = context.queue.clone();
+        lock.color_texture_format = context.color_texture_format();
         lock.storage = WgpuAtlasStorage::default();
         lock.tiles_by_key.clear();
         lock.pending_uploads.clear();
@@ -167,8 +184,7 @@ impl WgpuAtlasState {
         let size = min_size.min(&max_atlas_size).max(&DEFAULT_ATLAS_SIZE);
         let format = match kind {
             AtlasTextureKind::Monochrome => wgpu::TextureFormat::R8Unorm,
-            AtlasTextureKind::Subpixel => wgpu::TextureFormat::Bgra8Unorm,
-            AtlasTextureKind::Polychrome => wgpu::TextureFormat::Bgra8Unorm,
+            AtlasTextureKind::Subpixel | AtlasTextureKind::Polychrome => self.color_texture_format,
         };
 
         let texture = self.device.create_texture(&wgpu::TextureDescriptor {
@@ -221,11 +237,14 @@ impl WgpuAtlasState {
     }
 
     fn upload_texture(&mut self, id: AtlasTextureId, bounds: Bounds<DevicePixels>, bytes: &[u8]) {
-        self.pending_uploads.push(PendingUpload {
-            id,
-            bounds,
-            data: bytes.to_vec(),
-        });
+        let data = self
+            .storage
+            .get(id)
+            .map(|texture| swizzle_upload_data(bytes, texture.format))
+            .unwrap_or_else(|| bytes.to_vec());
+
+        self.pending_uploads
+            .push(PendingUpload { id, bounds, data });
     }
 
     fn flush_uploads(&mut self) {
@@ -341,7 +360,7 @@ impl WgpuAtlasTexture {
     fn bytes_per_pixel(&self) -> u8 {
         match self.format {
             wgpu::TextureFormat::R8Unorm => 1,
-            wgpu::TextureFormat::Bgra8Unorm => 4,
+            wgpu::TextureFormat::Bgra8Unorm | wgpu::TextureFormat::Rgba8Unorm => 4,
             _ => 4,
         }
     }
@@ -355,6 +374,19 @@ impl WgpuAtlasTexture {
     }
 }
 
+fn swizzle_upload_data(bytes: &[u8], format: wgpu::TextureFormat) -> Vec<u8> {
+    match format {
+        wgpu::TextureFormat::Rgba8Unorm => {
+            let mut data = bytes.to_vec();
+            for pixel in data.chunks_exact_mut(4) {
+                pixel.swap(0, 2);
+            }
+            data
+        }
+        _ => bytes.to_vec(),
+    }
+}
+
 #[cfg(all(test, not(target_family = "wasm")))]
 mod tests {
     use super::*;
@@ -400,7 +432,7 @@ mod tests {
     fn before_frame_skips_uploads_for_removed_texture() -> anyhow::Result<()> {
         let (device, queue) = test_device_and_queue()?;
 
-        let atlas = WgpuAtlas::new(device, queue);
+        let atlas = WgpuAtlas::new(device, queue, wgpu::TextureFormat::Bgra8Unorm);
         let key = AtlasKey::Image(RenderImageParams {
             image_id: ImageId(1),
             frame_index: 0,
@@ -417,7 +449,24 @@ mod tests {
             .expect("tile should be created");
         atlas.remove(&key);
         atlas.before_frame();
-
         Ok(())
     }
+
+    #[test]
+    fn swizzle_upload_data_preserves_bgra_uploads() {
+        let input = vec![0x10, 0x20, 0x30, 0x40];
+        assert_eq!(
+            swizzle_upload_data(&input, wgpu::TextureFormat::Bgra8Unorm),
+            input
+        );
+    }
+
+    #[test]
+    fn swizzle_upload_data_converts_bgra_to_rgba() {
+        let input = vec![0x10, 0x20, 0x30, 0x40, 0xAA, 0xBB, 0xCC, 0xDD];
+        assert_eq!(
+            swizzle_upload_data(&input, wgpu::TextureFormat::Rgba8Unorm),
+            vec![0x30, 0x20, 0x10, 0x40, 0xCC, 0xBB, 0xAA, 0xDD]
+        );
+    }
 }

crates/gpui_wgpu/src/wgpu_context.rs 🔗

@@ -4,6 +4,7 @@ use anyhow::Context as _;
 use gpui_util::ResultExt;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
+use wgpu::TextureFormat;
 
 pub struct WgpuContext {
     pub instance: wgpu::Instance,
@@ -11,6 +12,7 @@ pub struct WgpuContext {
     pub device: Arc<wgpu::Device>,
     pub queue: Arc<wgpu::Queue>,
     dual_source_blending: bool,
+    color_texture_format: wgpu::TextureFormat,
     device_lost: Arc<AtomicBool>,
 }
 
@@ -41,7 +43,7 @@ impl WgpuContext {
 
         // Select an adapter by actually testing surface configuration with the real device.
         // This is the only reliable way to determine compatibility on hybrid GPU systems.
-        let (adapter, device, queue, dual_source_blending) =
+        let (adapter, device, queue, dual_source_blending, color_texture_format) =
             pollster::block_on(Self::select_adapter_and_device(
                 &instance,
                 device_id_filter,
@@ -72,6 +74,7 @@ impl WgpuContext {
             device: Arc::new(device),
             queue: Arc::new(queue),
             dual_source_blending,
+            color_texture_format,
             device_lost,
         })
     }
@@ -116,7 +119,7 @@ impl WgpuContext {
 
     async fn create_device(
         adapter: &wgpu::Adapter,
-    ) -> anyhow::Result<(wgpu::Device, wgpu::Queue, bool)> {
+    ) -> anyhow::Result<(wgpu::Device, wgpu::Queue, bool, TextureFormat)> {
         let dual_source_blending = adapter
             .features()
             .contains(wgpu::Features::DUAL_SOURCE_BLENDING);
@@ -131,6 +134,8 @@ impl WgpuContext {
             );
         }
 
+        let color_atlas_texture_format = Self::select_color_texture_format(adapter)?;
+
         let (device, queue) = adapter
             .request_device(&wgpu::DeviceDescriptor {
                 label: Some("gpui_device"),
@@ -145,7 +150,12 @@ impl WgpuContext {
             .await
             .map_err(|e| anyhow::anyhow!("Failed to create wgpu device: {e}"))?;
 
-        Ok((device, queue, dual_source_blending))
+        Ok((
+            device,
+            queue,
+            dual_source_blending,
+            color_atlas_texture_format,
+        ))
     }
 
     #[cfg(not(target_family = "wasm"))]
@@ -185,7 +195,13 @@ impl WgpuContext {
         device_id_filter: Option<u32>,
         surface: &wgpu::Surface<'_>,
         compositor_gpu: Option<&CompositorGpuHint>,
-    ) -> anyhow::Result<(wgpu::Adapter, wgpu::Device, wgpu::Queue, bool)> {
+    ) -> anyhow::Result<(
+        wgpu::Adapter,
+        wgpu::Device,
+        wgpu::Queue,
+        bool,
+        TextureFormat,
+    )> {
         let mut adapters: Vec<_> = instance.enumerate_adapters(wgpu::Backends::all()).await;
 
         if adapters.is_empty() {
@@ -269,13 +285,19 @@ impl WgpuContext {
             log::info!("Testing adapter: {} ({:?})...", info.name, info.backend);
 
             match Self::try_adapter_with_surface(&adapter, surface).await {
-                Ok((device, queue, dual_source_blending)) => {
+                Ok((device, queue, dual_source_blending, color_atlas_texture_format)) => {
                     log::info!(
                         "Selected GPU (passed configuration test): {} ({:?})",
                         info.name,
                         info.backend
                     );
-                    return Ok((adapter, device, queue, dual_source_blending));
+                    return Ok((
+                        adapter,
+                        device,
+                        queue,
+                        dual_source_blending,
+                        color_atlas_texture_format,
+                    ));
                 }
                 Err(e) => {
                     log::info!(
@@ -297,7 +319,7 @@ impl WgpuContext {
     async fn try_adapter_with_surface(
         adapter: &wgpu::Adapter,
         surface: &wgpu::Surface<'_>,
-    ) -> anyhow::Result<(wgpu::Device, wgpu::Queue, bool)> {
+    ) -> anyhow::Result<(wgpu::Device, wgpu::Queue, bool, TextureFormat)> {
         let caps = surface.get_capabilities(adapter);
         if caps.formats.is_empty() {
             anyhow::bail!("no compatible surface formats");
@@ -306,7 +328,8 @@ impl WgpuContext {
             anyhow::bail!("no compatible alpha modes");
         }
 
-        let (device, queue, dual_source_blending) = Self::create_device(adapter).await?;
+        let (device, queue, dual_source_blending, color_atlas_texture_format) =
+            Self::create_device(adapter).await?;
         let error_scope = device.push_error_scope(wgpu::ErrorFilter::Validation);
 
         let test_config = wgpu::SurfaceConfiguration {
@@ -327,13 +350,55 @@ impl WgpuContext {
             anyhow::bail!("surface configuration failed: {e}");
         }
 
-        Ok((device, queue, dual_source_blending))
+        Ok((
+            device,
+            queue,
+            dual_source_blending,
+            color_atlas_texture_format,
+        ))
     }
 
+    fn select_color_texture_format(adapter: &wgpu::Adapter) -> anyhow::Result<wgpu::TextureFormat> {
+        let required_usages = wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST;
+        let bgra_features = adapter.get_texture_format_features(wgpu::TextureFormat::Bgra8Unorm);
+        if bgra_features.allowed_usages.contains(required_usages) {
+            return Ok(wgpu::TextureFormat::Bgra8Unorm);
+        }
+
+        let rgba_features = adapter.get_texture_format_features(wgpu::TextureFormat::Rgba8Unorm);
+        if rgba_features.allowed_usages.contains(required_usages) {
+            let info = adapter.get_info();
+            log::warn!(
+                "Adapter {} ({:?}) does not support Bgra8Unorm atlas textures with usages {:?}; \
+                 falling back to Rgba8Unorm atlas textures.",
+                info.name,
+                info.backend,
+                required_usages,
+            );
+            return Ok(wgpu::TextureFormat::Rgba8Unorm);
+        }
+
+        let info = adapter.get_info();
+        Err(anyhow::anyhow!(
+            "Adapter {} ({:?}, device={:#06x}) does not support a usable color atlas texture \
+             format with usages {:?}. Bgra8Unorm allowed usages: {:?}; \
+             Rgba8Unorm allowed usages: {:?}.",
+            info.name,
+            info.backend,
+            info.device,
+            required_usages,
+            bgra_features.allowed_usages,
+            rgba_features.allowed_usages,
+        ))
+    }
     pub fn supports_dual_source_blending(&self) -> bool {
         self.dual_source_blending
     }
 
+    pub fn color_texture_format(&self) -> wgpu::TextureFormat {
+        self.color_texture_format
+    }
+
     /// Returns true if the GPU device was lost (e.g., due to driver crash, suspend/resume).
     /// When this returns true, the context should be recreated.
     pub fn device_lost(&self) -> bool {

crates/gpui_wgpu/src/wgpu_renderer.rs 🔗

@@ -217,10 +217,7 @@ impl WgpuRenderer {
             None => ctx_ref.insert(WgpuContext::new(instance, &surface, compositor_gpu)?),
         };
 
-        let atlas = Arc::new(WgpuAtlas::new(
-            Arc::clone(&context.device),
-            Arc::clone(&context.queue),
-        ));
+        let atlas = Arc::new(WgpuAtlas::from_context(context));
 
         Self::new_internal(
             Some(Rc::clone(&gpu_context)),
@@ -243,10 +240,7 @@ impl WgpuRenderer {
             .create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
             .map_err(|e| anyhow::anyhow!("Failed to create surface: {e}"))?;
 
-        let atlas = Arc::new(WgpuAtlas::new(
-            Arc::clone(&context.device),
-            Arc::clone(&context.queue),
-        ));
+        let atlas = Arc::new(WgpuAtlas::from_context(context));
 
         Self::new_internal(None, context, surface, config, None, atlas)
     }
@@ -1807,8 +1801,7 @@ impl WgpuRenderer {
         let context = ctx_ref.as_ref().expect("context should exist");
 
         self.resources = None;
-        self.atlas
-            .handle_device_lost(Arc::clone(&context.device), Arc::clone(&context.queue));
+        self.atlas.handle_device_lost(context);
 
         *self = Self::new_internal(
             Some(gpu_context.clone()),