Skip to content

Commit

Permalink
text_system split (#7779)
Browse files Browse the repository at this point in the history
# Objective

`text_system` runs before the UI layout is calculated and the size of
the text node is determined, so it cannot correctly shape the text to
fit the layout, and has no way of determining if the text needs to be
wrapped.

The function `text_constraint` attempts to determine the size of the
node from the local size constraints in the `Style` component. It can't
be made to work, you have to compute the whole layout to get the correct
size. A simple example of where this fails completely is a text node set
to stretch to fill the empty space adjacent to a node with size
constraints set to `Val::Percent(50.)`. The text node will take up half
the space, even though its size constraints are `Val::Auto`

Also because the `text_system` queries for changes to the `Style`
component, when a style value is changed that doesn't affect the node's
geometry the text is recomputed unnecessarily.

Querying on changes to `Node` is not much better. The UI layout is
changed to fit the `CalculatedSize` of the text, so the size of the node
is changed and so the text and UI layout get recalculated multiple times
from a single change to a `Text`.

Also, the `MeasureFunc` doesn't work at all, it doesn't have enough
information to fit the text correctly and makes no attempt.
 
Fixes #7663,  #6717, #5834, #1490,

## Solution

Split the `text_system` into two functions:
* `measure_text_system` which calculates the size constraints for the
text node and runs before `UiSystem::Flex`
* `text_system` which runs after `UiSystem::Flex` and generates the
actual text.
* Fix the `MeasureFunc` calculations.
---

Text wrapping in main:
<img width="961" alt="Capturemain"
src="https://user-images.githubusercontent.com/27962798/220425740-4fe4bf46-24fb-4685-a1cf-bc01e139e72d.PNG">

With this PR:
<img width="961" alt="captured_wrap"
src="https://user-images.githubusercontent.com/27962798/220425807-949996b0-f127-4637-9f33-56a6da944fb0.PNG">

## Changelog

* Removed the previous fields from `CalculatedSize`. `CalculatedSize`
now contains a boxed `Measure`.
 * Added `measurement` module to `bevy_ui`.
* Added the method `create_text_measure` to `TextPipeline`.
* Added a new system `measure_text_system` that runs before
`UiSystem::Flex` that creates a `MeasureFunc` for the text.
* Rescheduled  `text_system` to run after `UiSystem::Flex`.
* Added a trait `Measure`. A `Measure` is used to compute the size of a
UI node when the size of that node is based on its content.
* Added `ImageMeasure` and `TextMeasure` which implement `Measure`.
* Added a new component `UiImageSize` which is used by
`update_image_calculated_size_system` to track image size changes.
* Added a `UiImageSize` component to `ImageBundle`.

## Migration Guide

`ImageBundle` has a new component `UiImageSize` which contains the size
of the image bundle's texture and is updated automatically by
`update_image_calculated_size_system`

---------

Co-authored-by: François <mockersf@gmail.com>
  • Loading branch information
ickshonpe and mockersf authored Apr 17, 2023
1 parent 328347f commit ffc62c1
Show file tree
Hide file tree
Showing 10 changed files with 464 additions and 147 deletions.
143 changes: 141 additions & 2 deletions crates/bevy_text/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use bevy_render::texture::Image;
use bevy_sprite::TextureAtlas;
use bevy_utils::HashMap;

use glyph_brush_layout::{FontId, SectionText};
use glyph_brush_layout::{FontId, GlyphPositioner, SectionGeometry, SectionText};

use crate::{
error::TextError, glyph_brush::GlyphBrush, scale_value, BreakLineOn, Font, FontAtlasSet,
Expand Down Expand Up @@ -54,7 +54,7 @@ impl TextPipeline {
font_atlas_warning: &mut FontAtlasWarning,
y_axis_orientation: YAxisOrientation,
) -> Result<TextLayoutInfo, TextError> {
let mut scaled_fonts = Vec::new();
let mut scaled_fonts = Vec::with_capacity(sections.len());
let sections = sections
.iter()
.map(|section| {
Expand Down Expand Up @@ -92,6 +92,9 @@ impl TextPipeline {
for sg in &section_glyphs {
let scaled_font = scaled_fonts[sg.section_index];
let glyph = &sg.glyph;
// The fonts use a coordinate system increasing upwards so ascent is a positive value
// and descent is negative, but Bevy UI uses a downwards increasing coordinate system,
// so we have to subtract from the baseline position to get the minimum and maximum values.
min_x = min_x.min(glyph.position.x);
min_y = min_y.min(glyph.position.y - scaled_font.ascent());
max_x = max_x.max(glyph.position.x + scaled_font.h_advance(glyph.id));
Expand All @@ -114,4 +117,140 @@ impl TextPipeline {

Ok(TextLayoutInfo { glyphs, size })
}

pub fn create_text_measure(
&mut self,
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
.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)
})
.collect::<Result<Vec<_>, _>>()?;

Ok(TextMeasureInfo::new(
auto_fonts,
scaled_fonts,
sections,
text_alignment,
linebreak_behaviour.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,
) -> Self {
let mut info = Self {
fonts,
scaled_fonts,
sections,
text_alignment,
linebreak_behaviour,
min_width_content_size: Vec2::ZERO,
max_width_content_size: 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;
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 {
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)
.calculate_glyphs(&self.fonts, &geom, sections);

let mut min_x: f32 = std::f32::MAX;
let mut min_y: f32 = std::f32::MAX;
let mut max_x: f32 = std::f32::MIN;
let mut max_y: f32 = std::f32::MIN;

for sg in section_glyphs {
let scaled_font = &self.scaled_fonts[sg.section_index];
let glyph = &sg.glyph;
// The fonts use a coordinate system increasing upwards so ascent is a positive value
// and descent is negative, but Bevy UI uses a downwards increasing coordinate system,
// so we have to subtract from the baseline position to get the minimum and maximum values.
min_x = min_x.min(glyph.position.x);
min_y = min_y.min(glyph.position.y - scaled_font.ascent());
max_x = max_x.max(glyph.position.x + scaled_font.h_advance(glyph.id));
max_y = max_y.max(glyph.position.y - scaled_font.descent());
}

Vec2::new(max_x - min_x, max_y - min_y)
}

pub fn compute_size(&self, bounds: Vec2) -> Vec2 {
let sections = self.prepare_section_texts();
self.compute_size_from_section_texts(&sections, bounds)
}
}
21 changes: 6 additions & 15 deletions crates/bevy_text/src/text2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use bevy_ecs::{
event::EventReader,
prelude::With,
reflect::ReflectComponent,
system::{Commands, Local, Query, Res, ResMut},
system::{Local, Query, Res, ResMut},
};
use bevy_math::{Vec2, Vec3};
use bevy_reflect::Reflect;
Expand Down Expand Up @@ -72,6 +72,8 @@ pub struct Text2dBundle {
pub visibility: Visibility,
/// Algorithmically-computed indication of whether an entity is visible and should be extracted for rendering.
pub computed_visibility: ComputedVisibility,
/// Contains the size of the text and its glyph's position and scale data. Generated via [`TextPipeline::queue_text`]
pub text_layout_info: TextLayoutInfo,
}

pub fn extract_text2d_sprite(
Expand Down Expand Up @@ -147,7 +149,6 @@ pub fn extract_text2d_sprite(
/// It does not modify or observe existing ones.
#[allow(clippy::too_many_arguments)]
pub fn update_text2d_layout(
mut commands: Commands,
// Text items which should be reprocessed again, generally when the font hasn't loaded yet.
mut queue: Local<HashSet<Entity>>,
mut textures: ResMut<Assets<Image>>,
Expand All @@ -159,12 +160,7 @@ pub fn update_text2d_layout(
mut texture_atlases: ResMut<Assets<TextureAtlas>>,
mut font_atlas_set_storage: ResMut<Assets<FontAtlasSet>>,
mut text_pipeline: ResMut<TextPipeline>,
mut text_query: Query<(
Entity,
Ref<Text>,
Ref<Text2dBounds>,
Option<&mut TextLayoutInfo>,
)>,
mut text_query: Query<(Entity, Ref<Text>, Ref<Text2dBounds>, &mut TextLayoutInfo)>,
) {
// We need to consume the entire iterator, hence `last`
let factor_changed = scale_factor_changed.iter().last().is_some();
Expand All @@ -175,7 +171,7 @@ pub fn update_text2d_layout(
.map(|window| window.resolution.scale_factor())
.unwrap_or(1.0);

for (entity, text, bounds, text_layout_info) in &mut text_query {
for (entity, text, bounds, mut text_layout_info) in &mut text_query {
if factor_changed || text.is_changed() || bounds.is_changed() || queue.remove(&entity) {
let text_bounds = Vec2::new(
scale_value(bounds.size.x, scale_factor),
Expand Down Expand Up @@ -204,12 +200,7 @@ pub fn update_text2d_layout(
Err(e @ TextError::FailedToAddGlyph(_)) => {
panic!("Fatal error when processing text: {e}.");
}
Ok(info) => match text_layout_info {
Some(mut t) => *t = info,
None => {
commands.entity(entity).insert(info);
}
},
Ok(info) => *text_layout_info = info,
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ bevy_window = { path = "../bevy_window", version = "0.11.0-dev" }
bevy_utils = { path = "../bevy_utils", version = "0.11.0-dev" }

# other
taffy = { version = "0.3.5", default-features = false, features = ["std"] }
taffy = { version = "0.3.10", default-features = false, features = ["std"] }
serde = { version = "1", features = ["derive"] }
smallvec = { version = "1.6", features = ["union", "const_generics"] }
bytemuck = { version = "1.5", features = ["derive"] }
Expand Down
52 changes: 21 additions & 31 deletions crates/bevy_ui/src/flex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,45 +97,35 @@ impl FlexSurface {
&mut self,
entity: Entity,
style: &Style,
calculated_size: CalculatedSize,
calculated_size: &CalculatedSize,
context: &LayoutContext,
) {
let taffy = &mut self.taffy;
let taffy_style = convert::from_style(context, style);
let scale_factor = context.scale_factor;
let measure = taffy::node::MeasureFunc::Boxed(Box::new(
move |constraints: Size<Option<f32>>, _available: Size<AvailableSpace>| {
let mut size = Size {
width: (scale_factor * calculated_size.size.x as f64) as f32,
height: (scale_factor * calculated_size.size.y as f64) as f32,
};
match (constraints.width, constraints.height) {
(None, None) => {}
(Some(width), None) => {
if calculated_size.preserve_aspect_ratio {
size.height = width * size.height / size.width;
}
size.width = width;
}
(None, Some(height)) => {
if calculated_size.preserve_aspect_ratio {
size.width = height * size.width / size.height;
}
size.height = height;
}
(Some(width), Some(height)) => {
size.width = width;
size.height = height;
}
let measure = calculated_size.measure.dyn_clone();
let measure_func = taffy::node::MeasureFunc::Boxed(Box::new(
move |constraints: Size<Option<f32>>, available: Size<AvailableSpace>| {
let size = measure.measure(
constraints.width,
constraints.height,
available.width,
available.height,
);
taffy::geometry::Size {
width: size.x,
height: size.y,
}
size
},
));
if let Some(taffy_node) = self.entity_to_taffy.get(&entity) {
self.taffy.set_style(*taffy_node, taffy_style).unwrap();
self.taffy.set_measure(*taffy_node, Some(measure)).unwrap();
self.taffy
.set_measure(*taffy_node, Some(measure_func))
.unwrap();
} else {
let taffy_node = taffy.new_leaf_with_measure(taffy_style, measure).unwrap();
let taffy_node = taffy
.new_leaf_with_measure(taffy_style, measure_func)
.unwrap();
self.entity_to_taffy.insert(entity, taffy_node);
}
}
Expand Down Expand Up @@ -307,7 +297,7 @@ pub fn flex_node_system(
for (entity, style, calculated_size) in &query {
// TODO: remove node from old hierarchy if its root has changed
if let Some(calculated_size) = calculated_size {
flex_surface.upsert_leaf(entity, style, *calculated_size, viewport_values);
flex_surface.upsert_leaf(entity, style, calculated_size, viewport_values);
} else {
flex_surface.upsert_node(entity, style, viewport_values);
}
Expand All @@ -322,7 +312,7 @@ pub fn flex_node_system(
}

for (entity, style, calculated_size) in &changed_size_query {
flex_surface.upsert_leaf(entity, style, *calculated_size, &viewport_values);
flex_surface.upsert_leaf(entity, style, calculated_size, &viewport_values);
}

// clean up removed nodes
Expand Down
11 changes: 7 additions & 4 deletions crates/bevy_ui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod ui_node;
#[cfg(feature = "bevy_text")]
mod accessibility;
pub mod camera_config;
pub mod measurement;
pub mod node_bundles;
pub mod update;
pub mod widget;
Expand All @@ -24,17 +25,20 @@ use bevy_render::extract_component::ExtractComponentPlugin;
pub use flex::*;
pub use focus::*;
pub use geometry::*;
pub use measurement::*;
pub use render::*;
pub use ui_node::*;

#[doc(hidden)]
pub mod prelude {
#[doc(hidden)]
pub use crate::{
camera_config::*, geometry::*, node_bundles::*, ui_node::*, widget::*, Interaction, UiScale,
camera_config::*, geometry::*, node_bundles::*, ui_node::*, widget::Button, widget::Label,
Interaction, UiScale,
};
}

use crate::prelude::UiCameraConfig;
use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
use bevy_input::InputSystem;
Expand All @@ -43,8 +47,6 @@ use stack::ui_stack_system;
pub use stack::UiStack;
use update::update_clipping_system;

use crate::prelude::UiCameraConfig;

/// The basic plugin for Bevy UI
#[derive(Default)]
pub struct UiPlugin;
Expand Down Expand Up @@ -114,7 +116,7 @@ impl Plugin for UiPlugin {
#[cfg(feature = "bevy_text")]
app.add_systems(
PostUpdate,
widget::text_system
widget::measure_text_system
.before(UiSystem::Flex)
// Potential conflict: `Assets<Image>`
// In practice, they run independently since `bevy_render::camera_update_system`
Expand Down Expand Up @@ -149,6 +151,7 @@ impl Plugin for UiPlugin {
.before(TransformSystem::TransformPropagate),
ui_stack_system.in_set(UiSystem::Stack),
update_clipping_system.after(TransformSystem::TransformPropagate),
widget::text_system.after(UiSystem::Flex),
),
);

Expand Down
Loading

0 comments on commit ffc62c1

Please sign in to comment.