Cleanup some bevy_text pipeline.rs (#9111)

## Objective

- `bevy_text/src/pipeline.rs` had some crufty code.

## Solution

Remove the cruft.

- `&mut self` argument was unused by
`TextPipeline::create_text_measure`, so we replace it with a constructor
`TextMeasureInfo::from_text`.
- We also pass a `&Text` to `from_text` since there is no reason to
split the struct before passing it as argument.
- from_text also checks beforehand that every Font exist in the
Assets<Font>. This allows rust to skip the drop code on the Vecs we
create in the method, since there is no early exit.
- We also remove the scaled_fonts field on `TextMeasureInfo`. This
avoids an additional allocation. We can re-use the font on `fonts`
instead in `compute_size`. Building a `ScaledFont` seems fairly cheap,
when looking at the ab_glyph internals.
- We also implement ToSectionText on TextMeasureSection, this let us
skip creating a whole new Vec each time we call compute_size.
- This let us remove compute_size_from_section_text, since its only
purpose was to not have to allocate the Vec we just made redundant.
- Make some immutabe `Vec<T>` into `Box<[T]>` and `String` into
`Box<str>`
- `{min,max}_width_content_size` fields of `TextMeasureInfo` have name
`width` in them, yet the contain information on both width and height.
- `TextMeasureInfo::linebreak_behaviour`  -> `linebreak_behavior`

## Migration Guide

- The `ResMut<TextPipeline>` argument to `measure_text_system` doesn't
exist anymore. If you were calling this system manually, you should
remove the argument.
- The `{min,max}_width_content_size` fields of `TextMeasureInfo` are
renamed to `min` and `max` respectively
- Other changes to `TextMeasureInfo` may also break your code if you
were manually building it. Please consider using the new
`TextMeasureInfo::from_text` to build one instead.
- `TextPipeline::create_text_measure` has been removed in favor of
`TextMeasureInfo::from_text`

Author: nicopap

crates/bevy_text/src/glyph_brush.rs

@@ -84,7 +84,7 @@ impl GlyphBrush {
             })
             .collect::<Result<Vec<_>, _>>()?;
 
-        let text_bounds = compute_text_bounds(&glyphs, |index| &sections_data[index].3);
+        let text_bounds = compute_text_bounds(&glyphs, |index| sections_data[index].3);
 
         let mut positioned_glyphs = Vec::new();
         for sg in glyphs {
@@ -203,12 +203,12 @@ impl GlyphPlacementAdjuster {
 
 /// Computes the minimal bounding rectangle for a block of text.
 /// Ignores empty trailing lines.
-pub(crate) fn compute_text_bounds<'a, T>(
+pub(crate) fn compute_text_bounds<T>(
     section_glyphs: &[SectionGlyph],
-    get_scaled_font: impl Fn(usize) -> &'a PxScaleFont<T>,
+    get_scaled_font: impl Fn(usize) -> PxScaleFont<T>,
 ) -> bevy_math::Rect
 where
-    T: ab_glyph::Font + 'a,
+    T: ab_glyph::Font,
 {
     let mut text_bounds = Rect {
         min: Vec2::splat(std::f32::MAX),

crates/bevy_text/src/pipeline.rs

@@ -1,4 +1,4 @@
-use ab_glyph::PxScale;
+use ab_glyph::{Font as AbglyphFont, PxScale};
 use bevy_asset::{Assets, Handle, HandleId};
 use bevy_ecs::component::Component;
 use bevy_ecs::system::Resource;
@@ -7,12 +7,12 @@ use bevy_render::texture::Image;
 use bevy_sprite::TextureAtlas;
 use bevy_utils::HashMap;
 
-use glyph_brush_layout::{FontId, GlyphPositioner, SectionGeometry, SectionText};
+use glyph_brush_layout::{FontId, GlyphPositioner, SectionGeometry, SectionText, ToSectionText};
 
 use crate::{
     compute_text_bounds, error::TextError, glyph_brush::GlyphBrush, scale_value, BreakLineOn, Font,
-    FontAtlasSet, FontAtlasWarning, PositionedGlyph, TextAlignment, TextSection, TextSettings,
-    YAxisOrientation,
+    FontAtlasSet, FontAtlasWarning, PositionedGlyph, Text, TextAlignment, TextSection,
+    TextSettings, YAxisOrientation,
 };
 
 #[derive(Default, Resource)]
@@ -85,7 +85,7 @@ impl TextPipeline {
             return Ok(TextLayoutInfo::default());
         }
 
-        let size = compute_text_bounds(&section_glyphs, |index| &scaled_fonts[index]).size();
+        let size = compute_text_bounds(&section_glyphs, |index| scaled_fonts[index]).size();
 
         let glyphs = self.brush.process_glyphs(
             section_glyphs,
@@ -101,123 +101,110 @@ impl TextPipeline {
 
         Ok(TextLayoutInfo { glyphs, size })
     }
+}
 
-    pub fn create_text_measure(
-        &mut self,
+#[derive(Debug, Clone)]
+pub struct TextMeasureSection {
+    pub text: Box<str>,
+    pub scale: f32,
+    pub font_id: FontId,
+}
+
+#[derive(Debug, Clone, Default)]
+pub struct TextMeasureInfo {
+    pub fonts: Box<[ab_glyph::FontArc]>,
+    pub sections: Box<[TextMeasureSection]>,
+    pub text_alignment: TextAlignment,
+    pub linebreak_behavior: glyph_brush_layout::BuiltInLineBreaker,
+    pub min: Vec2,
+    pub max: Vec2,
+}
+
+impl TextMeasureInfo {
+    pub fn from_text(
+        text: &Text,
         fonts: &Assets<Font>,
-        sections: &[TextSection],
         scale_factor: f64,
-        text_alignment: TextAlignment,
-        linebreak_behaviour: BreakLineOn,
     ) -> Result<TextMeasureInfo, TextError> {
-        let mut auto_fonts = Vec::with_capacity(sections.len());
-        let mut scaled_fonts = Vec::with_capacity(sections.len());
-        let sections = sections
+        let sections = &text.sections;
+        for section in sections {
+            if !fonts.contains(&section.style.font) {
+                return Err(TextError::NoSuchFont);
+            }
+        }
+        let (auto_fonts, sections) = sections
             .iter()
             .enumerate()
             .map(|(i, section)| {
-                let font = fonts
-                    .get(&section.style.font)
-                    .ok_or(TextError::NoSuchFont)?;
-                let font_size = scale_value(section.style.font_size, scale_factor);
-                auto_fonts.push(font.font.clone());
-                let px_scale_font = ab_glyph::Font::into_scaled(font.font.clone(), font_size);
-                scaled_fonts.push(px_scale_font);
-
-                let section = TextMeasureSection {
-                    font_id: FontId(i),
-                    scale: PxScale::from(font_size),
-                    text: section.value.clone(),
-                };
-
-                Ok(section)
+                // SAFETY: we exited early earlier in this function if
+                // one of the fonts was missing.
+                let font = unsafe { fonts.get(&section.style.font).unwrap_unchecked() };
+                (
+                    font.font.clone(),
+                    TextMeasureSection {
+                        font_id: FontId(i),
+                        scale: scale_value(section.style.font_size, scale_factor),
+                        text: section.value.clone().into_boxed_str(),
+                    },
+                )
             })
-            .collect::<Result<Vec<_>, _>>()?;
+            .unzip();
 
-        Ok(TextMeasureInfo::new(
+        Ok(Self::new(
             auto_fonts,
-            scaled_fonts,
             sections,
-            text_alignment,
-            linebreak_behaviour.into(),
+            text.alignment,
+            text.linebreak_behavior.into(),
         ))
     }
-}
-
-#[derive(Debug, Clone)]
-pub struct TextMeasureSection {
-    pub text: String,
-    pub scale: PxScale,
-    pub font_id: FontId,
-}
-
-#[derive(Debug, Clone)]
-pub struct TextMeasureInfo {
-    pub fonts: Vec<ab_glyph::FontArc>,
-    pub scaled_fonts: Vec<ab_glyph::PxScaleFont<ab_glyph::FontArc>>,
-    pub sections: Vec<TextMeasureSection>,
-    pub text_alignment: TextAlignment,
-    pub linebreak_behaviour: glyph_brush_layout::BuiltInLineBreaker,
-    pub min_width_content_size: Vec2,
-    pub max_width_content_size: Vec2,
-}
-
-impl TextMeasureInfo {
     fn new(
         fonts: Vec<ab_glyph::FontArc>,
-        scaled_fonts: Vec<ab_glyph::PxScaleFont<ab_glyph::FontArc>>,
         sections: Vec<TextMeasureSection>,
         text_alignment: TextAlignment,
-        linebreak_behaviour: glyph_brush_layout::BuiltInLineBreaker,
+        linebreak_behavior: glyph_brush_layout::BuiltInLineBreaker,
     ) -> Self {
         let mut info = Self {
-            fonts,
-            scaled_fonts,
-            sections,
+            fonts: fonts.into_boxed_slice(),
+            sections: sections.into_boxed_slice(),
             text_alignment,
-            linebreak_behaviour,
-            min_width_content_size: Vec2::ZERO,
-            max_width_content_size: Vec2::ZERO,
+            linebreak_behavior,
+            min: Vec2::ZERO,
+            max: Vec2::ZERO,
         };
 
-        let section_texts = info.prepare_section_texts();
-        let min =
-            info.compute_size_from_section_texts(&section_texts, Vec2::new(0.0, f32::INFINITY));
-        let max = info.compute_size_from_section_texts(
-            &section_texts,
-            Vec2::new(f32::INFINITY, f32::INFINITY),
-        );
-        info.min_width_content_size = min;
-        info.max_width_content_size = max;
+        let min = info.compute_size(Vec2::new(0.0, f32::INFINITY));
+        let max = info.compute_size(Vec2::INFINITY);
+        info.min = min;
+        info.max = max;
         info
     }
 
-    fn prepare_section_texts(&self) -> Vec<SectionText> {
-        self.sections
-            .iter()
-            .map(|section| SectionText {
-                font_id: section.font_id,
-                scale: section.scale,
-                text: &section.text,
-            })
-            .collect::<Vec<_>>()
-    }
-
-    fn compute_size_from_section_texts(&self, sections: &[SectionText], bounds: Vec2) -> Vec2 {
+    pub fn compute_size(&self, bounds: Vec2) -> Vec2 {
+        let sections = &self.sections;
         let geom = SectionGeometry {
             bounds: (bounds.x, bounds.y),
             ..Default::default()
         };
         let section_glyphs = glyph_brush_layout::Layout::default()
             .h_align(self.text_alignment.into())
-            .line_breaker(self.linebreak_behaviour)
+            .line_breaker(self.linebreak_behavior)
             .calculate_glyphs(&self.fonts, &geom, sections);
 
-        compute_text_bounds(&section_glyphs, |index| &self.scaled_fonts[index]).size()
+        compute_text_bounds(&section_glyphs, |index| {
+            let font = &self.fonts[index];
+            let font_size = self.sections[index].scale;
+            font.into_scaled(font_size)
+        })
+        .size()
     }
-
-    pub fn compute_size(&self, bounds: Vec2) -> Vec2 {
-        let sections = self.prepare_section_texts();
-        self.compute_size_from_section_texts(&sections, bounds)
+}
+impl ToSectionText for TextMeasureSection {
+    #[inline(always)]
+    fn to_section_text(&self) -> SectionText<'_> {
+        SectionText {
+            text: &self.text,
+            scale: PxScale::from(self.scale),
+            font_id: self.font_id,
+        }
     }
 }

crates/bevy_text/src/text.rs

@@ -140,11 +140,12 @@ impl TextSection {
 }
 
 /// Describes horizontal alignment preference for positioning & bounds.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Reflect, Serialize, Deserialize)]
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Reflect, Serialize, Deserialize)]
 #[reflect(Serialize, Deserialize)]
 pub enum TextAlignment {
     /// Leftmost character is immediately to the right of the render position.<br/>
     /// Bounds start from the render position and advance rightwards.
+    #[default]
     Left,
     /// Leftmost & rightmost characters are equidistant to the render position.<br/>
     /// Bounds start from the render position and advance equally left & right.

crates/bevy_ui/src/widget/text.rs

@@ -53,20 +53,17 @@ impl Measure for TextMeasure {
         _available_height: AvailableSpace,
     ) -> Vec2 {
         let x = width.unwrap_or_else(|| match available_width {
-            AvailableSpace::Definite(x) => x.clamp(
-                self.info.min_width_content_size.x,
-                self.info.max_width_content_size.x,
-            ),
-            AvailableSpace::MinContent => self.info.min_width_content_size.x,
-            AvailableSpace::MaxContent => self.info.max_width_content_size.x,
+            AvailableSpace::Definite(x) => x.clamp(self.info.min.x, self.info.max.x),
+            AvailableSpace::MinContent => self.info.min.x,
+            AvailableSpace::MaxContent => self.info.max.x,
         });
 
         height
             .map_or_else(
                 || match available_width {
                     AvailableSpace::Definite(_) => self.info.compute_size(Vec2::new(x, f32::MAX)),
-                    AvailableSpace::MinContent => Vec2::new(x, self.info.min_width_content_size.y),
-                    AvailableSpace::MaxContent => Vec2::new(x, self.info.max_width_content_size.y),
+                    AvailableSpace::MinContent => Vec2::new(x, self.info.min.y),
+                    AvailableSpace::MaxContent => Vec2::new(x, self.info.max.y),
                 },
                 |y| Vec2::new(x, y),
             )
@@ -77,24 +74,15 @@ impl Measure for TextMeasure {
 #[inline]
 fn create_text_measure(
     fonts: &Assets<Font>,
-    text_pipeline: &mut TextPipeline,
     scale_factor: f64,
     text: Ref<Text>,
     mut content_size: Mut<ContentSize>,
     mut text_flags: Mut<TextFlags>,
 ) {
-    match text_pipeline.create_text_measure(
-        fonts,
-        &text.sections,
-        scale_factor,
-        text.alignment,
-        text.linebreak_behavior,
-    ) {
+    match TextMeasureInfo::from_text(&text, fonts, scale_factor) {
         Ok(measure) => {
             if text.linebreak_behavior == BreakLineOn::NoWrap {
-                content_size.set(FixedMeasure {
-                    size: measure.max_width_content_size,
-                });
+                content_size.set(FixedMeasure { size: measure.max });
             } else {
                 content_size.set(TextMeasure { info: measure });
             }
@@ -127,7 +115,6 @@ pub fn measure_text_system(
     fonts: Res<Assets<Font>>,
     windows: Query<&Window, With<PrimaryWindow>>,
     ui_scale: Res<UiScale>,
-    mut text_pipeline: ResMut<TextPipeline>,
     mut text_query: Query<(Ref<Text>, &mut ContentSize, &mut TextFlags), With<Node>>,
 ) {
     let window_scale_factor = windows
@@ -142,29 +129,15 @@ pub fn measure_text_system(
         // scale factor unchanged, only create new measure funcs for modified text
         for (text, content_size, text_flags) in text_query.iter_mut() {
             if text.is_changed() || text_flags.needs_new_measure_func {
-                create_text_measure(
-                    &fonts,
-                    &mut text_pipeline,
-                    scale_factor,
-                    text,
-                    content_size,
-                    text_flags,
-                );
+                create_text_measure(&fonts, scale_factor, text, content_size, text_flags);
             }
         }
     } else {
         // scale factor changed, create new measure funcs for all text
         *last_scale_factor = scale_factor;
 
         for (text, content_size, text_flags) in text_query.iter_mut() {
-            create_text_measure(
-                &fonts,
-                &mut text_pipeline,
-                scale_factor,
-                text,
-                content_size,
-                text_flags,
-            );
+            create_text_measure(&fonts, scale_factor, text, content_size, text_flags);
         }
     }
 }