From 4d1b045855ff10fe5cd10775f7a3e9158085f191 Mon Sep 17 00:00:00 2001 From: Eero Lehtinen Date: Sun, 18 May 2025 09:24:37 +0300 Subject: [PATCH 1/9] Fix point light shadow glitches (#19265) # Objective Fixes #18945 ## Solution Entities that are not visible in any view (camera or light), get their render meshes removed. When they become visible somewhere again, the meshes get recreated and assigned possibly different ids. Point/spot light visible entities weren't cleared when the lights themseves went out of view, which caused them to try to queue these fake visible entities for rendering every frame. The shadow phase cache usually flushes non visible entites, but because of this bug it never flushed them and continued to queue meshes with outdated ids. The simple solution is to every frame clear all visible entities for all point/spot lights that may or may not be visible. The visible entities get repopulated directly afterwards. I also renamed the `global_point_lights` to `global_visible_clusterable` to make it clear that it includes only visible things. ## Testing - Tested with the code from the issue. --- crates/bevy_pbr/src/render/light.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index d71dccc71a..dfc7f679f3 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -220,7 +220,8 @@ pub fn extract_lights( mut commands: Commands, point_light_shadow_map: Extract>, directional_light_shadow_map: Extract>, - global_point_lights: Extract>, + global_visible_clusterable: Extract>, + cubemap_visible_entities: Extract>>, point_lights: Extract< Query<( Entity, @@ -276,6 +277,16 @@ pub fn extract_lights( if directional_light_shadow_map.is_changed() { commands.insert_resource(directional_light_shadow_map.clone()); } + + // Clear previous visible entities for all cubemapped lights as they might not be in the + // `global_visible_clusterable` list anymore. + commands.try_insert_batch( + cubemap_visible_entities + .iter() + .map(|render_entity| (render_entity, RenderCubemapVisibleEntities::default())) + .collect::>(), + ); + // This is the point light shadow map texel size for one face of the cube as a distance of 1.0 // world unit from the light. // point_light_texel_size = 2.0 * 1.0 * tan(PI / 4.0) / cube face width in texels @@ -286,7 +297,7 @@ pub fn extract_lights( let point_light_texel_size = 2.0 / point_light_shadow_map.size as f32; let mut point_lights_values = Vec::with_capacity(*previous_point_lights_len); - for entity in global_point_lights.iter().copied() { + for entity in global_visible_clusterable.iter().copied() { let Ok(( main_entity, render_entity, @@ -350,7 +361,7 @@ pub fn extract_lights( commands.try_insert_batch(point_lights_values); let mut spot_lights_values = Vec::with_capacity(*previous_spot_lights_len); - for entity in global_point_lights.iter().copied() { + for entity in global_visible_clusterable.iter().copied() { if let Ok(( main_entity, render_entity, From 2db103708ad10d12a1c7c4cc6e6fb6a0ff1f8eec Mon Sep 17 00:00:00 2001 From: Eero Lehtinen Date: Sun, 18 May 2025 09:28:30 +0300 Subject: [PATCH 2/9] Make sure prepass notices changes in alpha mode (#19170) # Objective Fixes #19150 ## Solution Normally the `validate_cached_entity` in https://github.com/bevyengine/bevy/blob/86cc02dca229662bdfb371a0e5a1716560ea7baa/crates/bevy_pbr/src/prepass/mod.rs#L1109-L1126 marks unchanged entites as clean, which makes them remain in the phase. If a material is changed to an `alpha_mode` that isn't supposed to be added to the prepass pipeline, the specialization system just `continue`s and doesn't indicate to the cache that the entity is not clean anymore. I made these invalid entities get removed from the pipeline cache so that they are correctly not marked clean and then removed from the phase. ## Testing Tested with the example code from the issue. --- crates/bevy_pbr/src/prepass/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index cc8dcea6a0..03a797eba1 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -983,12 +983,18 @@ pub fn specialize_prepass_material_meshes( AlphaMode::Blend | AlphaMode::Premultiplied | AlphaMode::Add - | AlphaMode::Multiply => continue, + | AlphaMode::Multiply => { + // In case this material was previously in a valid alpha_mode, remove it to + // stop the queue system from assuming its retained cache to be valid. + view_specialized_material_pipeline_cache.remove(visible_entity); + continue; + } } if material.properties.reads_view_transmission_texture { // No-op: Materials reading from `ViewTransmissionTexture` are not rendered in the `Opaque3d` // phase, and are therefore also excluded from the prepass much like alpha-blended materials. + view_specialized_material_pipeline_cache.remove(visible_entity); continue; } From 45ba5b9f0347710d0b84979631fae6f690434d62 Mon Sep 17 00:00:00 2001 From: atlv Date: Sun, 18 May 2025 02:30:38 -0400 Subject: [PATCH 3/9] refactor(render): cleanup add_import_to_composer (#19269) # Objective - Reduce nesting ## Solution - Refactor ## Testing - `bevy run --example=3d_scene web --open` --- .../src/render_resource/pipeline_cache.rs | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/crates/bevy_render/src/render_resource/pipeline_cache.rs b/crates/bevy_render/src/render_resource/pipeline_cache.rs index f2ab16a23e..7c54b0f406 100644 --- a/crates/bevy_render/src/render_resource/pipeline_cache.rs +++ b/crates/bevy_render/src/render_resource/pipeline_cache.rs @@ -203,28 +203,25 @@ impl ShaderCache { shaders: &HashMap, Shader>, import: &ShaderImport, ) -> Result<(), PipelineCacheError> { - if !composer.contains_module(&import.module_name()) { - if let Some(shader_handle) = import_path_shaders.get(import) { - if let Some(shader) = shaders.get(shader_handle) { - for import in &shader.imports { - Self::add_import_to_composer( - composer, - import_path_shaders, - shaders, - import, - )?; - } - - composer.add_composable_module(shader.into())?; - } else { - Err(PipelineCacheError::ShaderImportNotYetAvailable)?; - } - } else { - Err(PipelineCacheError::ShaderImportNotYetAvailable)?; - } - // if we fail to add a module the composer will tell us what is missing + // Early out if we've already imported this module + if composer.contains_module(&import.module_name()) { + return Ok(()); } + // Check if the import is available (this handles the recursive import case) + let shader = import_path_shaders + .get(import) + .and_then(|handle| shaders.get(handle)) + .ok_or(PipelineCacheError::ShaderImportNotYetAvailable)?; + + // Recurse down to ensure all import dependencies are met + for import in &shader.imports { + Self::add_import_to_composer(composer, import_path_shaders, shaders, import)?; + } + + composer.add_composable_module(shader.into())?; + // if we fail to add a module the composer will tell us what is missing + Ok(()) } From e7e9973c80c259ccf27a02090097887d9060d8fc Mon Sep 17 00:00:00 2001 From: SpecificProtagonist Date: Mon, 19 May 2025 03:35:07 +0200 Subject: [PATCH 4/9] Per world error handler (#18810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective [see original comment](https://github.com/bevyengine/bevy/pull/18801#issuecomment-2796981745) > Alternately, could we store it on the World instead of a global? I think we have a World nearby whenever we call default_error_handler(). That would avoid the need for atomics or locks, since we could do ordinary reads and writes to the World. Global error handlers don't actually need to be global – per world is enough. This allows using different handlers for different worlds and also removes the restrictions on changing the handler only once. ## Solution Each `World` can now store its own error handler in a resource. For convenience, you can also set the default error handler for an `App`, which applies it to the worlds of all `SubApp`s. The old behavior of only being able to set the error handler once is kept for apps. We also don't need the `configurable_error_handler` feature anymore now. ## Testing New/adjusted tests for failing schedule systems & observers. --- ## Showcase ```rust App::new() .set_error_handler(info) … ``` --- Cargo.toml | 6 +- crates/bevy_app/src/app.rs | 53 +++++++++++++- crates/bevy_ecs/Cargo.toml | 7 -- crates/bevy_ecs/src/error/command_handling.rs | 41 +++++++---- crates/bevy_ecs/src/error/handler.rs | 71 ++++++------------- crates/bevy_ecs/src/error/mod.rs | 16 ++--- crates/bevy_ecs/src/observer/runner.rs | 43 +++++++---- .../src/schedule/executor/multi_threaded.rs | 24 ++++--- .../bevy_ecs/src/schedule/executor/simple.rs | 26 ++++--- .../src/schedule/executor/single_threaded.rs | 26 ++++--- crates/bevy_ecs/src/schedule/schedule.rs | 32 ++++++++- crates/bevy_ecs/src/system/commands/mod.rs | 16 ++--- crates/bevy_ecs/src/world/mod.rs | 11 +++ .../bevy_ecs/src/world/unsafe_world_cell.rs | 13 ++++ crates/bevy_internal/Cargo.toml | 3 - docs/cargo_features.md | 1 - examples/ecs/error_handling.rs | 22 ++---- examples/ecs/fallible_params.rs | 16 ++--- .../per-world-error-handler.md | 10 +++ 19 files changed, 272 insertions(+), 165 deletions(-) create mode 100644 release-content/migration-guides/per-world-error-handler.md diff --git a/Cargo.toml b/Cargo.toml index a3d3a2ab63..7cf1334e81 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -291,9 +291,6 @@ bevy_log = ["bevy_internal/bevy_log"] # Enable input focus subsystem bevy_input_focus = ["bevy_internal/bevy_input_focus"] -# Use the configurable global error handler as the default error handler. -configurable_error_handler = ["bevy_internal/configurable_error_handler"] - # Enable passthrough loading for SPIR-V shaders (Only supported on Vulkan, shader capabilities and extensions must agree with the platform implementation) spirv_shader_passthrough = ["bevy_internal/spirv_shader_passthrough"] @@ -2215,7 +2212,6 @@ wasm = false name = "fallible_params" path = "examples/ecs/fallible_params.rs" doc-scrape-examples = true -required-features = ["configurable_error_handler"] [package.metadata.example.fallible_params] name = "Fallible System Parameters" @@ -2227,7 +2223,7 @@ wasm = false name = "error_handling" path = "examples/ecs/error_handling.rs" doc-scrape-examples = true -required-features = ["bevy_mesh_picking_backend", "configurable_error_handler"] +required-features = ["bevy_mesh_picking_backend"] [package.metadata.example.error_handling] name = "Error handling" diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 60431ca479..81d7baeb4b 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -10,6 +10,7 @@ use alloc::{ pub use bevy_derive::AppLabel; use bevy_ecs::{ component::RequiredComponentsError, + error::{DefaultErrorHandler, ErrorHandler}, event::{event_update_system, EventCursor}, intern::Interned, prelude::*, @@ -85,6 +86,7 @@ pub struct App { /// [`WinitPlugin`]: https://docs.rs/bevy/latest/bevy/winit/struct.WinitPlugin.html /// [`ScheduleRunnerPlugin`]: https://docs.rs/bevy/latest/bevy/app/struct.ScheduleRunnerPlugin.html pub(crate) runner: RunnerFn, + default_error_handler: Option, } impl Debug for App { @@ -143,6 +145,7 @@ impl App { sub_apps: HashMap::default(), }, runner: Box::new(run_once), + default_error_handler: None, } } @@ -1115,7 +1118,12 @@ impl App { } /// Inserts a [`SubApp`] with the given label. - pub fn insert_sub_app(&mut self, label: impl AppLabel, sub_app: SubApp) { + pub fn insert_sub_app(&mut self, label: impl AppLabel, mut sub_app: SubApp) { + if let Some(handler) = self.default_error_handler { + sub_app + .world_mut() + .get_resource_or_insert_with(|| DefaultErrorHandler(handler)); + } self.sub_apps.sub_apps.insert(label.intern(), sub_app); } @@ -1334,6 +1342,49 @@ impl App { self.world_mut().add_observer(observer); self } + + /// Gets the error handler to set for new supapps. + /// + /// Note that the error handler of existing subapps may differ. + pub fn get_error_handler(&self) -> Option { + self.default_error_handler + } + + /// Set the [default error handler] for the all subapps (including the main one and future ones) + /// that do not have one. + /// + /// May only be called once and should be set by the application, not by libraries. + /// + /// The handler will be called when an error is produced and not otherwise handled. + /// + /// # Panics + /// Panics if called multiple times. + /// + /// # Example + /// ``` + /// # use bevy_app::*; + /// # use bevy_ecs::error::warn; + /// # fn MyPlugins(_: &mut App) {} + /// App::new() + /// .set_error_handler(warn) + /// .add_plugins(MyPlugins) + /// .run(); + /// ``` + /// + /// [default error handler]: bevy_ecs::error::DefaultErrorHandler + pub fn set_error_handler(&mut self, handler: ErrorHandler) -> &mut Self { + assert!( + self.default_error_handler.is_none(), + "`set_error_handler` called multiple times on same `App`" + ); + self.default_error_handler = Some(handler); + for sub_app in self.sub_apps.iter_mut() { + sub_app + .world_mut() + .get_resource_or_insert_with(|| DefaultErrorHandler(handler)); + } + self + } } type RunnerFn = Box AppExit>; diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 97cdcee082..28987f1413 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -33,13 +33,6 @@ bevy_reflect = ["dep:bevy_reflect"] ## Extends reflection support to functions. reflect_functions = ["bevy_reflect", "bevy_reflect/functions"] -## Use the configurable global error handler as the default error handler. -## -## This is typically used to turn panics from the ECS into loggable errors. -## This may be useful for production builds, -## but can result in a measurable performance impact, especially for commands. -configurable_error_handler = [] - ## Enables automatic backtrace capturing in BevyError backtrace = ["std"] diff --git a/crates/bevy_ecs/src/error/command_handling.rs b/crates/bevy_ecs/src/error/command_handling.rs index d85ad4a87e..bf2741d376 100644 --- a/crates/bevy_ecs/src/error/command_handling.rs +++ b/crates/bevy_ecs/src/error/command_handling.rs @@ -7,22 +7,17 @@ use crate::{ world::{error::EntityMutableFetchError, World}, }; -use super::{default_error_handler, BevyError, ErrorContext}; +use super::{BevyError, ErrorContext, ErrorHandler}; -/// Takes a [`Command`] that returns a Result and uses a given error handler function to convert it into +/// Takes a [`Command`] that potentially returns a Result and uses a given error handler function to convert it into /// a [`Command`] that internally handles an error if it occurs and returns `()`. -pub trait HandleError { +pub trait HandleError: Send + 'static { /// Takes a [`Command`] that returns a Result and uses a given error handler function to convert it into /// a [`Command`] that internally handles an error if it occurs and returns `()`. - fn handle_error_with(self, error_handler: fn(BevyError, ErrorContext)) -> impl Command; + fn handle_error_with(self, error_handler: ErrorHandler) -> impl Command; /// Takes a [`Command`] that returns a Result and uses the default error handler function to convert it into /// a [`Command`] that internally handles an error if it occurs and returns `()`. - fn handle_error(self) -> impl Command - where - Self: Sized, - { - self.handle_error_with(default_error_handler()) - } + fn handle_error(self) -> impl Command; } impl HandleError> for C @@ -30,7 +25,7 @@ where C: Command>, E: Into, { - fn handle_error_with(self, error_handler: fn(BevyError, ErrorContext)) -> impl Command { + fn handle_error_with(self, error_handler: ErrorHandler) -> impl Command { move |world: &mut World| match self.apply(world) { Ok(_) => {} Err(err) => (error_handler)( @@ -41,6 +36,18 @@ where ), } } + + fn handle_error(self) -> impl Command { + move |world: &mut World| match self.apply(world) { + Ok(_) => {} + Err(err) => world.default_error_handler()( + err.into(), + ErrorContext::Command { + name: type_name::().into(), + }, + ), + } + } } impl HandleError for C @@ -52,6 +59,13 @@ where self.apply(world); } } + + #[inline] + fn handle_error(self) -> impl Command { + move |world: &mut World| { + self.apply(world); + } + } } impl HandleError for C @@ -63,10 +77,7 @@ where self } #[inline] - fn handle_error(self) -> impl Command - where - Self: Sized, - { + fn handle_error(self) -> impl Command { self } } diff --git a/crates/bevy_ecs/src/error/handler.rs b/crates/bevy_ecs/src/error/handler.rs index 688b599473..d53e1b3e05 100644 --- a/crates/bevy_ecs/src/error/handler.rs +++ b/crates/bevy_ecs/src/error/handler.rs @@ -1,9 +1,8 @@ -#[cfg(feature = "configurable_error_handler")] -use bevy_platform::sync::OnceLock; use core::fmt::Display; -use crate::{component::Tick, error::BevyError}; +use crate::{component::Tick, error::BevyError, prelude::Resource}; use alloc::borrow::Cow; +use derive_more::derive::{Deref, DerefMut}; /// Context for a [`BevyError`] to aid in debugging. #[derive(Debug, PartialEq, Eq, Clone)] @@ -77,53 +76,6 @@ impl ErrorContext { } } -/// A global error handler. This can be set at startup, as long as it is set before -/// any uses. This should generally be configured _before_ initializing the app. -/// -/// This should be set inside of your `main` function, before initializing the Bevy app. -/// The value of this error handler can be accessed using the [`default_error_handler`] function, -/// which calls [`OnceLock::get_or_init`] to get the value. -/// -/// **Note:** this is only available when the `configurable_error_handler` feature of `bevy_ecs` (or `bevy`) is enabled! -/// -/// # Example -/// -/// ``` -/// # use bevy_ecs::error::{GLOBAL_ERROR_HANDLER, warn}; -/// GLOBAL_ERROR_HANDLER.set(warn).expect("The error handler can only be set once, globally."); -/// // initialize Bevy App here -/// ``` -/// -/// To use this error handler in your app for custom error handling logic: -/// -/// ```rust -/// use bevy_ecs::error::{default_error_handler, GLOBAL_ERROR_HANDLER, BevyError, ErrorContext, panic}; -/// -/// fn handle_errors(error: BevyError, ctx: ErrorContext) { -/// let error_handler = default_error_handler(); -/// error_handler(error, ctx); -/// } -/// ``` -/// -/// # Warning -/// -/// As this can *never* be overwritten, library code should never set this value. -#[cfg(feature = "configurable_error_handler")] -pub static GLOBAL_ERROR_HANDLER: OnceLock = OnceLock::new(); - -/// The default error handler. This defaults to [`panic()`], -/// but if set, the [`GLOBAL_ERROR_HANDLER`] will be used instead, enabling error handler customization. -/// The `configurable_error_handler` feature must be enabled to change this from the panicking default behavior, -/// as there may be runtime overhead. -#[inline] -pub fn default_error_handler() -> fn(BevyError, ErrorContext) { - #[cfg(not(feature = "configurable_error_handler"))] - return panic; - - #[cfg(feature = "configurable_error_handler")] - return *GLOBAL_ERROR_HANDLER.get_or_init(|| panic); -} - macro_rules! inner { ($call:path, $e:ident, $c:ident) => { $call!( @@ -135,6 +87,25 @@ macro_rules! inner { }; } +/// Defines how Bevy reacts to errors. +pub type ErrorHandler = fn(BevyError, ErrorContext); + +/// Error handler to call when an error is not handled otherwise. +/// Defaults to [`panic()`]. +/// +/// When updated while a [`Schedule`] is running, it doesn't take effect for +/// that schedule until it's completed. +/// +/// [`Schedule`]: crate::schedule::Schedule +#[derive(Resource, Deref, DerefMut, Copy, Clone)] +pub struct DefaultErrorHandler(pub ErrorHandler); + +impl Default for DefaultErrorHandler { + fn default() -> Self { + Self(panic) + } +} + /// Error handler that panics with the system error. #[track_caller] #[inline] diff --git a/crates/bevy_ecs/src/error/mod.rs b/crates/bevy_ecs/src/error/mod.rs index 950deee3ec..231bdda940 100644 --- a/crates/bevy_ecs/src/error/mod.rs +++ b/crates/bevy_ecs/src/error/mod.rs @@ -7,8 +7,9 @@ //! All [`BevyError`]s returned by a system, observer or command are handled by an "error handler". By default, the //! [`panic`] error handler function is used, resulting in a panic with the error message attached. //! -//! You can change the default behavior by registering a custom error handler. -//! Modify the [`GLOBAL_ERROR_HANDLER`] value to set a custom error handler function for your entire app. +//! You can change the default behavior by registering a custom error handler: +//! Use [`DefaultErrorHandler`] to set a custom error handler function for a world, +//! or `App::set_error_handler` for a whole app. //! In practice, this is generally feature-flagged: panicking or loudly logging errors in development, //! and quietly logging or ignoring them in production to avoid crashing the app. //! @@ -33,10 +34,8 @@ //! The [`ErrorContext`] allows you to access additional details relevant to providing //! context surrounding the error – such as the system's [`name`] – in your error messages. //! -//! Remember to turn on the `configurable_error_handler` feature to set a global error handler! -//! //! ```rust, ignore -//! use bevy_ecs::error::{GLOBAL_ERROR_HANDLER, BevyError, ErrorContext}; +//! use bevy_ecs::error::{BevyError, ErrorContext, DefaultErrorHandler}; //! use log::trace; //! //! fn my_error_handler(error: BevyError, ctx: ErrorContext) { @@ -48,10 +47,9 @@ //! } //! //! fn main() { -//! // This requires the "configurable_error_handler" feature to be enabled to be in scope. -//! GLOBAL_ERROR_HANDLER.set(my_error_handler).expect("The error handler can only be set once."); -//! -//! // Initialize your Bevy App here +//! let mut world = World::new(); +//! world.insert_resource(DefaultErrorHandler(my_error_handler)); +//! // Use your world here //! } //! ``` //! diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index ac1caa5ad0..be7bc4ede2 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -3,7 +3,7 @@ use core::any::Any; use crate::{ component::{ComponentHook, ComponentId, HookContext, Mutable, StorageType}, - error::{default_error_handler, ErrorContext}, + error::{ErrorContext, ErrorHandler}, observer::{ObserverDescriptor, ObserverTrigger}, prelude::*, query::DebugCheckedUnwrap, @@ -190,7 +190,7 @@ pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate: /// [`SystemParam`]: crate::system::SystemParam pub struct Observer { hook_on_add: ComponentHook, - error_handler: Option, + error_handler: Option, system: Box, pub(crate) descriptor: ObserverDescriptor, pub(crate) last_trigger_id: u32, @@ -232,6 +232,7 @@ impl Observer { system: Box::new(|| {}), descriptor: Default::default(), hook_on_add: |mut world, hook_context| { + let default_error_handler = world.default_error_handler(); world.commands().queue(move |world: &mut World| { let entity = hook_context.entity; if let Some(mut observe) = world.get_mut::(entity) { @@ -239,7 +240,7 @@ impl Observer { return; } if observe.error_handler.is_none() { - observe.error_handler = Some(default_error_handler()); + observe.error_handler = Some(default_error_handler); } world.register_observer(entity); } @@ -348,8 +349,6 @@ fn observer_system_runner>( return; } state.last_trigger_id = last_trigger; - // SAFETY: Observer was triggered so must have an `Observer` component. - let error_handler = unsafe { state.error_handler.debug_checked_unwrap() }; let trigger: Trigger = Trigger::new( // SAFETY: Caller ensures `ptr` is castable to `&mut T` @@ -377,7 +376,10 @@ fn observer_system_runner>( match (*system).validate_param_unsafe(world) { Ok(()) => { if let Err(err) = (*system).run_unsafe(trigger, world) { - error_handler( + let handler = state + .error_handler + .unwrap_or_else(|| world.default_error_handler()); + handler( err, ErrorContext::Observer { name: (*system).name(), @@ -389,7 +391,10 @@ fn observer_system_runner>( } Err(e) => { if !e.skipped { - error_handler( + let handler = state + .error_handler + .unwrap_or_else(|| world.default_error_handler()); + handler( e.into(), ErrorContext::Observer { name: (*system).name(), @@ -424,9 +429,6 @@ fn hook_on_add>( observe.descriptor.events.push(event_id); observe.descriptor.components.extend(components); - if observe.error_handler.is_none() { - observe.error_handler = Some(default_error_handler()); - } let system: *mut dyn ObserverSystem = observe.system.downcast_mut::().unwrap(); // SAFETY: World reference is exclusive and initialize does not touch system, so references do not alias unsafe { @@ -439,7 +441,11 @@ fn hook_on_add>( #[cfg(test)] mod tests { use super::*; - use crate::{event::Event, observer::Trigger}; + use crate::{ + error::{ignore, DefaultErrorHandler}, + event::Event, + observer::Trigger, + }; #[derive(Event)] struct TriggerEvent; @@ -467,11 +473,20 @@ mod tests { Err("I failed!".into()) } + // Using observer error handler let mut world = World::default(); world.init_resource::(); - let observer = Observer::new(system).with_error_handler(crate::error::ignore); - world.spawn(observer); - Schedule::default().run(&mut world); + world.spawn(Observer::new(system).with_error_handler(ignore)); + world.trigger(TriggerEvent); + assert!(world.resource::().0); + + // Using world error handler + let mut world = World::default(); + world.init_resource::(); + world.spawn(Observer::new(system)); + // Test that the correct handler is used when the observer was added + // before the default handler + world.insert_resource(DefaultErrorHandler(ignore)); world.trigger(TriggerEvent); assert!(world.resource::().0); } diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index b0757cc031..763504eaec 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -13,7 +13,7 @@ use std::sync::{Mutex, MutexGuard}; use tracing::{info_span, Span}; use crate::{ - error::{default_error_handler, BevyError, ErrorContext, Result}, + error::{ErrorContext, ErrorHandler, Result}, prelude::Resource, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, system::ScheduleSystem, @@ -134,7 +134,7 @@ pub struct ExecutorState { struct Context<'scope, 'env, 'sys> { environment: &'env Environment<'env, 'sys>, scope: &'scope Scope<'scope, 'env, ()>, - error_handler: fn(BevyError, ErrorContext), + error_handler: ErrorHandler, } impl Default for MultiThreadedExecutor { @@ -240,7 +240,7 @@ impl SystemExecutor for MultiThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(BevyError, ErrorContext), + error_handler: ErrorHandler, ) { let state = self.state.get_mut().unwrap(); // reset counts @@ -484,6 +484,7 @@ impl ExecutorState { system, conditions, context.environment.world_cell, + context.error_handler, ) } { self.skip_system_and_signal_dependents(system_index); @@ -584,9 +585,9 @@ impl ExecutorState { system: &mut ScheduleSystem, conditions: &mut Conditions, world: UnsafeWorldCell, + error_handler: ErrorHandler, ) -> bool { let mut should_run = !self.skipped_systems.contains(system_index); - let error_handler = default_error_handler(); for set_idx in conditions.sets_with_conditions_of_systems[system_index].ones() { if self.evaluated_sets.contains(set_idx) { @@ -599,7 +600,11 @@ impl ExecutorState { // required by the conditions. // - `update_archetype_component_access` has been called for each run condition. let set_conditions_met = unsafe { - evaluate_and_fold_conditions(&mut conditions.set_conditions[set_idx], world) + evaluate_and_fold_conditions( + &mut conditions.set_conditions[set_idx], + world, + error_handler, + ) }; if !set_conditions_met { @@ -617,7 +622,11 @@ impl ExecutorState { // required by the conditions. // - `update_archetype_component_access` has been called for each run condition. let system_conditions_met = unsafe { - evaluate_and_fold_conditions(&mut conditions.system_conditions[system_index], world) + evaluate_and_fold_conditions( + &mut conditions.system_conditions[system_index], + world, + error_handler, + ) }; if !system_conditions_met { @@ -822,9 +831,8 @@ fn apply_deferred( unsafe fn evaluate_and_fold_conditions( conditions: &mut [BoxedCondition], world: UnsafeWorldCell, + error_handler: ErrorHandler, ) -> bool { - let error_handler = default_error_handler(); - #[expect( clippy::unnecessary_fold, reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 701a8d8f06..584c5a1073 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -10,7 +10,7 @@ use tracing::info_span; use std::eprintln; use crate::{ - error::{default_error_handler, BevyError, ErrorContext}, + error::{ErrorContext, ErrorHandler}, schedule::{ executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule, }, @@ -50,7 +50,7 @@ impl SystemExecutor for SimpleExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(BevyError, ErrorContext), + error_handler: ErrorHandler, ) { // If stepping is enabled, make sure we skip those systems that should // not be run. @@ -73,8 +73,11 @@ impl SystemExecutor for SimpleExecutor { } // evaluate system set's conditions - let set_conditions_met = - evaluate_and_fold_conditions(&mut schedule.set_conditions[set_idx], world); + let set_conditions_met = evaluate_and_fold_conditions( + &mut schedule.set_conditions[set_idx], + world, + error_handler, + ); if !set_conditions_met { self.completed_systems @@ -86,8 +89,11 @@ impl SystemExecutor for SimpleExecutor { } // evaluate system's conditions - let system_conditions_met = - evaluate_and_fold_conditions(&mut schedule.system_conditions[system_index], world); + let system_conditions_met = evaluate_and_fold_conditions( + &mut schedule.system_conditions[system_index], + world, + error_handler, + ); should_run &= system_conditions_met; @@ -175,9 +181,11 @@ impl SimpleExecutor { since = "0.17.0", note = "Use SingleThreadedExecutor instead. See https://github.com/bevyengine/bevy/issues/18453 for motivation." )] -fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut World) -> bool { - let error_handler = default_error_handler(); - +fn evaluate_and_fold_conditions( + conditions: &mut [BoxedCondition], + world: &mut World, + error_handler: ErrorHandler, +) -> bool { #[expect( clippy::unnecessary_fold, reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 82e9e354a8..0076103637 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -8,7 +8,7 @@ use tracing::info_span; use std::eprintln; use crate::{ - error::{default_error_handler, BevyError, ErrorContext}, + error::{ErrorContext, ErrorHandler}, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, world::World, }; @@ -50,7 +50,7 @@ impl SystemExecutor for SingleThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(BevyError, ErrorContext), + error_handler: ErrorHandler, ) { // If stepping is enabled, make sure we skip those systems that should // not be run. @@ -73,8 +73,11 @@ impl SystemExecutor for SingleThreadedExecutor { } // evaluate system set's conditions - let set_conditions_met = - evaluate_and_fold_conditions(&mut schedule.set_conditions[set_idx], world); + let set_conditions_met = evaluate_and_fold_conditions( + &mut schedule.set_conditions[set_idx], + world, + error_handler, + ); if !set_conditions_met { self.completed_systems @@ -86,8 +89,11 @@ impl SystemExecutor for SingleThreadedExecutor { } // evaluate system's conditions - let system_conditions_met = - evaluate_and_fold_conditions(&mut schedule.system_conditions[system_index], world); + let system_conditions_met = evaluate_and_fold_conditions( + &mut schedule.system_conditions[system_index], + world, + error_handler, + ); should_run &= system_conditions_met; @@ -193,9 +199,11 @@ impl SingleThreadedExecutor { } } -fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut World) -> bool { - let error_handler: fn(BevyError, ErrorContext) = default_error_handler(); - +fn evaluate_and_fold_conditions( + conditions: &mut [BoxedCondition], + world: &mut World, + error_handler: ErrorHandler, +) -> bool { #[expect( clippy::unnecessary_fold, reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index a754f1e1d4..b15860406b 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -27,7 +27,6 @@ use tracing::info_span; use crate::{ component::{ComponentId, Components, Tick}, - error::default_error_handler, prelude::Component, resource::Resource, schedule::*, @@ -442,7 +441,7 @@ impl Schedule { self.initialize(world) .unwrap_or_else(|e| panic!("Error when initializing schedule {:?}: {e}", self.label)); - let error_handler = default_error_handler(); + let error_handler = world.default_error_handler(); #[cfg(not(feature = "bevy_debug_stepping"))] self.executor @@ -2061,6 +2060,7 @@ mod tests { use bevy_ecs_macros::ScheduleLabel; use crate::{ + error::{ignore, panic, DefaultErrorHandler, Result}, prelude::{ApplyDeferred, Res, Resource}, schedule::{ tests::ResMut, IntoScheduleConfigs, Schedule, ScheduleBuildSettings, SystemSet, @@ -2810,4 +2810,32 @@ mod tests { .expect("CheckSystemRan Resource Should Exist"); assert_eq!(value.0, 2); } + + #[test] + fn test_default_error_handler() { + #[derive(Resource, Default)] + struct Ran(bool); + + fn system(mut ran: ResMut) -> Result { + ran.0 = true; + Err("I failed!".into()) + } + + // Test that the default error handler is used + let mut world = World::default(); + world.init_resource::(); + world.insert_resource(DefaultErrorHandler(ignore)); + let mut schedule = Schedule::default(); + schedule.add_systems(system).run(&mut world); + assert!(world.resource::().0); + + // Test that the handler doesn't change within the schedule + schedule.add_systems( + (|world: &mut World| { + world.insert_resource(DefaultErrorHandler(panic)); + }) + .before(system), + ); + schedule.run(&mut world); + } } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 621a9de77e..8b10b64b28 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -88,8 +88,8 @@ use crate::{ /// A [`Command`] can return a [`Result`](crate::error::Result), /// which will be passed to an [error handler](crate::error) if the `Result` is an error. /// -/// The [default error handler](crate::error::default_error_handler) panics. -/// It can be configured by setting the `GLOBAL_ERROR_HANDLER`. +/// The default error handler panics. It can be configured via +/// the [`DefaultErrorHandler`](crate::error::DefaultErrorHandler) resource. /// /// Alternatively, you can customize the error handler for a specific command /// by calling [`Commands::queue_handled`]. @@ -508,7 +508,7 @@ impl<'w, 's> Commands<'w, 's> { /// Pushes a generic [`Command`] to the command queue. /// /// If the [`Command`] returns a [`Result`], - /// it will be handled using the [default error handler](crate::error::default_error_handler). + /// it will be handled using the [default error handler](crate::error::DefaultErrorHandler). /// /// To use a custom error handler, see [`Commands::queue_handled`]. /// @@ -643,7 +643,7 @@ impl<'w, 's> Commands<'w, 's> { /// This command will fail if any of the given entities do not exist. /// /// It will internally return a [`TryInsertBatchError`](crate::world::error::TryInsertBatchError), - /// which will be handled by the [default error handler](crate::error::default_error_handler). + /// which will be handled by the [default error handler](crate::error::DefaultErrorHandler). #[track_caller] pub fn insert_batch(&mut self, batch: I) where @@ -674,7 +674,7 @@ impl<'w, 's> Commands<'w, 's> { /// This command will fail if any of the given entities do not exist. /// /// It will internally return a [`TryInsertBatchError`](crate::world::error::TryInsertBatchError), - /// which will be handled by the [default error handler](crate::error::default_error_handler). + /// which will be handled by the [default error handler](crate::error::DefaultErrorHandler). #[track_caller] pub fn insert_batch_if_new(&mut self, batch: I) where @@ -1175,8 +1175,8 @@ impl<'w, 's> Commands<'w, 's> { /// An [`EntityCommand`] can return a [`Result`](crate::error::Result), /// which will be passed to an [error handler](crate::error) if the `Result` is an error. /// -/// The [default error handler](crate::error::default_error_handler) panics. -/// It can be configured by setting the `GLOBAL_ERROR_HANDLER`. +/// The default error handler panics. It can be configured via +/// the [`DefaultErrorHandler`](crate::error::DefaultErrorHandler) resource. /// /// Alternatively, you can customize the error handler for a specific command /// by calling [`EntityCommands::queue_handled`]. @@ -1768,7 +1768,7 @@ impl<'a> EntityCommands<'a> { /// Pushes an [`EntityCommand`] to the queue, /// which will get executed for the current [`Entity`]. /// - /// The [default error handler](crate::error::default_error_handler) + /// The [default error handler](crate::error::DefaultErrorHandler) /// will be used to handle error cases. /// Every [`EntityCommand`] checks whether the entity exists at the time of execution /// and returns an error if it does not. diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 87340551af..3a1195aea3 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -14,6 +14,7 @@ pub mod unsafe_world_cell; #[cfg(feature = "bevy_reflect")] pub mod reflect; +use crate::error::{DefaultErrorHandler, ErrorHandler}; pub use crate::{ change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD}, world::command_queue::CommandQueue, @@ -3046,6 +3047,16 @@ impl World { // SAFETY: We just initialized the bundle so its id should definitely be valid. unsafe { self.bundles.get(id).debug_checked_unwrap() } } + + /// Convenience method for accessing the world's default error handler, + /// which can be overwritten with [`DefaultErrorHandler`]. + #[inline] + pub fn default_error_handler(&self) -> ErrorHandler { + self.get_resource::() + .copied() + .unwrap_or_default() + .0 + } } impl World { diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index ea5f21c22e..3f74f855a6 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -7,6 +7,7 @@ use crate::{ change_detection::{MaybeLocation, MutUntyped, Ticks, TicksMut}, component::{ComponentId, ComponentTicks, Components, Mutable, StorageType, Tick, TickCells}, entity::{ContainsEntity, Entities, Entity, EntityDoesNotExistError, EntityLocation}, + error::{DefaultErrorHandler, ErrorHandler}, observer::Observers, prelude::Component, query::{DebugCheckedUnwrap, ReadOnlyQueryData}, @@ -705,6 +706,18 @@ impl<'w> UnsafeWorldCell<'w> { (*self.ptr).last_trigger_id = (*self.ptr).last_trigger_id.wrapping_add(1); } } + + /// Convenience method for accessing the world's default error handler, + /// + /// # Safety + /// Must have read access to [`DefaultErrorHandler`]. + #[inline] + pub unsafe fn default_error_handler(&self) -> ErrorHandler { + self.get_resource::() + .copied() + .unwrap_or_default() + .0 + } } impl Debug for UnsafeWorldCell<'_> { diff --git a/crates/bevy_internal/Cargo.toml b/crates/bevy_internal/Cargo.toml index 28d234f2b4..38f89d4ef8 100644 --- a/crates/bevy_internal/Cargo.toml +++ b/crates/bevy_internal/Cargo.toml @@ -279,9 +279,6 @@ custom_cursor = ["bevy_winit/custom_cursor"] # Experimental support for nodes that are ignored for UI layouting ghost_nodes = ["bevy_ui/ghost_nodes"] -# Use the configurable global error handler as the default error handler. -configurable_error_handler = ["bevy_ecs/configurable_error_handler"] - # Allows access to the `std` crate. Enabling this feature will prevent compilation # on `no_std` targets, but provides access to certain additional features on # supported platforms. diff --git a/docs/cargo_features.md b/docs/cargo_features.md index f15fa1c4c6..1a1cb68fda 100644 --- a/docs/cargo_features.md +++ b/docs/cargo_features.md @@ -70,7 +70,6 @@ The default feature set enables most of the expected features of a game engine, |bevy_remote|Enable the Bevy Remote Protocol| |bevy_ui_debug|Provides a debug overlay for bevy UI| |bmp|BMP image format support| -|configurable_error_handler|Use the configurable global error handler as the default error handler.| |critical-section|`critical-section` provides the building blocks for synchronization primitives on all platforms, including `no_std`.| |dds|DDS compressed texture support| |debug_glam_assert|Enable assertions in debug builds to check the validity of parameters passed to glam| diff --git a/examples/ecs/error_handling.rs b/examples/ecs/error_handling.rs index d326d7d4aa..31f9838aaa 100644 --- a/examples/ecs/error_handling.rs +++ b/examples/ecs/error_handling.rs @@ -1,13 +1,7 @@ //! Showcases how fallible systems and observers can make use of Rust's powerful result handling //! syntax. -//! -//! Important note: to set the global error handler, the `configurable_error_handler` feature must be -//! enabled. This feature is disabled by default, as it may introduce runtime overhead, especially for commands. -use bevy::ecs::{ - error::{warn, GLOBAL_ERROR_HANDLER}, - world::DeferredWorld, -}; +use bevy::ecs::{error::warn, world::DeferredWorld}; use bevy::math::sampling::UniformMeshSampler; use bevy::prelude::*; @@ -16,17 +10,15 @@ use rand::SeedableRng; use rand_chacha::ChaCha8Rng; fn main() { + let mut app = App::new(); // By default, fallible systems that return an error will panic. // - // We can change this by setting a custom error handler, which applies globally. - // Here we set the global error handler using one of the built-in - // error handlers. Bevy provides built-in handlers for `panic`, `error`, `warn`, `info`, + // We can change this by setting a custom error handler, which applies to the entire app + // (you can also set it for specific `World`s). + // Here we it using one of the built-in error handlers. + // Bevy provides built-in handlers for `panic`, `error`, `warn`, `info`, // `debug`, `trace` and `ignore`. - GLOBAL_ERROR_HANDLER - .set(warn) - .expect("The error handler can only be set once, globally."); - - let mut app = App::new(); + app.set_error_handler(warn); app.add_plugins(DefaultPlugins); diff --git a/examples/ecs/fallible_params.rs b/examples/ecs/fallible_params.rs index 1ba510cda9..94a8007aec 100644 --- a/examples/ecs/fallible_params.rs +++ b/examples/ecs/fallible_params.rs @@ -2,7 +2,7 @@ //! from running if their acquiry conditions aren't met. //! //! Fallible system parameters include: -//! - [`Res`], [`ResMut`] - Resource has to exist, and the [`GLOBAL_ERROR_HANDLER`] will be called if it doesn't. +//! - [`Res`], [`ResMut`] - Resource has to exist, and the [`World::get_default_error_handler`] will be called if it doesn't. //! - [`Single`] - There must be exactly one matching entity, but the system will be silently skipped otherwise. //! - [`Option>`] - There must be zero or one matching entity. The system will be silently skipped if there are more. //! - [`Populated`] - There must be at least one matching entity, but the system will be silently skipped otherwise. @@ -18,19 +18,13 @@ //! //! [`SystemParamValidationError`]: bevy::ecs::system::SystemParamValidationError //! [`SystemParam::validate_param`]: bevy::ecs::system::SystemParam::validate_param +//! [`default_error_handler`]: bevy::ecs::error::default_error_handler -use bevy::ecs::error::{warn, GLOBAL_ERROR_HANDLER}; +use bevy::ecs::error::warn; use bevy::prelude::*; use rand::Rng; fn main() { - // By default, if a parameter fail to be fetched, - // the `GLOBAL_ERROR_HANDLER` will be used to handle the error, - // which by default is set to panic. - GLOBAL_ERROR_HANDLER - .set(warn) - .expect("The error handler can only be set once, globally."); - println!(); println!("Press 'A' to add enemy ships and 'R' to remove them."); println!("Player ship will wait for enemy ships and track one if it exists,"); @@ -38,6 +32,10 @@ fn main() { println!(); App::new() + // By default, if a parameter fail to be fetched, + // `World::get_default_error_handler` will be used to handle the error, + // which by default is set to panic. + .set_error_handler(warn) .add_plugins(DefaultPlugins) .add_systems(Startup, setup) .add_systems(Update, (user_input, move_targets, track_targets).chain()) diff --git a/release-content/migration-guides/per-world-error-handler.md b/release-content/migration-guides/per-world-error-handler.md new file mode 100644 index 0000000000..1a963d13c6 --- /dev/null +++ b/release-content/migration-guides/per-world-error-handler.md @@ -0,0 +1,10 @@ +--- +title: Global default error handler +pull_requests: [18810] +--- + +Worlds can now have different default error handlers, so there no longer is a global handler. + +Replace uses of `GLOBAL_ERROR_HANDLER` with `App`'s `.set_error_handler(handler)`. +For worlds that do not directly belong to an `App`/`SubApp`, +insert the `DefaultErrorHandler(handler)` resource. From 37b16d869d797c2428ae9afcc189976ac5acdd4d Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 19 May 2025 20:17:20 +0100 Subject: [PATCH 5/9] Remove `YAxisOrientation` from `bevy_text` (#19077) # Objective Been looking for simplifications in the text systems as part of the text input changes. This enum isn't very helpful I think. We can remove it and the associated parameters and instead just negate the glyph's y-offsets in `extract_text2d_sprite`. ## Solution Remove the `YAxisOrientation` enum and parameters. Queue text sprites relative to the top-left in `extract_text2d_sprite` and negate the glyph's y-offset. ## Testing The `text2d` example can be used for testing: ``` cargo run --example text2d ``` --- crates/bevy_text/src/lib.rs | 12 ------------ crates/bevy_text/src/pipeline.rs | 7 +------ crates/bevy_text/src/text2d.rs | 11 +++++------ crates/bevy_ui/src/widget/text.rs | 3 +-- 4 files changed, 7 insertions(+), 26 deletions(-) diff --git a/crates/bevy_text/src/lib.rs b/crates/bevy_text/src/lib.rs index 70ac992924..2bc74a1aa7 100644 --- a/crates/bevy_text/src/lib.rs +++ b/crates/bevy_text/src/lib.rs @@ -87,18 +87,6 @@ pub const DEFAULT_FONT_DATA: &[u8] = include_bytes!("FiraMono-subset.ttf"); #[derive(Default)] pub struct TextPlugin; -/// Text is rendered for two different view projections; -/// 2-dimensional text ([`Text2d`]) is rendered in "world space" with a `BottomToTop` Y-axis, -/// while UI is rendered with a `TopToBottom` Y-axis. -/// This matters for text because the glyph positioning is different in either layout. -/// For `TopToBottom`, 0 is the top of the text, while for `BottomToTop` 0 is the bottom. -pub enum YAxisOrientation { - /// Top to bottom Y-axis orientation, for UI - TopToBottom, - /// Bottom to top Y-axis orientation, for 2d world space - BottomToTop, -} - /// System set in [`PostUpdate`] where all 2d text update systems are executed. #[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] pub struct Text2dUpdateSystems; diff --git a/crates/bevy_text/src/pipeline.rs b/crates/bevy_text/src/pipeline.rs index 2a47866f76..93ee4907bd 100644 --- a/crates/bevy_text/src/pipeline.rs +++ b/crates/bevy_text/src/pipeline.rs @@ -17,7 +17,7 @@ use cosmic_text::{Attrs, Buffer, Family, Metrics, Shaping, Wrap}; use crate::{ error::TextError, ComputedTextBlock, Font, FontAtlasSets, FontSmoothing, JustifyText, - LineBreak, PositionedGlyph, TextBounds, TextEntity, TextFont, TextLayout, YAxisOrientation, + LineBreak, PositionedGlyph, TextBounds, TextEntity, TextFont, TextLayout, }; /// A wrapper resource around a [`cosmic_text::FontSystem`] @@ -228,7 +228,6 @@ impl TextPipeline { font_atlas_sets: &mut FontAtlasSets, texture_atlases: &mut Assets, textures: &mut Assets, - y_axis_orientation: YAxisOrientation, computed: &mut ComputedTextBlock, font_system: &mut CosmicFontSystem, swash_cache: &mut SwashCache, @@ -348,10 +347,6 @@ impl TextPipeline { let x = glyph_size.x as f32 / 2.0 + left + physical_glyph.x as f32; let y = line_y.round() + physical_glyph.y as f32 - top + glyph_size.y as f32 / 2.0; - let y = match y_axis_orientation { - YAxisOrientation::TopToBottom => y, - YAxisOrientation::BottomToTop => box_size.y - y, - }; let position = Vec2::new(x, y); diff --git a/crates/bevy_text/src/text2d.rs b/crates/bevy_text/src/text2d.rs index a9419e89c0..5069804df8 100644 --- a/crates/bevy_text/src/text2d.rs +++ b/crates/bevy_text/src/text2d.rs @@ -2,7 +2,7 @@ use crate::pipeline::CosmicFontSystem; use crate::{ ComputedTextBlock, Font, FontAtlasSets, LineBreak, PositionedGlyph, SwashCache, TextBounds, TextColor, TextError, TextFont, TextLayout, TextLayoutInfo, TextPipeline, TextReader, TextRoot, - TextSpanAccess, TextWriter, YAxisOrientation, + TextSpanAccess, TextWriter, }; use bevy_asset::Assets; use bevy_color::LinearRgba; @@ -182,10 +182,10 @@ pub fn extract_text2d_sprite( text_bounds.width.unwrap_or(text_layout_info.size.x), text_bounds.height.unwrap_or(text_layout_info.size.y), ); - let bottom_left = - -(anchor.as_vec() + 0.5) * size + (size.y - text_layout_info.size.y) * Vec2::Y; + + let top_left = (Anchor::TOP_LEFT.0 - anchor.as_vec()) * size; let transform = - *global_transform * GlobalTransform::from_translation(bottom_left.extend(0.)) * scaling; + *global_transform * GlobalTransform::from_translation(top_left.extend(0.)) * scaling; let mut color = LinearRgba::WHITE; let mut current_span = usize::MAX; @@ -218,7 +218,7 @@ pub fn extract_text2d_sprite( .textures[atlas_info.location.glyph_index] .as_rect(); extracted_slices.slices.push(ExtractedSlice { - offset: *position, + offset: Vec2::new(position.x, -position.y), rect, size: rect.size(), }); @@ -316,7 +316,6 @@ pub fn update_text2d_layout( &mut font_atlas_sets, &mut texture_atlases, &mut textures, - YAxisOrientation::BottomToTop, computed.as_mut(), &mut font_system, &mut swash_cache, diff --git a/crates/bevy_ui/src/widget/text.rs b/crates/bevy_ui/src/widget/text.rs index 0153fa954c..785040c1e9 100644 --- a/crates/bevy_ui/src/widget/text.rs +++ b/crates/bevy_ui/src/widget/text.rs @@ -20,7 +20,7 @@ use bevy_reflect::{std_traits::ReflectDefault, Reflect}; use bevy_text::{ scale_value, ComputedTextBlock, CosmicFontSystem, Font, FontAtlasSets, LineBreak, SwashCache, TextBounds, TextColor, TextError, TextFont, TextLayout, TextLayoutInfo, TextMeasureInfo, - TextPipeline, TextReader, TextRoot, TextSpanAccess, TextWriter, YAxisOrientation, + TextPipeline, TextReader, TextRoot, TextSpanAccess, TextWriter, }; use taffy::style::AvailableSpace; use tracing::error; @@ -328,7 +328,6 @@ fn queue_text( font_atlas_sets, texture_atlases, textures, - YAxisOrientation::TopToBottom, computed, font_system, swash_cache, From fe16624d3c4af10c48b195a5d2196a70ae651445 Mon Sep 17 00:00:00 2001 From: theotherphil Date: Mon, 19 May 2025 20:22:07 +0100 Subject: [PATCH 6/9] Add boilerplate docs for PointerHits::new and HitData::new (#19259) # Objective Add documentation for the last two functions in bevy_picking that are missing them. ## Solution Add boilerplate "Constructs an X" to `PointerHits::new()` and `HitData::new()`. This form of no-information documentation of `new()` functions is used in several places in the repo, and @alice-i-cecile agreed that this is a reasonable approach - the params are already documented on the fields within the struct definition. --------- Co-authored-by: Jan Hohenheim --- crates/bevy_picking/src/backend.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_picking/src/backend.rs b/crates/bevy_picking/src/backend.rs index 3758816ac9..9e28cc6d7c 100644 --- a/crates/bevy_picking/src/backend.rs +++ b/crates/bevy_picking/src/backend.rs @@ -84,7 +84,7 @@ pub struct PointerHits { } impl PointerHits { - #[expect(missing_docs, reason = "Not all docs are written yet, see #3492.")] + /// Construct [`PointerHits`]. pub fn new(pointer: prelude::PointerId, picks: Vec<(Entity, HitData)>, order: f32) -> Self { Self { pointer, @@ -114,7 +114,7 @@ pub struct HitData { } impl HitData { - #[expect(missing_docs, reason = "Not all docs are written yet, see #3492.")] + /// Construct a [`HitData`]. pub fn new(camera: Entity, depth: f32, position: Option, normal: Option) -> Self { Self { camera, From 70e6a9010d256cf982d4659e405b169cc5dcbba0 Mon Sep 17 00:00:00 2001 From: theotherphil Date: Mon, 19 May 2025 20:34:59 +0100 Subject: [PATCH 7/9] Add missing words in Traversal doc comment (#19298) # Objective Minor docs fix - add missing "is responsible". --- crates/bevy_ecs/src/traversal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/traversal.rs b/crates/bevy_ecs/src/traversal.rs index 342ad47849..306ae7c92d 100644 --- a/crates/bevy_ecs/src/traversal.rs +++ b/crates/bevy_ecs/src/traversal.rs @@ -10,7 +10,7 @@ use crate::{entity::Entity, query::ReadOnlyQueryData, relationship::Relationship /// Infinite loops are possible, and are not checked for. While looping can be desirable in some contexts /// (for example, an observer that triggers itself multiple times before stopping), following an infinite /// traversal loop without an eventual exit will cause your application to hang. Each implementer of `Traversal` -/// for documenting possible looping behavior, and consumers of those implementations are responsible for +/// is responsible for documenting possible looping behavior, and consumers of those implementations are responsible for /// avoiding infinite loops in their code. /// /// Traversals may be parameterized with additional data. For example, in observer event propagation, the From 17914943a3c30f9cb006481b8241d3d909f5486e Mon Sep 17 00:00:00 2001 From: Eero Lehtinen Date: Mon, 19 May 2025 22:42:09 +0300 Subject: [PATCH 8/9] Fix spot light shadow glitches (#19273) # Objective Spot light shadows are still broken after fixing point lights in #19265 ## Solution Fix spot lights in the same way, just using the spot light specific visible entities component. I also changed the query to be directly in the render world instead of being extracted to be more accurate. ## Testing Tested with the same code but changing `PointLight` to `SpotLight`. --- crates/bevy_pbr/src/render/light.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index dfc7f679f3..f57ba9adf3 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -221,7 +221,17 @@ pub fn extract_lights( point_light_shadow_map: Extract>, directional_light_shadow_map: Extract>, global_visible_clusterable: Extract>, - cubemap_visible_entities: Extract>>, + previous_point_lights: Query< + Entity, + ( + With, + With, + ), + >, + previous_spot_lights: Query< + Entity, + (With, With), + >, point_lights: Extract< Query<( Entity, @@ -278,14 +288,20 @@ pub fn extract_lights( commands.insert_resource(directional_light_shadow_map.clone()); } - // Clear previous visible entities for all cubemapped lights as they might not be in the + // Clear previous visible entities for all point/spot lights as they might not be in the // `global_visible_clusterable` list anymore. commands.try_insert_batch( - cubemap_visible_entities + previous_point_lights .iter() .map(|render_entity| (render_entity, RenderCubemapVisibleEntities::default())) .collect::>(), ); + commands.try_insert_batch( + previous_spot_lights + .iter() + .map(|render_entity| (render_entity, RenderVisibleMeshEntities::default())) + .collect::>(), + ); // This is the point light shadow map texel size for one face of the cube as a distance of 1.0 // world unit from the light. From 55edb0b476fcd411000b65461366e8a886097858 Mon Sep 17 00:00:00 2001 From: Lucas Franca Date: Mon, 19 May 2025 20:56:48 -0300 Subject: [PATCH 9/9] Fix warnings and errors reported on Rust beta (#19294) # Objective Fixes errors and warnings on this week's Rust beta pipeline * https://github.com/bevyengine/bevy/issues/18748#issuecomment-2890820218 --- clippy.toml | 1 - crates/bevy_macro_utils/src/bevy_manifest.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/clippy.toml b/clippy.toml index 2c98e8ed02..372ffbaf0b 100644 --- a/clippy.toml +++ b/clippy.toml @@ -41,7 +41,6 @@ disallowed-methods = [ { path = "f32::asinh", reason = "use bevy_math::ops::asinh instead for libm determinism" }, { path = "f32::acosh", reason = "use bevy_math::ops::acosh instead for libm determinism" }, { path = "f32::atanh", reason = "use bevy_math::ops::atanh instead for libm determinism" }, - { path = "criterion::black_box", reason = "use core::hint::black_box instead" }, ] # Require `bevy_ecs::children!` to use `[]` braces, instead of `()` or `{}`. diff --git a/crates/bevy_macro_utils/src/bevy_manifest.rs b/crates/bevy_macro_utils/src/bevy_manifest.rs index 8d32781069..b6df0e0e0f 100644 --- a/crates/bevy_macro_utils/src/bevy_manifest.rs +++ b/crates/bevy_macro_utils/src/bevy_manifest.rs @@ -95,7 +95,7 @@ impl BevyManifest { return None; }; - let mut path = Self::parse_str::(&format!("::{}", package)); + let mut path = Self::parse_str::(&format!("::{package}")); if let Some(module) = name.strip_prefix("bevy_") { path.segments.push(Self::parse_str(module)); }