diff --git a/crates/horizon-client-render/src/crash.rs b/crates/horizon-client-render/src/crash.rs index 40bdab8d33..411e093a05 100644 --- a/crates/horizon-client-render/src/crash.rs +++ b/crates/horizon-client-render/src/crash.rs @@ -3,6 +3,7 @@ //! Written to `crash-reports/crash--client.txt` in the game folder when //! the main thread panics or `main` gets an error back. It leaves the machine only scrubbed, and //! only if the player allows crash reports (`telemetry`). +use std::cell::{Cell, RefCell}; use std::fmt::Write as _; use std::path::{Path, PathBuf}; use std::sync::Mutex; @@ -14,6 +15,51 @@ use chrono::{DateTime, Local}; static DETAILS: Mutex> = Mutex::new(Vec::new()); static REPORTED: AtomicBool = AtomicBool::new(false); +thread_local! { + /// How many `contain` calls this thread is inside. + static CONTAINED: Cell = const { Cell::new(0) }; + /// The message of the last panic `contain` caught on this thread, from the hook. + static CAUGHT: RefCell> = const { RefCell::new(None) }; +} + +/// Runs `f`, and returns a panic in it as an error with the panic's message instead of letting +/// it end the game. For work that is not the game's own and whose failure the caller recovers +/// from, such as handing a shader pack's programs to the GPU driver's compiler: the panic goes +/// to the log with its stack, and no crash report is made for it. +pub fn contain(f: impl FnOnce() -> T) -> Result { + CONTAINED.with(|c| c.set(c.get() + 1)); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)); + CONTAINED.with(|c| c.set(c.get() - 1)); + result.map_err(|payload| { + CAUGHT + .with(|c| c.borrow_mut().take()) + .unwrap_or_else(|| payload_text(&*payload)) + }) +} + +/// For the hook: a panic inside `contain` is logged with its stack and kept for `contain` to +/// return. False for any other panic. +fn contained_panic(info: &std::panic::PanicHookInfo) -> bool { + if CONTAINED.with(Cell::get) == 0 { + return false; + } + let message = panic_message(info); + let stack = stack_lines( + &std::backtrace::Backtrace::force_capture().to_string(), + true, + ); + log::error!( + "Recovered from an error: {message}\n{}", + stack + .lines() + .map(|l| format!("\tat {l}")) + .collect::>() + .join("\n") + ); + CAUGHT.with(|c| *c.borrow_mut() = Some(message)); + true +} + /// Sets one line of the report's details (replacing the last value under `key`). pub fn set_detail(key: &'static str, value: impl Into) { let mut details = DETAILS.lock().unwrap_or_else(|e| e.into_inner()); @@ -308,13 +354,16 @@ fn deliver(home: &Path, report: Report) -> Option { } } -fn panic_message(info: &std::panic::PanicHookInfo) -> String { - let payload = info.payload(); - let text = payload +fn payload_text(payload: &(dyn std::any::Any + Send)) -> String { + payload .downcast_ref::<&str>() .map(|s| s.to_string()) .or_else(|| payload.downcast_ref::().cloned()) - .unwrap_or_else(|| "Box".into()); + .unwrap_or_else(|| "Box".into()) +} + +fn panic_message(info: &std::panic::PanicHookInfo) -> String { + let text = payload_text(info.payload()); match info.location() { Some(at) => format!("panicked at {at}:\n{text}"), None => format!("panicked:\n{text}"), @@ -417,10 +466,13 @@ pub fn panic_report(info: &std::panic::PanicHookInfo, side: &'static str) -> Rep /// Reports panics: on the main thread a crash report (the panic then ends the game, exit code /// 101); on other threads the error goes to the log, as vanilla's uncaught exception handler -/// does. The default hook still prints the panic. +/// does. The default hook still prints the panic. A panic inside `contain` is only logged. pub fn install(home: PathBuf, side: &'static str) { let default = std::panic::take_hook(); std::panic::set_hook(Box::new(move |info| { + if contained_panic(info) { + return; + } default(info); let main = std::thread::current().name() == Some("main"); if main { @@ -609,6 +661,17 @@ mod tests { ); } + #[test] + fn contained_panics_come_back_as_errors() { + assert_eq!(contain(|| 7), Ok(7)); + let error = contain(|| -> u8 { panic!("a pack's program") }).unwrap_err(); + assert!(error.contains("a pack's program"), "{error}"); + // Nested, and the thread is no longer inside one after. + let outer = contain(|| contain(|| -> u8 { panic!("inner") })); + assert!(outer.unwrap().unwrap_err().contains("inner")); + assert_eq!(CONTAINED.with(Cell::get), 0); + } + #[test] fn the_debug_crash_is_reported_as_vanillas() { let error: Box = DebugCrash::new().into(); diff --git a/crates/horizon-client-render/src/gfx/pack_post.rs b/crates/horizon-client-render/src/gfx/pack_post.rs index b9320277ab..7eea498ef1 100644 --- a/crates/horizon-client-render/src/gfx/pack_post.rs +++ b/crates/horizon-client-render/src/gfx/pack_post.rs @@ -476,10 +476,12 @@ fn sampler( }) } -/// Runs `create` and returns its validation error, if any. +/// Runs `create` and returns its validation error, if any. A panic while it runs is an error +/// too: naga's backends turn a module they cannot write into a panic as the driver gets the +/// pipeline (#3525), which a pack's program must not turn into a crash. fn checked(device: &wgpu::Device, create: impl FnOnce() -> T) -> Result { let scope = device.push_error_scope(wgpu::ErrorFilter::Validation); - let value = create(); + let value = crate::crash::contain(create); match pollster::block_on(scope.pop()) { Some(e) => { // The details are in the error's description or its sources. @@ -494,7 +496,7 @@ fn checked(device: &wgpu::Device, create: impl FnOnce() -> T) -> Result Ok(value), + None => value.map_err(|panic| format!("the shader compiler failed: {panic}")), } } @@ -2629,6 +2631,38 @@ mod tests { assert_eq!(used, [1, 2, 3]); } + /// #3525: naga's SPIR-V writer panicked on Photon's gbuffers programs as the pipeline was + /// made. Such a panic skips the pass, as a validation error does, and the device goes on. + #[test] + fn a_shader_compiler_panic_skips_the_pass() { + let instance = wgpu::Instance::new(wgpu::InstanceDescriptor::new_without_display_handle()); + let Ok(adapter) = pollster::block_on(instance.request_adapter(&Default::default())) else { + return; + }; + let Ok((device, _queue)) = pollster::block_on(adapter.request_device(&Default::default())) + else { + return; + }; + let failed = checked(&device, || -> wgpu::Buffer { + panic!("internal error: entered unreachable code: Expression [89] is not cached!") + }); + let error = failed.expect_err("the panic comes back as an error"); + assert!(error.contains("Expression [89] is not cached!"), "{error}"); + let buffer = |usage| { + checked(&device, || { + device.create_buffer(&wgpu::BufferDescriptor { + label: None, + size: 16, + usage, + mapped_at_creation: false, + }) + }) + }; + // The device still makes things, and its errors still come back. + assert!(buffer(wgpu::BufferUsages::UNIFORM).is_ok()); + assert!(buffer(wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::MAP_WRITE).is_err()); + } + #[test] fn pack_formats_map_to_renderable_ones() { let f = |name| texture_format(Format::parse(name).unwrap(), wgpu::Features::empty()); diff --git a/crates/horizon-client/src/game/shader_packs.rs b/crates/horizon-client/src/game/shader_packs.rs index 4146283d31..26b4d1b49f 100644 --- a/crates/horizon-client/src/game/shader_packs.rs +++ b/crates/horizon-client/src/game/shader_packs.rs @@ -6,6 +6,7 @@ use horizon_client::camera::CameraFluid; use horizon_client::gfx::pack_uniforms::PackInputs; +use horizon_client::text::Text; use horizon_shaders::{BlockIds, Macros, Pack, Properties, dimension_folder, load_post}; use super::{Dim, Game}; @@ -64,79 +65,87 @@ impl Game<'_> { { let name = name.as_str(); let started = std::time::Instant::now(); - match Pack::open(&path) { - Ok(pack) => { - let macros = Macros::standard(); - let text = pack.read_text("shaders.properties").unwrap_or_default(); - let properties = Properties::parse(&text, ¯os); - for note in &properties.notes { - log::warn!( - "shader pack {name}: shaders.properties:{}: {}", - note.line, - note.message - ); - } - let folder = dimension_folder(&pack, dimension); - let inputs = renderer.pack_inputs(); - let mut post = load_post(&pack, &folder, &properties, ¯os, &inputs); - let biomes = self.res.biomes(); - let names: Vec<&str> = biomes.list.iter().map(|b| b.name.as_str()).collect(); - post.custom.set_biomes(&names); - let text = pack.read_text("block.properties").unwrap_or_default(); - let ids = BlockIds::parse(&Properties::parse(&text, ¯os)); - for note in &ids.notes { - log::warn!("shader pack {name}: block.properties: {note}"); - } - let text = pack.read_text("entity.properties"); - let entities = BlockIds::parse_kind( - &Properties::parse(text.as_deref().unwrap_or_default(), ¯os), - "entity", - ); - let text = pack.read_text("item.properties"); - let items = BlockIds::parse_kind( - &Properties::parse(text.as_deref().unwrap_or_default(), ¯os), - "item", - ); - self.pack_ids = Some(PackIds { - items: items.plain(), - missing_item: if text.is_some() { -1 } else { 0 }, - // OQ-21: on unless the pack turns it off. - old_hand_light: properties - .get("oldHandLight") - .is_none_or(|v| v.trim() != "false"), - entities: entities.plain(), - missing_entity: if text.is_some() { 65535 } else { 0 }, - missing_block: if ids.is_empty() { 0 } else { 65535 }, - blocks: ids.clone(), - block_states: RefCell::default(), - }); - let started_ids = std::time::Instant::now(); - let table = block_id_table(&self.pack, &ids); - log::info!( - "shader pack {name}: mc_Entity for {} of {} block sprites in {} ms", - table.chunks(2).filter(|p| p[0] != -1).count(), - table.len() / 2, - started_ids.elapsed().as_millis() - ); - if let Some((programs, notes)) = - renderer.set_shader_pack(dimension, Some(post), &table) - { - for note in notes { - log::warn!("shader pack {name}: {note}"); + // A pack is not the game's own code: one that makes the shader compiler (or our + // translation of it) panic is turned off instead of crashing every world load (#3525). + let loaded = horizon_client::crash::contain(|| { + match Pack::open(&path) { + Ok(pack) => { + let macros = Macros::standard(); + let text = pack.read_text("shaders.properties").unwrap_or_default(); + let properties = Properties::parse(&text, ¯os); + for note in &properties.notes { + log::warn!( + "shader pack {name}: shaders.properties:{}: {}", + note.line, + note.message + ); } - log::info!( - "shader pack {name}: folder `{folder}`, {} programs ({}) in {} ms", - programs.len(), - programs.join(" "), - started.elapsed().as_millis() + let folder = dimension_folder(&pack, dimension); + let inputs = renderer.pack_inputs(); + let mut post = load_post(&pack, &folder, &properties, ¯os, &inputs); + let biomes = self.res.biomes(); + let names: Vec<&str> = + biomes.list.iter().map(|b| b.name.as_str()).collect(); + post.custom.set_biomes(&names); + let text = pack.read_text("block.properties").unwrap_or_default(); + let ids = BlockIds::parse(&Properties::parse(&text, ¯os)); + for note in &ids.notes { + log::warn!("shader pack {name}: block.properties: {note}"); + } + let text = pack.read_text("entity.properties"); + let entities = BlockIds::parse_kind( + &Properties::parse(text.as_deref().unwrap_or_default(), ¯os), + "entity", ); + let text = pack.read_text("item.properties"); + let items = BlockIds::parse_kind( + &Properties::parse(text.as_deref().unwrap_or_default(), ¯os), + "item", + ); + self.pack_ids = Some(PackIds { + items: items.plain(), + missing_item: if text.is_some() { -1 } else { 0 }, + // OQ-21: on unless the pack turns it off. + old_hand_light: properties + .get("oldHandLight") + .is_none_or(|v| v.trim() != "false"), + entities: entities.plain(), + missing_entity: if text.is_some() { 65535 } else { 0 }, + missing_block: if ids.is_empty() { 0 } else { 65535 }, + blocks: ids.clone(), + block_states: RefCell::default(), + }); + let started_ids = std::time::Instant::now(); + let table = block_id_table(&self.pack, &ids); + log::info!( + "shader pack {name}: mc_Entity for {} of {} block sprites in {} ms", + table.chunks(2).filter(|p| p[0] != -1).count(), + table.len() / 2, + started_ids.elapsed().as_millis() + ); + if let Some((programs, notes)) = + renderer.set_shader_pack(dimension, Some(post), &table) + { + for note in notes { + log::warn!("shader pack {name}: {note}"); + } + log::info!( + "shader pack {name}: folder `{folder}`, {} programs ({}) in {} ms", + programs.len(), + programs.join(" "), + started.elapsed().as_millis() + ); + } + } + Err(e) => { + self.pack_ids = None; + log::warn!("shader pack {name}: {e}"); + renderer.set_shader_pack(dimension, None, &[]); } } - Err(e) => { - self.pack_ids = None; - log::warn!("shader pack {name}: {e}"); - renderer.set_shader_pack(dimension, None, &[]); - } + }); + if let Err(panic) = loaded { + self.shader_pack_failed(name, dimension, &panic); } } let lerp = |(old, new): (f32, f32)| old + (new - old) * partial; @@ -175,6 +184,25 @@ impl Game<'_> { } } + /// Turns off the pack `name` after loading it panicked: nothing runs in its place, it is no + /// longer the picked pack (so the next start does not load it again), and the chat says so. + fn shader_pack_failed(&mut self, name: &str, dimension: i32, error: &str) { + log::error!("shader pack {name}: turned off: {error}"); + self.pack_ids = None; + if let Some(renderer) = &mut self.renderer { + // Marks this dimension as loaded, without a pack, so it is not tried again. + renderer.set_shader_pack(dimension, None, &[]); + } + if self.settings.horizon.shader_pack == name { + self.settings.horizon.shader_pack.clear(); + self.settings.save(&Self::options_path()); + } + self.system_message(Text::colored( + format!("Shader pack {name} could not be loaded and was turned off"), + 0xFF5555, + )); + } + /// `heldItemId(2)` and `heldBlockLightValue(2)`: main hand, then off hand (5.2, 5.3). fn held_for_pack(&mut self) -> ([i32; 2], [i32; 2]) { if self.pack_ids.is_none() {