Never crash: the GPU compositor is panic-free (Rule 9) (#55)

- A failed readback mapping (e.g. device lost) now returns Unsupported,
  so the caller falls back to the CPU compositor, instead of
  expect("mapped readback buffer").
- Pipeline creation skips kernels without an entry point instead of
  expect; resident/effect entries are looked up with let-else after the
  insert; effect-program targets are Option instead of indexing +
  expect; copies without a source are skipped; the planner returns
  Unsupported instead of unreachable! for an adjustment layer.
- gpu denies unwrap/expect/panic/unimplemented/todo/unreachable.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas 2026-10-04 13:39:48 -04:00 committed by GitHub
parent 4218d07f40
commit f8f48db78a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 31 additions and 19 deletions

View File

@ -616,6 +616,7 @@ pub(crate) fn group_key(layer: &Layer, light: &GlobalLight) -> u64 {
} }
#[cfg(test)] #[cfg(test)]
#[allow(clippy::unreachable)] // clippy.toml exempts unwrap/expect/panic in tests, not unreachable!
mod tests { mod tests {
use super::*; use super::*;
use photocraft_color::BlendMode; use photocraft_color::BlendMode;

View File

@ -23,6 +23,7 @@
//! (Multichannel documents, documents or effect regions larger than the device's texture limit) //! (Multichannel documents, documents or effect regions larger than the device's texture limit)
//! returns [`Unsupported`]; callers fall back to the CPU compositor. //! returns [`Unsupported`]; callers fall back to the CPU compositor.
#![forbid(unsafe_code)] #![forbid(unsafe_code)]
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
pub mod bounds; pub mod bounds;
mod fx; mod fx;
@ -444,9 +445,10 @@ impl Compositor {
bind_group_layouts: &[Some(&bgl0), Some(&bgl1)], bind_group_layouts: &[Some(&bgl0), Some(&bgl1)],
immediate_size: 0, immediate_size: 0,
}); });
// `None` only for kernels without a fragment entry point, which are never drawn.
let pipeline = |k: Kernel, format: wgpu::TextureFormat| { let pipeline = |k: Kernel, format: wgpu::TextureFormat| {
let entry = k.entry().expect("drawn kernels have an entry point"); let entry = k.entry()?;
device.create_render_pipeline(&wgpu::RenderPipelineDescriptor { Some(device.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
label: Some(entry), label: Some(entry),
layout: Some(&layout), layout: Some(&layout),
vertex: wgpu::VertexState { module: &module, entry_point: Some("vs"), buffers: &[], compilation_options: Default::default() }, vertex: wgpu::VertexState { module: &module, entry_point: Some("vs"), buffers: &[], compilation_options: Default::default() },
@ -461,13 +463,14 @@ impl Compositor {
}), }),
multiview_mask: None, multiview_mask: None,
cache: None, cache: None,
}) }))
}; };
let mut pipelines: HashMap<_, _> = Kernel::DRAWN.iter().filter(|k| !k.is_map()).map(|&k| ((k, acc_format), pipeline(k, acc_format))).collect(); let mut pipelines: HashMap<_, _> =
Kernel::DRAWN.iter().filter(|k| !k.is_map()).filter_map(|&k| Some(((k, acc_format), pipeline(k, acc_format)?))).collect();
// Effect maps render to R32Float / R16Float, which some adapters (e.g. GL without float // Effect maps render to R32Float / R16Float, which some adapters (e.g. GL without float
// render targets) can't: then layer effects use the CPU compositor and the rest stays here. // render targets) can't: then layer effects use the CPU compositor and the rest stays here.
let maps = Kernel::DRAWN.iter().filter(|k| k.is_map()).flat_map(|&k| [(k, MAP32), (k, MAP16)]); let maps = Kernel::DRAWN.iter().filter(|k| k.is_map()).flat_map(|&k| [(k, MAP32), (k, MAP16)]);
let (maps, map_error) = first_error(device, || maps.map(|key| (key, pipeline(key.0, key.1))).collect::<Vec<_>>()); let (maps, map_error) = first_error(device, || maps.filter_map(|key| Some((key, pipeline(key.0, key.1)?))).collect::<Vec<_>>());
if let Some(e) = &map_error { if let Some(e) = &map_error {
log::info!("GPU compositor: no effect-map pipelines ({e}); layer effects use the CPU"); log::info!("GPU compositor: no effect-map pipelines ({e}); layer effects use the CPU");
} else { } else {
@ -738,7 +741,9 @@ impl Compositor {
}; };
match p.kernel { match p.kernel {
Kernel::CopyRect | Kernel::CopyFull => { Kernel::CopyRect | Kernel::CopyFull => {
let src = &self.pool[p.a.expect("copy source") as usize].0; // The planner always gives copies a source.
let Some(a) = p.a else { continue };
let src = &self.pool[a as usize].0;
let dst = &self.pool[p.dst as usize].0; let dst = &self.pool[p.dst as usize].0;
encoder.copy_texture_to_texture( encoder.copy_texture_to_texture(
wgpu::TexelCopyTextureInfo { wgpu::TexelCopyTextureInfo {
@ -837,7 +842,8 @@ impl Compositor {
} }
self.residents.insert(key, r); self.residents.insert(key, r);
} }
let r = self.residents.get_mut(&key).expect("inserted"); // Present: inserted above when it was missing or stale.
let r = self.residents.get_mut(&key)?;
r.last_used = self.frame; r.last_used = self.frame;
r.doc = doc; r.doc = doc;
for (c, t) in surface.tiles() { for (c, t) in surface.tiles() {
@ -956,8 +962,9 @@ impl Compositor {
// Moved by whole pixels: maps are computed relative to the region, so if the shape // Moved by whole pixels: maps are computed relative to the region, so if the shape
// moved unchanged with it, every map is still exact. // moved unchanged with it, every map is still exact.
let v = fx::shape(doc, layer, region); let v = fx::shape(doc, layer, region);
let e = self.fx.get_mut(&layer.id).expect("entry"); if let Some(e) = self.fx.get_mut(&layer.id)
if v == e.shape_cpu { && v == e.shape_cpu
{
e.region = region; e.region = region;
rebuild = false; rebuild = false;
} }
@ -982,7 +989,8 @@ impl Compositor {
); );
damage = region; damage = region;
} }
let e = self.fx.get_mut(&layer.id).expect("inserted"); // Present: kept or (re)inserted above.
let Some(e) = self.fx.get_mut(&layer.id) else { return };
e.last_used = frame; e.last_used = frame;
e.doc = doc.id; e.doc = doc.id;
e.tiles = [content_src.map(fx::snapshot), mask_src.map(fx::snapshot)]; e.tiles = [content_src.map(fx::snapshot), mask_src.map(fx::snapshot)];
@ -1041,7 +1049,7 @@ impl Compositor {
if prog.maps == 0 { if prog.maps == 0 {
continue; continue;
} }
let e = self.fx.get_mut(&layer.id).expect("entry"); let Some(e) = self.fx.get_mut(&layer.id) else { return };
let same = e.progs[i].key == prog.key && e.progs[i].maps.len() == prog.maps; let same = e.progs[i].key == prog.key && e.progs[i].maps.len() == prog.maps;
let d = if same { damage } else { region }; let d = if same { damage } else { region };
if d.is_empty() { if d.is_empty() {
@ -1085,17 +1093,19 @@ impl Compositor {
let v = self.temp(device, region, &temps); let v = self.temp(device, region, &temps);
temps.push(v); temps.push(v);
} }
let target_of = |i: usize| -> (wgpu::TextureView, wgpu::TextureFormat) { // `None` only if a program's map or temporary wasn't allocated, which `sync_fx` and
// `MapProgram::temps` rule out.
let target_of = |i: usize| -> Option<(wgpu::TextureView, wgpu::TextureFormat)> {
match prog.stages[i].out { match prog.stages[i].out {
Some(k) => (finals[k].clone().expect("final map allocated"), MAP16), Some(k) => Some((finals.get(k)?.clone()?, MAP16)),
None => (temps[assign[i].expect("temporary assigned")].clone(), MAP32), None => Some((temps.get((*assign.get(i)?)?)?.clone(), MAP32)),
} }
}; };
let view_of = |inp: Option<fx::In>| -> Option<wgpu::TextureView> { let view_of = |inp: Option<fx::In>| -> Option<wgpu::TextureView> {
match inp? { match inp? {
fx::In::Shape => Some(shape.clone()), fx::In::Shape => Some(shape.clone()),
fx::In::Field(k) => fields.get(&k).map(|f| f.1.clone()), fx::In::Field(k) => fields.get(&k).map(|f| f.1.clone()),
fx::In::Val(k) => Some(target_of(k).0), fx::In::Val(k) => target_of(k).map(|t| t.0),
} }
}; };
let mut draws = Vec::with_capacity(prog.stages.len()); let mut draws = Vec::with_capacity(prog.stages.len());
@ -1105,7 +1115,7 @@ impl Compositor {
continue; continue;
} }
stats.fx_pixels += w.width() as u64 * w.height() as u64; stats.fx_pixels += w.width() as u64 * w.height() as u64;
let (target, format) = target_of(i); let Some((target, format)) = target_of(i) else { continue };
let mut p = plan::Pass::new(s.kernel, 0); let mut p = plan::Pass::new(s.kernel, 0);
p.params[0] = s.p0; p.params[0] = s.p0;
p.params[1] = s.p1; p.params[1] = s.p1;
@ -1377,12 +1387,12 @@ pub fn render_to_vec_stats(
let w = rect.width() as usize; let w = rect.width() as usize;
let mut out = vec![[0.0f32; 4]; w * rect.height() as usize]; let mut out = vec![[0.0f32; 4]; w * rect.height() as usize];
for (r, b, row) in &staging { for (r, b, row) in &staging {
let data = b.slice(..).get_mapped_range().expect("mapped readback buffer"); let data = b.slice(..).get_mapped_range().map_err(|e| Unsupported(format!("GPU readback failed: {e:?}")))?;
for y in 0..r.height() as usize { for y in 0..r.height() as usize {
for x in 0..r.width() as usize { for x in 0..r.width() as usize {
let o = y * *row as usize + x * bpp as usize; let o = y * *row as usize + x * bpp as usize;
let px: [f32; 4] = if bpp == 16 { let px: [f32; 4] = if bpp == 16 {
std::array::from_fn(|i| f32::from_le_bytes(data[o + i * 4..o + i * 4 + 4].try_into().expect("4 bytes"))) std::array::from_fn(|i| f32::from_le_bytes([data[o + i * 4], data[o + i * 4 + 1], data[o + i * 4 + 2], data[o + i * 4 + 3]]))
} else { } else {
std::array::from_fn(|i| half::f16::from_le_bytes([data[o + i * 2], data[o + i * 2 + 1]]).to_f32()) std::array::from_fn(|i| half::f16::from_le_bytes([data[o + i * 2], data[o + i * 2 + 1]]).to_f32())
}; };

View File

@ -662,7 +662,8 @@ impl<'a> Planner<'a> {
} }
Ok(s) Ok(s)
} }
LayerContent::Adjustment(_) => unreachable!("adjustments handled by the caller"), // Adjustments are handled by the caller and never get here.
LayerContent::Adjustment(_) => Err(Unsupported("adjustment layer reached the content planner".into())),
LayerContent::Fill(f @ Fill::Pattern { name, scale, id, angle, link, phase }) if layer.fill_cache.as_ref().is_none_or(|c| c.fill != *f) => { LayerContent::Fill(f @ Fill::Pattern { name, scale, id, angle, link, phase }) if layer.fill_cache.as_ref().is_none_or(|c| c.fill != *f) => {
// compose::render_fill: the pattern tiled from the layer's frame (transparent // compose::render_fill: the pattern tiled from the layer's frame (transparent
// when missing), then the layer's masks. // when missing), then the layer's masks.