Fix issue emilk#2578 - row_start_x tracks a virtual position in the…

… source paragraph (the one that is too long) for which length has already been processed. When creating an empty row, this position should not be updated as no glyphs were consumed from the source paragraph. - added example that would demonstrate the problem if the line was included, and that is fixed with this commit

Fix issue emilk#2578 There was confusion in the code over how to
break when on a non-empty visual row (`first_row_indentation > 0.0`), causing
text to be shifted left outside the ui frame. This is the case for example
when another label has already been placed in this `ui.horizontal_wrapped()`.
This fix will not create an empty row, essentially starting a newline, but
rather try to fit as much text as possible on the existing row. IMO this is
the desired use of a wrapping layout. I've also added an example that would
demonstrate the problem if the line was included, and that is fixed with this
commit

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 );
+            });
+       });
+    }
+}