Fix text wrapping too late

crates/egui/src/layout.rs

@@ -704,35 +704,37 @@ impl Layout {
         item_spacing: Vec2,
     ) {
         egui_assert!(!cursor.any_nan());
+        let mut newline = false;
         if self.main_wrap {
             if cursor.intersects(frame_rect.shrink(1.0)) {
                 // make row/column larger if necessary
                 *cursor = cursor.union(frame_rect);
             } else {
-                // this is a new row or column. We temporarily use NAN for what will be filled in later.
+                // this is a new row or column. the cursor moves to the edge of the frame
+                newline = true;
                 match self.main_dir {
                     Direction::LeftToRight => {
                         *cursor = Rect::from_min_max(
-                            pos2(f32::NAN, frame_rect.min.y),
+                            pos2(widget_rect.max.x, frame_rect.min.y),
                             pos2(INFINITY, frame_rect.max.y),
                         );
                     }
                     Direction::RightToLeft => {
                         *cursor = Rect::from_min_max(
                             pos2(-INFINITY, frame_rect.min.y),
-                            pos2(f32::NAN, frame_rect.max.y),
+                            pos2(widget_rect.min.x, frame_rect.max.y),
                         );
                     }
                     Direction::TopDown => {
                         *cursor = Rect::from_min_max(
-                            pos2(frame_rect.min.x, f32::NAN),
+                            pos2(frame_rect.min.x, widget_rect.max.y),
                             pos2(frame_rect.max.x, INFINITY),
                         );
                     }
                     Direction::BottomUp => {
                         *cursor = Rect::from_min_max(
                             pos2(frame_rect.min.x, -INFINITY),
-                            pos2(frame_rect.max.x, f32::NAN),
+                            pos2(frame_rect.max.x, widget_rect.min.y),
                         );
                     }
                 };
@@ -748,20 +750,23 @@ impl Layout {
             }
         }
 
-        match self.main_dir {
-            Direction::LeftToRight => {
-                cursor.min.x = widget_rect.max.x + item_spacing.x;
-            }
-            Direction::RightToLeft => {
-                cursor.max.x = widget_rect.min.x - item_spacing.x;
-            }
-            Direction::TopDown => {
-                cursor.min.y = widget_rect.max.y + item_spacing.y;
-            }
-            Direction::BottomUp => {
-                cursor.max.y = widget_rect.min.y - item_spacing.y;
-            }
-        };
+        // apply item_spacing unless this is a newline
+        if !newline {
+            match self.main_dir {
+                Direction::LeftToRight => {
+                    cursor.min.x = widget_rect.max.x + item_spacing.x;
+                }
+                Direction::RightToLeft => {
+                    cursor.max.x = widget_rect.min.x - item_spacing.x;
+                }
+                Direction::TopDown => {
+                    cursor.min.y = widget_rect.max.y + item_spacing.y;
+                }
+                Direction::BottomUp => {
+                    cursor.max.y = widget_rect.min.y - item_spacing.y;
+                }
+            };
+        }
     }
 
     /// Move to the next row in a wrapping layout.

crates/epaint/src/text/text_layout.rs

@@ -147,6 +147,9 @@ fn layout_section(
         paragraph.empty_paragraph_height = line_height; // TODO(emilk): replace this hack with actually including `\n` in the glyphs?
     }
 
+    // TODO(bu5hm4nn): in a label widget, `leading_space` is used to adjust for existing text in a screen row,
+    // but the comment on `LayoutSection::leading_space` makes it clear it was originally intended for typographical
+    // indentation and not for screen layout
     paragraph.cursor_x += leading_space;
 
     let mut last_glyph_id = None;
@@ -244,34 +247,20 @@ fn line_break(paragraph: &Paragraph, job: &LayoutJob, out_rows: &mut Vec<Row>, e
     let mut first_row_indentation = paragraph.glyphs[0].pos.x;
     let mut row_start_x = 0.0;
     let mut row_start_idx = 0;
+    let mut non_empty_rows = 0;
 
     for i in 0..paragraph.glyphs.len() {
-        if job.wrap.max_rows <= out_rows.len() {
-            *elided = true;
+        let potential_row_width = paragraph.glyphs[i].max_x() - row_start_x - first_row_indentation;
+
+        if job.wrap.max_rows > 0 && non_empty_rows >= job.wrap.max_rows {
             break;
         }
 
-        let potential_row_width = paragraph.glyphs[i].max_x() - row_start_x;
-
-        if job.wrap.max_width < potential_row_width {
-            // Row break:
-
-            if first_row_indentation > 0.0
-                && !row_break_candidates.has_good_candidate(job.wrap.break_anywhere)
-            {
-                // Allow the first row to be completely empty, because we know there will be more space on the next row:
-                // TODO(emilk): this records the height of this first row as zero, though that is probably fine since first_row_indentation usually comes with a first_row_min_height.
-                out_rows.push(Row {
-                    section_index_at_start: paragraph.section_index_at_start,
-                    glyphs: vec![],
-                    visuals: Default::default(),
-                    rect: rect_from_x_range(first_row_indentation..=first_row_indentation),
-                    ends_with_newline: false,
-                });
-                row_start_x += first_row_indentation;
-                first_row_indentation = 0.0;
-            } else if let Some(last_kept_index) = row_break_candidates.get(job.wrap.break_anywhere)
-            {
+        // (bu5hm4nn): we want to actually allow as much text as possible on the first line so
+        // we don't need a special case for the first row, but we need to subtract
+        // the first_row_indentation from the allowed max width
+        if potential_row_width > (job.wrap.max_width - first_row_indentation) {
+            if let Some(last_kept_index) = row_break_candidates.get(job.wrap.break_anywhere) {
                 let glyphs: Vec<Glyph> = paragraph.glyphs[row_start_idx..=last_kept_index]
                     .iter()
                     .copied()
@@ -297,6 +286,12 @@ fn line_break(paragraph: &Paragraph, job: &LayoutJob, out_rows: &mut Vec<Row>, e
                 row_start_idx = last_kept_index + 1;
                 row_start_x = paragraph.glyphs[row_start_idx].pos.x;
                 row_break_candidates = Default::default();
+                non_empty_rows += 1;
+
+                // (bu5hm4nn) first row indentation gets consumed the first time it's used
+                if first_row_indentation > 0.0 {
+                    first_row_indentation = 0.0;
+                }
             } else {
                 // Found no place to break, so we have to overrun wrap_width.
             }
@@ -925,6 +920,7 @@ impl RowBreakCandidates {
             .flatten()
     }
 
+    #[allow(dead_code)]
     fn has_good_candidate(&self, break_anywhere: bool) -> bool {
         if break_anywhere {
             self.any.is_some()
@@ -1062,3 +1058,36 @@ mod tests {
         );
     }
 }
+
+#[test]
+fn test_line_break_first_row_not_empty() {
+    let mut fonts = FontsImpl::new(1.0, 1024, super::FontDefinitions::default());
+    let mut layout_job = LayoutJob::single_section(
+        "SomeSuperLongTextThatDoesNotHaveAnyGoodBreakCandidatesButStillNeedsToBeBroken".into(),
+        super::TextFormat::default(),
+    );
+
+    // a small area
+    layout_job.wrap.max_width = 110.0;
+
+    // give the first row a leading space, simulating that there already is
+    // text in this visual row
+    layout_job.sections.first_mut().unwrap().leading_space = 50.0;
+
+    let galley = super::layout(&mut fonts, layout_job.into());
+    assert_eq!(
+        galley
+            .rows
+            .iter()
+            .map(|row| row.glyphs.iter().map(|g| g.chr).collect::<String>())
+            .collect::<Vec<_>>(),
+        vec![
+            "SomeSup",
+            "erLongTextThat",
+            "DoesNotHaveAn",
+            "yGoodBreakCand",
+            "idatesButStillNe",
+            "edsToBeBroken"
+        ]
+    );
+}

examples/wrapping-layout/Cargo.toml

@@ -0,0 +1,23 @@
+[package]
+name = "wrapping_layout"
+version = "0.1.0"
+authors = ["Emil Ernerfeldt <[email protected]>", "Bu5hm4nn <[email protected]>"]
+license = "MIT OR Apache-2.0"
+edition = "2021"
+rust-version = "1.72"
+publish = false
+
+
+[dependencies]
+eframe = { workspace = true, features = [
+    "default",
+    "__screenshot", # __screenshot is so we can dump a screenshot using EFRAME_SCREENSHOT_TO
+] }
+
+# For image support:
+egui_extras = { workspace = true, features = ["default", "image"] }
+
+env_logger = { version = "0.10", default-features = false, features = [
+    "auto-color",
+    "humantime",
+] }

examples/wrapping-layout/README.md

@@ -0,0 +1,7 @@
+Example showing text wrapping with `ui.horizontal_wrapped()`
+
+```sh
+cargo run -p wrapping_layout
+```
+
+![](screenshot.png)

examples/wrapping-layout/src/main.rs

@@ -0,0 +1,55 @@
+use eframe::{
+    egui::{self, WidgetText},
+    emath::Align,
+    epaint::Stroke,
+};
+
+fn main() -> Result<(), eframe::Error> {
+    env_logger::init(); // Log to stderr (if you run with `RUST_LOG=debug`).
+    let options = eframe::NativeOptions {
+        viewport: egui::ViewportBuilder::default().with_inner_size([320.0, 240.0]),
+        ..Default::default()
+    };
+    eframe::run_native(
+        "Horizontal Wrapped Layouts",
+        options,
+        Box::new(|cc| Box::new(MyEguiApp::new(cc))),
+    )
+}
+
+#[derive(Default)]
+struct MyEguiApp {}
+
+impl MyEguiApp {
+    fn new(_cc: &eframe::CreationContext<'_>) -> Self {
+        Self::default()
+    }
+}
+
+impl eframe::App for MyEguiApp {
+    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
+        egui::CentralPanel::default().show(ctx, |ui| {
+            ui.horizontal_wrapped(|ui| {
+                ui.hyperlink_to("@npub1vdaeclr2mnntmyw...", "whocares");
+                let text = " LotsOfTextPrecededByASpace5kgqfqqxwhkrkw60stn8aph4gm2h2053xvwvvlvjm3q9eqdpqxycrqvpqd3hhgar9wfujqarfvd4k2arncqzpgxqzz6sp5vfenc5l4uafsky0w069zs329edf608ggpjjveguwxfl3xlswg5vq9qyyssqj46d5x3gsnljffm79eqwszk4mk47lkxywdp8mxum7un3qm0ztwj9jf46cm4lw2un9hk4gttgtjdrk29h27xu4e3ume20sqsna8q7xwspqqkwq7";
+                ui.label(text);
+                ui.style_mut().visuals.widgets.noninteractive.fg_stroke = Stroke::new( 1.0, eframe::epaint::Color32::RED );
+                ui.label("More text followed by two newlines\n\n");
+                ui.style_mut().visuals.widgets.noninteractive.fg_stroke = Stroke::new( 1.0, eframe::epaint::Color32::GREEN );
+                ui.label("more text, no newline");
+                ui.reset_style();
+            });
+            ui.separator();
+            ui.horizontal_wrapped(|ui| {
+                ui.label("Hyperlink no newline:");
+                let url = "https://i.nostrimg.com/c72f5e1a2e162fad2625e15651a654465c06016016f7743b496021cafa2a524e/file.jpeg";
+                ui.hyperlink_to( url, url );
+                ui.end_row();
+                ui.label("Hyperlink break_anywhere=true");
+                let mut job = WidgetText::from(url).into_layout_job(ui.style(), egui::FontSelection::Default, Align::LEFT);
+                job.wrap.break_anywhere = true;
+                ui.hyperlink_to( job, url );
+            });
+       });
+    }
+}