From c31c03e32c8a15104aa490dc96c43fbacd3aa6fc Mon Sep 17 00:00:00 2001 From: Oleksiy Syvokon Date: Fri, 10 Apr 2026 20:42:12 +0300 Subject: [PATCH] gpui_wgpu: Fix WGPU panic from unsupported atlas texture formats 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. --- 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(-) diff --git a/crates/gpui_wgpu/src/wgpu_atlas.rs b/crates/gpui_wgpu/src/wgpu_atlas.rs index 55f6edee21b9f2da02268c66c665c34d5b52066a..4c2c6ab601442fbebabd6759c5111d5e852b509f 100644 --- a/crates/gpui_wgpu/src/wgpu_atlas.rs +++ b/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) -> etagere::Size { size2(size.width.0, size.height.0) } @@ -31,6 +33,7 @@ struct WgpuAtlasState { device: Arc, queue: Arc, max_texture_size: u32, + color_texture_format: wgpu::TextureFormat, storage: WgpuAtlasStorage, tiles_by_key: FxHashMap, pending_uploads: Vec, @@ -41,18 +44,31 @@ pub struct WgpuTextureInfo { } impl WgpuAtlas { - pub fn new(device: Arc, queue: Arc) -> Self { + pub fn new( + device: Arc, + queue: Arc, + 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, queue: Arc) { + 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, 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 { + 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] + ); + } } diff --git a/crates/gpui_wgpu/src/wgpu_context.rs b/crates/gpui_wgpu/src/wgpu_context.rs index 7c03c4752ebf2e76b04c384722f4a9c17054487a..39ac40ffd24c1e18222dabd3fdff255e31e9e5d9 100644 --- a/crates/gpui_wgpu/src/wgpu_context.rs +++ b/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, pub queue: Arc, dual_source_blending: bool, + color_texture_format: wgpu::TextureFormat, device_lost: Arc, } @@ -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, 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 { + 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 { diff --git a/crates/gpui_wgpu/src/wgpu_renderer.rs b/crates/gpui_wgpu/src/wgpu_renderer.rs index c25cba935447d76f0e112079b7c81a9463109806..c38de02707f9daf45b37c418f8c58fcd1805e29d 100644 --- a/crates/gpui_wgpu/src/wgpu_renderer.rs +++ b/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()),