Exclude \n when splitting Galleys

crates/egui/src/atomics/atom_kind.rs

@@ -84,7 +84,7 @@ impl<'a> AtomKind<'a> {
                 let wrap_mode = wrap_mode.unwrap_or(ui.wrap_mode());
                 let galley =
                     text.into_galley(ui, Some(wrap_mode), available_size.x, TextStyle::Button);
-                (galley.intrinsic_size, SizedAtomKind::Text(galley))
+                (galley.intrinsic_size(), SizedAtomKind::Text(galley))
             }
             AtomKind::Image(image) => {
                 let size = image.load_and_calc_size(ui, available_size);

crates/epaint/src/text/fonts.rs

@@ -825,7 +825,7 @@ impl GalleyCache {
                 let job = Arc::new(job);
                 if allow_split_paragraphs && should_cache_each_paragraph_individually(&job) {
                     let (child_galleys, child_hashes) =
-                        self.layout_each_paragraph_individuallly(fonts, &job);
+                        self.layout_each_paragraph_individually(fonts, &job);
                     debug_assert_eq!(
                         child_hashes.len(),
                         child_galleys.len(),
@@ -869,7 +869,7 @@ impl GalleyCache {
     }
 
     /// Split on `\n` and lay out (and cache) each paragraph individually.
-    fn layout_each_paragraph_individuallly(
+    fn layout_each_paragraph_individually(
         &mut self,
         fonts: &mut FontsImpl,
         job: &LayoutJob,
@@ -884,9 +884,11 @@ impl GalleyCache {
 
         while start < job.text.len() {
             let is_first_paragraph = start == 0;
+            // `end` will not include the `\n` since we don't want to create an empty row in our
+            // split galley
             let end = job.text[start..]
                 .find('\n')
-                .map_or(job.text.len(), |i| start + i + 1);
+                .map_or(job.text.len(), |i| start + i);
 
             let mut paragraph_job = LayoutJob {
                 text: job.text[start..end].to_owned(),
@@ -920,7 +922,7 @@ impl GalleyCache {
                 if section_range.end <= start {
                     // The section is behind us
                     current_section += 1;
-                } else if end <= section_range.start {
+                } else if end < section_range.start {
                     break; // Haven't reached this one yet.
                 } else {
                     // Section range overlaps with paragraph range
@@ -953,10 +955,6 @@ impl GalleyCache {
             // This will prevent us from invalidating cache entries unnecessarily:
             if max_rows_remaining != usize::MAX {
                 max_rows_remaining -= galley.rows.len();
-                // Ignore extra trailing row, see merging `Galley::concat` for more details.
-                if end < job.text.len() && !galley.elided {
-                    max_rows_remaining += 1;
-                }
             }
 
             let elided = galley.elided;
@@ -965,7 +963,7 @@ impl GalleyCache {
                 break;
             }
 
-            start = end;
+            start = end + 1;
         }
 
         (child_galleys, child_hashes)
@@ -1091,6 +1089,29 @@ mod tests {
                 Color32::WHITE,
                 f32::INFINITY,
             ),
+            {
+                let mut job = LayoutJob::simple(
+                    "hi".to_owned(),
+                    FontId::default(),
+                    Color32::WHITE,
+                    f32::INFINITY,
+                );
+                job.append("\n", 0.0, TextFormat::default());
+                job.append("\n", 0.0, TextFormat::default());
+                job.append("world", 0.0, TextFormat::default());
+                job.wrap.max_rows = 2;
+                job
+            },
+            {
+                let mut job = LayoutJob::simple(
+                    "Test text with a lot of words\n and a newline.".to_owned(),
+                    FontId::new(14.0, FontFamily::Monospace),
+                    Color32::WHITE,
+                    40.0,
+                );
+                job.first_row_min_height = 30.0;
+                job
+            },
             LayoutJob::simple(
                 "This some text that may be long.\nDet kanske också finns lite ÅÄÖ här.".to_owned(),
                 FontId::new(14.0, FontFamily::Proportional),
@@ -1213,7 +1234,7 @@ mod tests {
                         let text = job.text.clone();
                         let galley_unwrapped = layout(&mut fonts, job.into());
 
-                        let intrinsic_size = galley_wrapped.intrinsic_size;
+                        let intrinsic_size = galley_wrapped.intrinsic_size();
                         let unwrapped_size = galley_unwrapped.size();
 
                         let difference = (intrinsic_size - unwrapped_size).length().abs();
@@ -1232,7 +1253,7 @@ mod tests {
                             format!("{unwrapped_size:.4?}"),
                             "Unwrapped galley intrinsic size should exactly match its size. \
                                 {:.8?} vs {:8?}",
-                            galley_unwrapped.intrinsic_size,
+                            galley_unwrapped.intrinsic_size(),
                             galley_unwrapped.size(),
                         );
                     }

crates/epaint/src/text/text_layout.rs

@@ -204,20 +204,12 @@ fn calculate_intrinsic_size(
 ) -> Vec2 {
     let mut intrinsic_size = Vec2::ZERO;
     for (idx, paragraph) in paragraphs.iter().enumerate() {
-        if paragraph.glyphs.is_empty() {
-            if idx == 0 {
-                intrinsic_size.y += point_scale.round_to_pixel(paragraph.empty_paragraph_height);
-            }
-            continue;
-        }
-        intrinsic_size.x = f32::max(
-            paragraph
-                .glyphs
-                .last()
-                .map(|l| l.max_x())
-                .unwrap_or_default(),
-            intrinsic_size.x,
-        );
+        let width = paragraph
+            .glyphs
+            .last()
+            .map(|l| l.max_x())
+            .unwrap_or_default();
+        intrinsic_size.x = f32::max(intrinsic_size.x, width);
 
         let mut height = paragraph
             .glyphs
@@ -253,7 +245,7 @@ fn rows_from_paragraphs(
 
         if paragraph.glyphs.is_empty() {
             rows.push(PlacedRow {
-                pos: Pos2::ZERO,
+                pos: pos2(0.0, f32::NAN),
                 row: Arc::new(Row {
                     section_index_at_start: paragraph.section_index_at_start,
                     glyphs: vec![],
@@ -659,12 +651,12 @@ fn galley_from_rows(
     let mut cursor_y = 0.0;
 
     for placed_row in &mut rows {
-        let mut max_row_height = first_row_min_height.max(placed_row.rect().height());
+        let mut max_row_height = first_row_min_height.at_least(placed_row.height());
         let row = Arc::make_mut(&mut placed_row.row);
 
         first_row_min_height = 0.0;
         for glyph in &row.glyphs {
-            max_row_height = max_row_height.max(glyph.line_height);
+            max_row_height = max_row_height.at_least(glyph.line_height);
         }
         max_row_height = point_scale.round_to_pixel(max_row_height);
 
@@ -1212,4 +1204,72 @@ mod tests {
         assert_eq!(row.pos, Pos2::ZERO);
         assert_eq!(row.rect().max.x, row.glyphs.last().unwrap().max_x());
     }
+
+    #[test]
+    fn test_empty_row() {
+        let mut fonts = FontsImpl::new(
+            1.0,
+            1024,
+            AlphaFromCoverage::default(),
+            FontDefinitions::default(),
+        );
+
+        let font_id = FontId::default();
+        let font_height = fonts.font(&font_id).row_height();
+
+        let job = LayoutJob::simple(String::new(), font_id, Color32::WHITE, f32::INFINITY);
+
+        let galley = layout(&mut fonts, job.into());
+
+        assert_eq!(galley.rows.len(), 1, "Expected one row");
+        assert_eq!(
+            galley.rows[0].row.glyphs.len(),
+            0,
+            "Expected no glyphs in the empty row"
+        );
+        assert_eq!(
+            galley.size(),
+            Vec2::new(0.0, font_height.round()),
+            "Unexpected galley size"
+        );
+        assert_eq!(
+            galley.intrinsic_size(),
+            Vec2::new(0.0, font_height.round()),
+            "Unexpected intrinsic size"
+        );
+    }
+
+    #[test]
+    fn test_end_with_newline() {
+        let mut fonts = FontsImpl::new(
+            1.0,
+            1024,
+            AlphaFromCoverage::default(),
+            FontDefinitions::default(),
+        );
+
+        let font_id = FontId::default();
+        let font_height = fonts.font(&font_id).row_height();
+
+        let job = LayoutJob::simple("Hi!\n".to_owned(), font_id, Color32::WHITE, f32::INFINITY);
+
+        let galley = layout(&mut fonts, job.into());
+
+        assert_eq!(galley.rows.len(), 2, "Expected two rows");
+        assert_eq!(
+            galley.rows[1].row.glyphs.len(),
+            0,
+            "Expected no glyphs in the empty row"
+        );
+        assert_eq!(
+            galley.size().round(),
+            Vec2::new(17.0, font_height.round() * 2.0),
+            "Unexpected galley size"
+        );
+        assert_eq!(
+            galley.intrinsic_size().round(),
+            Vec2::new(17.0, font_height.round() * 2.0),
+            "Unexpected intrinsic size"
+        );
+    }
 }

crates/epaint/src/text/text_layout_types.rs

@@ -561,11 +561,7 @@ pub struct Galley {
     /// tessellation.
     pub pixels_per_point: f32,
 
-    /// This is the size that a non-wrapped, non-truncated, non-justified version of the text
-    /// would have.
-    ///
-    /// Useful for advanced layouting.
-    pub intrinsic_size: Vec2,
+    pub(crate) intrinsic_size: Vec2,
 }
 
 #[derive(Clone, Debug, PartialEq)]
@@ -801,6 +797,21 @@ impl Galley {
         self.rect.size()
     }
 
+    /// This is the size that a non-wrapped, non-truncated, non-justified version of the text
+    /// would have.
+    ///
+    /// Useful for advanced layouting.
+    #[inline]
+    pub fn intrinsic_size(&self) -> Vec2 {
+        // We do the rounding here instead of in `round_output_to_gui` so that rounding
+        // errors don't accumulate when concatenating multiple galleys.
+        if self.job.round_output_to_gui {
+            self.intrinsic_size.round_ui()
+        } else {
+            self.intrinsic_size
+        }
+    }
+
     pub(crate) fn round_output_to_gui(&mut self) {
         for placed_row in &mut self.rows {
             // Optimization: only call `make_mut` if necessary (can cause a deep clone)
@@ -827,8 +838,6 @@ impl Galley {
                 .at_most(rect.min.x + self.job.wrap.max_width)
                 .floor_ui();
         }
-
-        self.intrinsic_size = self.intrinsic_size.round_ui();
     }
 
     /// Append each galley under the previous one.
@@ -849,32 +858,28 @@ impl Galley {
 
         for (i, galley) in galleys.iter().enumerate() {
             let current_y_offset = merged_galley.rect.height();
-
-            let mut rows = galley.rows.iter();
-            // As documented in `Row::ends_with_newline`, a '\n' will always create a
-            // new `Row` immediately below the current one. Here it doesn't make sense
-            // for us to append this new row so we just ignore it.
-            let is_last_row = i + 1 == galleys.len();
-            if !is_last_row && !galley.elided {
-                let popped = rows.next_back();
-                debug_assert_eq!(popped.unwrap().row.glyphs.len(), 0, "Bug in Galley::concat");
-            }
-
-            merged_galley.rows.extend(rows.map(|placed_row| {
-                let new_pos = placed_row.pos + current_y_offset * Vec2::Y;
-                let new_pos = new_pos.round_to_pixels(pixels_per_point);
-                merged_galley.mesh_bounds = merged_galley
-                    .mesh_bounds
-                    .union(placed_row.visuals.mesh_bounds.translate(new_pos.to_vec2()));
-                merged_galley.rect = merged_galley
-                    .rect
-                    .union(Rect::from_min_size(new_pos, placed_row.size));
-
-                super::PlacedRow {
-                    pos: new_pos,
-                    row: placed_row.row.clone(),
-                }
-            }));
+            let is_last_galley = i + 1 == galleys.len();
+
+            merged_galley
+                .rows
+                .extend(galley.rows.iter().enumerate().map(|(row_idx, placed_row)| {
+                    let new_pos = placed_row.pos + current_y_offset * Vec2::Y;
+                    let new_pos = new_pos.round_to_pixels(pixels_per_point);
+                    merged_galley.mesh_bounds = merged_galley
+                        .mesh_bounds
+                        .union(placed_row.visuals.mesh_bounds.translate(new_pos.to_vec2()));
+                    merged_galley.rect = merged_galley
+                        .rect
+                        .union(Rect::from_min_size(new_pos, placed_row.size));
+
+                    let mut row = placed_row.row.clone();
+                    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;
+                    }
+                    super::PlacedRow { pos: new_pos, row }
+                }));
 
             merged_galley.num_vertices += galley.num_vertices;
             merged_galley.num_indices += galley.num_indices;