Fix issue emilk#2578 There was confusion in the code over how to brea…

…k 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 commi

crates/epaint/src/text/text_layout.rs

@@ -103,6 +103,9 @@ fn layout_section(
         paragraph.empty_paragraph_height = font_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;
@@ -198,27 +201,17 @@ fn line_break(
     let mut non_empty_rows = 0;
 
     for i in 0..paragraph.glyphs.len() {
-        let potential_row_width = paragraph.glyphs[i].max_x() - row_start_x;
+        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;
         }
 
-        if potential_row_width > job.wrap.max_width {
-            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 {
-                    glyphs: vec![],
-                    visuals: Default::default(),
-                    rect: rect_from_x_range(first_row_indentation..=first_row_indentation),
-                    ends_with_newline: false,
-                });
-                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()
@@ -242,6 +235,11 @@ fn line_break(
                 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.
             }
@@ -762,6 +760,7 @@ impl RowBreakCandidates {
             .flatten()
     }
 
+    #[allow(dead_code)]
     fn has_good_candidate(&self, break_anywhere: bool) -> bool {
         if break_anywhere {
             self.any.is_some()
@@ -855,3 +854,36 @@ fn test_pre_cjk() {
         vec!["日本語とEnglish", "の混在した文章"]
     );
 }
+
+#[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/src/main.rs

@@ -1,13 +1,17 @@
 use eframe::{
-    egui::{self, TextFormat},
-    epaint::text::LayoutJob,
+    egui::{self, WidgetText},
+    emath::Align,
+    epaint::Stroke,
 };
 
 fn main() -> Result<(), eframe::Error> {
-    let native_options = eframe::NativeOptions::default();
+    let options = eframe::NativeOptions {
+        initial_window_size: Some(egui::vec2(380.0, 440.0)),
+        ..Default::default()
+    };
     eframe::run_native(
-        "My egui App",
-        native_options,
+        "Horizontal Wrapped Layouts",
+        options,
         Box::new(|cc| Box::new(MyEguiApp::new(cc))),
     )
 }
@@ -24,12 +28,27 @@ impl MyEguiApp {
 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.horizontal_wrapped(|ui| {
                 ui.hyperlink_to("@npub1vdaeclr2mnntmyw...", "whocares");
-                let text = " lnbc10u1p3lz4dppp5dsj2mh5kgqfqqxwhkrkw60stn8aph4gm2h2053xvwvvlvjm3q9eqdpqxycrqvpqd3hhgar9wfujqarfvd4k2arncqzpgxqzz6sp5vfenc5l4uafsky0w069zs329edf608ggpjjveguwxfl3xlswg5vq9qyyssqj46d5x3gsnljffm79eqwszk4mk47lkxywdp8mxum7un3qm0ztwj9jf46cm4lw2un9hk4gttgtjdrk29h27xu4e3ume20sqsna8q7xwspqqkwq7";
-                let job = LayoutJob::single_section(text.to_owned(), TextFormat::default());
-                ui.label(job);
-           });
+                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_text_job(ui.style(), egui::FontSelection::Default, Align::LEFT);
+                job.job.wrap.break_anywhere = true;
+                ui.hyperlink_to( job.job, url );
+            });
        });
     }
 }