Merge pull request bevyengine#9 from tigregalis/ct-no-mutex

Remove last mutex on font system

crates/bevy_text/src/error.rs

@@ -11,10 +11,6 @@ pub enum TextError {
     /// Failed to add glyph to a newly created atlas for some reason
     #[error("failed to add glyph to newly-created atlas {0:?}")]
     FailedToAddGlyph(u16),
-    /// Failed to acquire mutex to cosmic-texts fontsystem
-    //TODO: this can be removed since the mutex should be possible to remove as well
-    #[error("font system mutex could not be acquired or is poisoned")]
-    FailedToAcquireMutex,
     /// Failed to get scaled glyph image for cache key
     #[error("failed to get scaled glyph image for cache key: {0:?}")]
     FailedToGetGlyphImage(CacheKey),

crates/bevy_text/src/pipeline.rs

@@ -1,4 +1,4 @@
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 
 use bevy_asset::{AssetId, Assets};
 use bevy_ecs::{component::Component, reflect::ReflectComponent, system::Resource};
@@ -16,16 +16,14 @@ use crate::{
 };
 
 /// A wrapper around a [`cosmic_text::FontSystem`]
-struct CosmicFontSystem(Arc<Mutex<cosmic_text::FontSystem>>);
+struct CosmicFontSystem(cosmic_text::FontSystem);
 
 impl Default for CosmicFontSystem {
     fn default() -> Self {
         let locale = sys_locale::get_locale().unwrap_or_else(|| String::from("en-US"));
         let db = cosmic_text::fontdb::Database::new();
         // TODO: consider using `cosmic_text::FontSystem::new()` (load system fonts by default)
-        Self(Arc::new(Mutex::new(
-            cosmic_text::FontSystem::new_with_locale_and_db(locale, db),
-        )))
+        Self(cosmic_text::FontSystem::new_with_locale_and_db(locale, db))
     }
 }
 
@@ -68,7 +66,7 @@ impl TextPipeline {
         scale_factor: f64,
         buffer: &mut CosmicBuffer,
     ) -> Result<(), TextError> {
-        let font_system = &mut acquire_font_system(&mut self.font_system)?;
+        let font_system = &mut self.font_system.0;
 
         // return early if the fonts are not loaded yet
         let mut font_size = 0.;
@@ -154,7 +152,7 @@ impl TextPipeline {
         )?;
 
         let box_size = buffer_dimensions(&buffer);
-        let font_system = &mut acquire_font_system(&mut self.font_system)?;
+        let font_system = &mut self.font_system.0;
         let swash_cache = &mut self.swash_cache.0;
 
         let glyphs = buffer
@@ -253,20 +251,26 @@ impl TextPipeline {
         let min_width_content_size = buffer_dimensions(&buffer);
 
         let max_width_content_size = {
-            let font_system = &mut acquire_font_system(&mut self.font_system)?;
+            let font_system = &mut self.font_system.0;
             buffer.set_size(font_system, None, None);
             buffer_dimensions(&buffer)
         };
 
         Ok(TextMeasureInfo {
             min: min_width_content_size,
             max: max_width_content_size,
-            font_system: Arc::clone(&self.font_system.0),
             // TODO: This clone feels wasteful, is there another way to structure TextMeasureInfo
             // that it doesn't need to own a buffer? - bytemunch
             buffer: buffer.0.clone(),
         })
     }
+
+    /// Get a mutable reference to the [`cosmic_text::FontSystem`].
+    ///
+    /// Used internally.
+    pub fn font_system_mut(&mut self) -> &mut cosmic_text::FontSystem {
+        &mut self.font_system.0
+    }
 }
 
 /// Render information for a corresponding [`Text`](crate::Text) component.
@@ -291,7 +295,6 @@ pub struct TextMeasureInfo {
     /// Maximum size for a text area in pixels, to be used when laying out widgets with taffy
     pub max: Vec2,
     buffer: cosmic_text::Buffer,
-    font_system: Arc<Mutex<cosmic_text::FontSystem>>,
 }
 
 impl std::fmt::Debug for TextMeasureInfo {
@@ -307,8 +310,11 @@ impl std::fmt::Debug for TextMeasureInfo {
 
 impl TextMeasureInfo {
     /// Computes the size of the text area within the provided bounds.
-    pub fn compute_size(&mut self, bounds: Vec2) -> Vec2 {
-        let font_system = &mut self.font_system.try_lock().expect("Failed to acquire lock");
+    pub fn compute_size(
+        &mut self,
+        bounds: Vec2,
+        font_system: &mut cosmic_text::FontSystem,
+    ) -> Vec2 {
         self.buffer
             .set_size(font_system, Some(bounds.x.ceil()), Some(bounds.y.ceil()));
         buffer_dimensions(&self.buffer)
@@ -374,14 +380,3 @@ fn buffer_dimensions(buffer: &Buffer) -> Vec2 {
 
     Vec2::new(width.ceil(), height).ceil()
 }
-
-/// Helper method to acquire a font system mutex.
-#[inline(always)]
-fn acquire_font_system(
-    font_system: &mut CosmicFontSystem,
-) -> Result<std::sync::MutexGuard<'_, cosmic_text::FontSystem>, TextError> {
-    font_system
-        .0
-        .try_lock()
-        .map_err(|_| TextError::FailedToAcquireMutex)
-}

crates/bevy_text/src/text2d.rs

@@ -230,11 +230,7 @@ pub fn update_text2d_layout(
                     // queue for further processing
                     queue.insert(entity);
                 }
-                Err(
-                    e @ (TextError::FailedToAddGlyph(_)
-                    | TextError::FailedToAcquireMutex
-                    | TextError::FailedToGetGlyphImage(_)),
-                ) => {
+                Err(e @ (TextError::FailedToAddGlyph(_) | TextError::FailedToGetGlyphImage(_))) => {
                     panic!("Fatal error when processing text: {e}.");
                 }
                 Ok(mut info) => {

crates/bevy_ui/src/layout/mod.rs

@@ -1,3 +1,4 @@
+use bevy_text::TextPipeline;
 use thiserror::Error;
 
 use crate::{ContentSize, DefaultUiCamera, Node, Outline, Style, TargetCamera, UiScale};
@@ -94,6 +95,7 @@ pub fn ui_layout_system(
     just_children_query: Query<&Children>,
     mut removed_components: UiLayoutSystemRemovedComponentParam,
     mut node_transform_query: Query<(&mut Node, &mut Transform)>,
+    #[cfg(feature = "bevy_text")] mut text_pipeline: ResMut<TextPipeline>,
 ) {
     struct CameraLayoutInfo {
         size: UVec2,
@@ -217,10 +219,18 @@ pub fn ui_layout_system(
         }
     }
 
+    #[cfg(feature = "bevy_text")]
+    let font_system = text_pipeline.font_system_mut();
+
     for (camera_id, camera) in &camera_layout_info {
         let inverse_target_scale_factor = camera.scale_factor.recip();
 
-        ui_surface.compute_camera_layout(*camera_id, camera.size);
+        ui_surface.compute_camera_layout(
+            *camera_id,
+            camera.size,
+            #[cfg(feature = "bevy_text")]
+            font_system,
+        );
         for root in &camera.root_nodes {
             update_uinode_geometry_recursive(
                 *root,

crates/bevy_ui/src/layout/ui_surface.rs

@@ -10,7 +10,7 @@ use bevy_utils::default;
 use bevy_utils::tracing::warn;
 
 use crate::layout::convert;
-use crate::{LayoutContext, LayoutError, Measure, NodeMeasure, Style};
+use crate::{LayoutContext, LayoutError, Measure, MeasureArgs, NodeMeasure, Style};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct RootNodePair {
@@ -196,7 +196,12 @@ without UI components as a child of an entity with UI components, results may be
     }
 
     /// Compute the layout for each window entity's corresponding root node in the layout.
-    pub fn compute_camera_layout(&mut self, camera: Entity, render_target_resolution: UVec2) {
+    pub fn compute_camera_layout(
+        &mut self,
+        camera: Entity,
+        render_target_resolution: UVec2,
+        #[cfg(feature = "bevy_text")] font_system: &mut bevy_text::cosmic_text::FontSystem,
+    ) {
         let Some(camera_root_nodes) = self.camera_roots.get(&camera) else {
             return;
         };
@@ -219,10 +224,14 @@ without UI components as a child of an entity with UI components, results may be
                         context
                             .map(|ctx| {
                                 let size = ctx.measure(
-                                    known_dimensions.width,
-                                    known_dimensions.height,
-                                    available_space.width,
-                                    available_space.height,
+                                    MeasureArgs {
+                                        width: known_dimensions.width,
+                                        height: known_dimensions.height,
+                                        available_width: available_space.width,
+                                        available_height: available_space.height,
+                                        #[cfg(feature = "bevy_text")]
+                                        font_system,
+                                    },
                                     style,
                                 );
                                 taffy::Size {

crates/bevy_ui/src/measurement.rs

@@ -16,18 +16,20 @@ impl std::fmt::Debug for ContentSize {
     }
 }
 
+pub struct MeasureArgs<'a> {
+    pub width: Option<f32>,
+    pub height: Option<f32>,
+    pub available_width: AvailableSpace,
+    pub available_height: AvailableSpace,
+    #[cfg(feature = "bevy_text")]
+    pub font_system: &'a mut bevy_text::cosmic_text::FontSystem,
+}
+
 /// A `Measure` is used to compute the size of a ui node
 /// when the size of that node is based on its content.
 pub trait Measure: Send + Sync + 'static {
     /// Calculate the size of the node given the constraints.
-    fn measure(
-        &mut self,
-        width: Option<f32>,
-        height: Option<f32>,
-        available_width: AvailableSpace,
-        available_height: AvailableSpace,
-        style: &taffy::Style,
-    ) -> Vec2;
+    fn measure<'a>(&mut self, measure_args: MeasureArgs<'a>, style: &taffy::Style) -> Vec2;
 }
 
 /// A type to serve as Taffy's node context (which allows the content size of leaf nodes to be computed)
@@ -43,28 +45,13 @@ pub enum NodeMeasure {
 }
 
 impl Measure for NodeMeasure {
-    fn measure(
-        &mut self,
-        width: Option<f32>,
-        height: Option<f32>,
-        available_width: AvailableSpace,
-        available_height: AvailableSpace,
-        style: &taffy::Style,
-    ) -> Vec2 {
+    fn measure(&mut self, measure_args: MeasureArgs, style: &taffy::Style) -> Vec2 {
         match self {
-            NodeMeasure::Fixed(fixed) => {
-                fixed.measure(width, height, available_width, available_height, style)
-            }
+            NodeMeasure::Fixed(fixed) => fixed.measure(measure_args, style),
             #[cfg(feature = "bevy_text")]
-            NodeMeasure::Text(text) => {
-                text.measure(width, height, available_width, available_height, style)
-            }
-            NodeMeasure::Image(image) => {
-                image.measure(width, height, available_width, available_height, style)
-            }
-            NodeMeasure::Custom(custom) => {
-                custom.measure(width, height, available_width, available_height, style)
-            }
+            NodeMeasure::Text(text) => text.measure(measure_args, style),
+            NodeMeasure::Image(image) => image.measure(measure_args, style),
+            NodeMeasure::Custom(custom) => custom.measure(measure_args, style),
         }
     }
 }
@@ -77,14 +64,7 @@ pub struct FixedMeasure {
 }
 
 impl Measure for FixedMeasure {
-    fn measure(
-        &mut self,
-        _: Option<f32>,
-        _: Option<f32>,
-        _: AvailableSpace,
-        _: AvailableSpace,
-        _: &taffy::Style,
-    ) -> Vec2 {
+    fn measure(&mut self, _: MeasureArgs, _: &taffy::Style) -> Vec2 {
         self.size
     }
 }

crates/bevy_ui/src/widget/image.rs

@@ -1,5 +1,6 @@
 use crate::{
-    measurement::AvailableSpace, ContentSize, Measure, Node, NodeMeasure, UiImage, UiScale,
+    ContentSize, Measure, MeasureArgs, Node, NodeMeasure, UiImage,
+    UiScale,
 };
 use bevy_asset::Assets;
 use bevy_ecs::prelude::*;
@@ -37,14 +38,15 @@ pub struct ImageMeasure {
 }
 
 impl Measure for ImageMeasure {
-    fn measure(
-        &mut self,
-        width: Option<f32>,
-        height: Option<f32>,
-        available_width: AvailableSpace,
-        available_height: AvailableSpace,
-        style: &taffy::Style,
-    ) -> Vec2 {
+    fn measure(&mut self, measure_args: MeasureArgs, style: &taffy::Style) -> Vec2 {
+        let MeasureArgs {
+            width,
+            height,
+            available_width,
+            available_height,
+            ..
+        } = measure_args;
+
         // Convert available width/height into an option
         let parent_width = available_width.into_option();
         let parent_height = available_height.into_option();

crates/bevy_ui/src/widget/text.rs

@@ -1,5 +1,6 @@
 use crate::{
-    ContentSize, DefaultUiCamera, FixedMeasure, Measure, Node, NodeMeasure, TargetCamera, UiScale,
+    ContentSize, DefaultUiCamera, FixedMeasure, Measure, MeasureArgs, Node, NodeMeasure,
+    TargetCamera, UiScale,
 };
 use bevy_asset::Assets;
 use bevy_ecs::{
@@ -47,14 +48,14 @@ pub struct TextMeasure {
 }
 
 impl Measure for TextMeasure {
-    fn measure(
-        &mut self,
-        width: Option<f32>,
-        height: Option<f32>,
-        available_width: AvailableSpace,
-        _available_height: AvailableSpace,
-        _style: &taffy::Style,
-    ) -> Vec2 {
+    fn measure(&mut self, measure_args: MeasureArgs, _style: &taffy::Style) -> Vec2 {
+        let MeasureArgs {
+            width,
+            height,
+            available_width,
+            font_system,
+            ..
+        } = measure_args;
         let x = width.unwrap_or_else(|| match available_width {
             AvailableSpace::Definite(x) => {
                 // It is possible for the "min content width" to be larger than
@@ -70,7 +71,9 @@ impl Measure for TextMeasure {
         height
             .map_or_else(
                 || match available_width {
-                    AvailableSpace::Definite(_) => self.info.compute_size(Vec2::new(x, f32::MAX)),
+                    AvailableSpace::Definite(_) => {
+                        self.info.compute_size(Vec2::new(x, f32::MAX), font_system)
+                    }
                     AvailableSpace::MinContent => Vec2::new(x, self.info.min.y),
                     AvailableSpace::MaxContent => Vec2::new(x, self.info.max.y),
                 },
@@ -112,11 +115,7 @@ fn create_text_measure(
             // Try again next frame
             text_flags.needs_new_measure_func = true;
         }
-        Err(
-            e @ (TextError::FailedToAddGlyph(_)
-            | TextError::FailedToAcquireMutex
-            | TextError::FailedToGetGlyphImage(_)),
-        ) => {
+        Err(e @ (TextError::FailedToAddGlyph(_) | TextError::FailedToGetGlyphImage(_))) => {
             panic!("Fatal error when processing text: {e}.");
         }
     };
@@ -232,11 +231,7 @@ fn queue_text(
                 // There was an error processing the text layout, try again next frame
                 text_flags.needs_recompute = true;
             }
-            Err(
-                e @ (TextError::FailedToAddGlyph(_)
-                | TextError::FailedToAcquireMutex
-                | TextError::FailedToGetGlyphImage(_)),
-            ) => {
+            Err(e @ (TextError::FailedToAddGlyph(_) | TextError::FailedToGetGlyphImage(_))) => {
                 panic!("Fatal error when processing text: {e}.");
             }
             Ok(mut info) => {