migrations/egui
1
0
Fork 0
mirror of https://github.com/emilk/egui.git synced 2026-06-26 14:49:06 -04:00
Code Activity

Avoid cloning Rows during Galley::concat (#7649)

Browse Source 
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/main/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

Moves `ends_with_newline` into `PlacedRow` to avoid clones during
layout.
I don't think there was a rationale stronger than "don't change too
much" for not doing this in https://github.com/emilk/egui/pull/5411, so
I should've just done this from the start.
This was a significant part of the profile for text layout (as it cloned
almost every `Row`, even though it only needed to change a single
boolean).

Before:
<img width="757" height="250" alt="image"
src="https://github.com/user-attachments/assets/d1c2afd1-f1ec-4cf5-9d05-f5a5a78052df"
/>

After:
<img width="615" height="249" alt="image"
src="https://github.com/user-attachments/assets/c70966da-c892-4e84-adba-494d0f37f263"
/>

(note that these profiles focus solely on the top-level
`Galley::layout_inline` subtree, also don't compare sample count as the
duration of these tests was completely arbitrary)

egui_demo_lib `*text_layout*` benches:
<img width="791" height="461" alt="image"
src="https://github.com/user-attachments/assets/4f97ce84-2768-4876-9488-d42f8f358ed1"
/>

* [X] I have followed the instructions in the PR template

(As usual, the tests fail for me even on master but the failures on
master and with these changes seem the same :))
This commit is contained in:
Hubert Głuchowski
2025-10-31 09:45:32 +00:00
committed by GitHub
parent 2669344d5c
commit e861c8ec79
4 changed files with 35 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
crates/egui/src/text_selection/visuals.rs
View File

@@ -31,7 +31,8 @@ pub fn paint_text_selection(
let max = galley.layout_from_cursor(max);
for ri in min.row..=max.row {
let row = Arc::make_mut(&mut galley.rows[ri].row);
let placed_row = &mut galley.rows[ri];
let row = Arc::make_mut(&mut placed_row.row);
let left = if ri == min.row {
row.x_offset(min.column)
@@ -41,7 +42,7 @@ pub fn paint_text_selection(
let right = if ri == max.row {
row.x_offset(max.column)
} else {
let newline_size = if row.ends_with_newline {
let newline_size = if placed_row.ends_with_newline {
row.height() / 2.0 // visualize that we select the newline
} else {
0.0

8
crates/epaint/src/shapes/text_shape.rs
View File

@@ -137,7 +137,12 @@ impl TextShape {
*mesh_bounds = transform.scaling * *mesh_bounds;
*intrinsic_size = transform.scaling * *intrinsic_size;
for text::PlacedRow { pos, row } in rows {
for text::PlacedRow {
pos,
row,
ends_with_newline: _,
} in rows
{
*pos *= transform.scaling;
let text::Row {
@@ -145,7 +150,6 @@ impl TextShape {
glyphs: _, // TODO(emilk): would it make sense to transform these?
size,
visuals,
ends_with_newline: _,
} = Arc::make_mut(row);
*size *= transform.scaling;

13
crates/epaint/src/text/text_layout.rs
View File

@@ -296,8 +296,8 @@ fn rows_from_paragraphs(
glyphs: vec![],
visuals: Default::default(),
size: vec2(0.0, paragraph.empty_paragraph_height),
ends_with_newline: !is_last_paragraph,
}),
ends_with_newline: !is_last_paragraph,
});
} else {
let paragraph_max_x = paragraph.glyphs.last().unwrap().max_x();
@@ -310,14 +310,13 @@ fn rows_from_paragraphs(
glyphs: paragraph.glyphs,
visuals: Default::default(),
size: vec2(paragraph_max_x, 0.0),
ends_with_newline: !is_last_paragraph,
}),
ends_with_newline: !is_last_paragraph,
});
} else {
line_break(&paragraph, job, &mut rows, elided);
let placed_row = rows.last_mut().unwrap();
let row = Arc::make_mut(&mut placed_row.row);
row.ends_with_newline = !is_last_paragraph;
placed_row.ends_with_newline = !is_last_paragraph;
}
}
}
@@ -363,8 +362,8 @@ fn line_break(
glyphs: vec![],
visuals: Default::default(),
size: Vec2::ZERO,
ends_with_newline: false,
}),
ends_with_newline: false,
});
row_start_x += first_row_indentation;
first_row_indentation = 0.0;
@@ -389,8 +388,8 @@ fn line_break(
glyphs,
visuals: Default::default(),
size: vec2(paragraph_max_x, 0.0),
ends_with_newline: false,
}),
ends_with_newline: false,
});
// Start a new row:
@@ -431,8 +430,8 @@ fn line_break(
glyphs,
visuals: Default::default(),
size: vec2(paragraph_max_x - paragraph_min_x, 0.0),
ends_with_newline: false,
}),
ends_with_newline: false,
});
}
}

38
crates/epaint/src/text/text_layout_types.rs
View File

@@ -572,6 +572,13 @@ pub struct PlacedRow {
/// The underlying unpositioned [`Row`].
pub row: Arc<Row>,
/// If true, this [`PlacedRow`] came from a paragraph ending with a `\n`.
/// The `\n` itself is omitted from row's [`Row::glyphs`].
/// A `\n` in the input text always creates a new [`PlacedRow`] below it,
/// so that text that ends with `\n` has an empty [`PlacedRow`] last.
/// This also implies that the last [`PlacedRow`] in a [`Galley`] always has `ends_with_newline == false`.
pub ends_with_newline: bool,
}
impl PlacedRow {
@@ -617,13 +624,6 @@ pub struct Row {
/// The mesh, ready to be rendered.
pub visuals: RowVisuals,
/// If true, this [`Row`] came from a paragraph ending with a `\n`.
/// The `\n` itself is omitted from [`Self::glyphs`].
/// A `\n` in the input text always creates a new [`Row`] below it,
/// so that text that ends with `\n` has an empty [`Row`] last.
/// This also implies that the last [`Row`] in a [`Galley`] always has `ends_with_newline == false`.
pub ends_with_newline: bool,
}
/// The tessellated output of a row.
@@ -735,12 +735,6 @@ impl Row {
self.glyphs.len()
}
/// Includes the implicit `\n` after the [`Row`], if any.
#[inline]
pub fn char_count_including_newline(&self) -> usize {
self.glyphs.len() + (self.ends_with_newline as usize)
}
/// Closest char at the desired x coordinate in row-relative coordinates.
/// Returns something in the range `[0, char_count_excluding_newline()]`.
pub fn char_at(&self, desired_x: f32) -> usize {
@@ -776,6 +770,12 @@ impl PlacedRow {
pub fn max_y(&self) -> f32 {
self.rect().bottom()
}
/// Includes the implicit `\n` after the [`PlacedRow`], if any.
#[inline]
pub fn char_count_including_newline(&self) -> usize {
self.row.glyphs.len() + (self.ends_with_newline as usize)
}
}
impl Galley {
@@ -867,13 +867,15 @@ impl Galley {
placed_row.visuals.mesh_bounds.translate(new_pos.to_vec2());
merged_galley.rect |= Rect::from_min_size(new_pos, placed_row.size);
let mut row = placed_row.row.clone();
let mut ends_with_newline = placed_row.ends_with_newline;
let is_last_row_in_galley = row_idx + 1 == galley.rows.len();
if !is_last_galley && is_last_row_in_galley {
// Since we remove the `\n` when splitting rows, we need to add it back here
Arc::make_mut(&mut row).ends_with_newline = true;
// Since we remove the `\n` when splitting rows, we need to add it back here
ends_with_newline |= !is_last_galley && is_last_row_in_galley;
super::PlacedRow {
pos: new_pos,
row: placed_row.row.clone(),
ends_with_newline,
}
super::PlacedRow { pos: new_pos, row }
}));
merged_galley.num_vertices += galley.num_vertices;