WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit e861c8e

Browse files
authored
Avoid cloning Rows during Galley::concat (#7649)
<!-- 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 #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 :))
1 parent 2669344 commit e861c8e

File tree

4 files changed

+35
-29
lines changed

4 files changed

+35
-29
lines changed

crates/egui/src/text_selection/visuals.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ pub fn paint_text_selection(
3131
let max = galley.layout_from_cursor(max);
3232

3333
for ri in min.row..=max.row {
34-
let row = Arc::make_mut(&mut galley.rows[ri].row);
34+
let placed_row = &mut galley.rows[ri];
35+
let row = Arc::make_mut(&mut placed_row.row);
3536

3637
let left = if ri == min.row {
3738
row.x_offset(min.column)
@@ -41,7 +42,7 @@ pub fn paint_text_selection(
4142
let right = if ri == max.row {
4243
row.x_offset(max.column)
4344
} else {
44-
let newline_size = if row.ends_with_newline {
45+
let newline_size = if placed_row.ends_with_newline {
4546
row.height() / 2.0 // visualize that we select the newline
4647
} else {
4748
0.0

crates/epaint/src/shapes/text_shape.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,19 @@ impl TextShape {
137137
*mesh_bounds = transform.scaling * *mesh_bounds;
138138
*intrinsic_size = transform.scaling * *intrinsic_size;
139139

140-
for text::PlacedRow { pos, row } in rows {
140+
for text::PlacedRow {
141+
pos,
142+
row,
143+
ends_with_newline: _,
144+
} in rows
145+
{
141146
*pos *= transform.scaling;
142147

143148
let text::Row {
144149
section_index_at_start: _,
145150
glyphs: _, // TODO(emilk): would it make sense to transform these?
146151
size,
147152
visuals,
148-
ends_with_newline: _,
149153
} = Arc::make_mut(row);
150154

151155
*size *= transform.scaling;

crates/epaint/src/text/text_layout.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,8 @@ fn rows_from_paragraphs(
296296
glyphs: vec![],
297297
visuals: Default::default(),
298298
size: vec2(0.0, paragraph.empty_paragraph_height),
299-
ends_with_newline: !is_last_paragraph,
300299
}),
300+
ends_with_newline: !is_last_paragraph,
301301
});
302302
} else {
303303
let paragraph_max_x = paragraph.glyphs.last().unwrap().max_x();
@@ -310,14 +310,13 @@ fn rows_from_paragraphs(
310310
glyphs: paragraph.glyphs,
311311
visuals: Default::default(),
312312
size: vec2(paragraph_max_x, 0.0),
313-
ends_with_newline: !is_last_paragraph,
314313
}),
314+
ends_with_newline: !is_last_paragraph,
315315
});
316316
} else {
317317
line_break(&paragraph, job, &mut rows, elided);
318318
let placed_row = rows.last_mut().unwrap();
319-
let row = Arc::make_mut(&mut placed_row.row);
320-
row.ends_with_newline = !is_last_paragraph;
319+
placed_row.ends_with_newline = !is_last_paragraph;
321320
}
322321
}
323322
}
@@ -363,8 +362,8 @@ fn line_break(
363362
glyphs: vec![],
364363
visuals: Default::default(),
365364
size: Vec2::ZERO,
366-
ends_with_newline: false,
367365
}),
366+
ends_with_newline: false,
368367
});
369368
row_start_x += first_row_indentation;
370369
first_row_indentation = 0.0;
@@ -389,8 +388,8 @@ fn line_break(
389388
glyphs,
390389
visuals: Default::default(),
391390
size: vec2(paragraph_max_x, 0.0),
392-
ends_with_newline: false,
393391
}),
392+
ends_with_newline: false,
394393
});
395394

396395
// Start a new row:
@@ -431,8 +430,8 @@ fn line_break(
431430
glyphs,
432431
visuals: Default::default(),
433432
size: vec2(paragraph_max_x - paragraph_min_x, 0.0),
434-
ends_with_newline: false,
435433
}),
434+
ends_with_newline: false,
436435
});
437436
}
438437
}

crates/epaint/src/text/text_layout_types.rs

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,13 @@ pub struct PlacedRow {
572572

573573
/// The underlying unpositioned [`Row`].
574574
pub row: Arc<Row>,
575+
576+
/// If true, this [`PlacedRow`] came from a paragraph ending with a `\n`.
577+
/// The `\n` itself is omitted from row's [`Row::glyphs`].
578+
/// A `\n` in the input text always creates a new [`PlacedRow`] below it,
579+
/// so that text that ends with `\n` has an empty [`PlacedRow`] last.
580+
/// This also implies that the last [`PlacedRow`] in a [`Galley`] always has `ends_with_newline == false`.
581+
pub ends_with_newline: bool,
575582
}
576583

577584
impl PlacedRow {
@@ -617,13 +624,6 @@ pub struct Row {
617624

618625
/// The mesh, ready to be rendered.
619626
pub visuals: RowVisuals,
620-
621-
/// If true, this [`Row`] came from a paragraph ending with a `\n`.
622-
/// The `\n` itself is omitted from [`Self::glyphs`].
623-
/// A `\n` in the input text always creates a new [`Row`] below it,
624-
/// so that text that ends with `\n` has an empty [`Row`] last.
625-
/// This also implies that the last [`Row`] in a [`Galley`] always has `ends_with_newline == false`.
626-
pub ends_with_newline: bool,
627627
}
628628

629629
/// The tessellated output of a row.
@@ -735,12 +735,6 @@ impl Row {
735735
self.glyphs.len()
736736
}
737737

738-
/// Includes the implicit `\n` after the [`Row`], if any.
739-
#[inline]
740-
pub fn char_count_including_newline(&self) -> usize {
741-
self.glyphs.len() + (self.ends_with_newline as usize)
742-
}
743-
744738
/// Closest char at the desired x coordinate in row-relative coordinates.
745739
/// Returns something in the range `[0, char_count_excluding_newline()]`.
746740
pub fn char_at(&self, desired_x: f32) -> usize {
@@ -776,6 +770,12 @@ impl PlacedRow {
776770
pub fn max_y(&self) -> f32 {
777771
self.rect().bottom()
778772
}
773+
774+
/// Includes the implicit `\n` after the [`PlacedRow`], if any.
775+
#[inline]
776+
pub fn char_count_including_newline(&self) -> usize {
777+
self.row.glyphs.len() + (self.ends_with_newline as usize)
778+
}
779779
}
780780

781781
impl Galley {
@@ -867,13 +867,15 @@ impl Galley {
867867
placed_row.visuals.mesh_bounds.translate(new_pos.to_vec2());
868868
merged_galley.rect |= Rect::from_min_size(new_pos, placed_row.size);
869869

870-
let mut row = placed_row.row.clone();
870+
let mut ends_with_newline = placed_row.ends_with_newline;
871871
let is_last_row_in_galley = row_idx + 1 == galley.rows.len();
872-
if !is_last_galley && is_last_row_in_galley {
873-
// Since we remove the `\n` when splitting rows, we need to add it back here
874-
Arc::make_mut(&mut row).ends_with_newline = true;
872+
// Since we remove the `\n` when splitting rows, we need to add it back here
873+
ends_with_newline |= !is_last_galley && is_last_row_in_galley;
874+
super::PlacedRow {
875+
pos: new_pos,
876+
row: placed_row.row.clone(),
877+
ends_with_newline,
875878
}
876-
super::PlacedRow { pos: new_pos, row }
877879
}));
878880

879881
merged_galley.num_vertices += galley.num_vertices;

0 commit comments

Comments
 (0)