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

crates/egui/src/widgets/label.rs

@@ -179,6 +179,10 @@ impl Label {
             }
             let galley = ui.fonts(|fonts| fonts.layout_job(layout_job));
 
+            if galley.on_newline {
+                ui.end_row();
+            }
+
             let pos = pos2(ui.max_rect().left(), ui.cursor().top());
             assert!(!galley.rows.is_empty(), "Galleys are never empty");
 

crates/epaint/src/text/text_layout.rs

@@ -80,6 +80,7 @@ pub fn layout(fonts: &mut FontsImpl, job: Arc<LayoutJob>) -> Galley {
             num_vertices: 0,
             num_indices: 0,
             pixels_per_point: fonts.pixels_per_point(),
+            on_newline: false,
             elided: true,
         };
     }
@@ -262,58 +263,65 @@ fn line_break(paragraph: &Paragraph, job: &LayoutJob, out_rows: &mut Vec<Row>, e
             break;
         }
 
-        if job.wrap.max_width + wrap_width_margin < 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)
-            {
-                let glyphs: Vec<Glyph> = paragraph.glyphs[row_start_idx..=last_kept_index]
-                    .iter()
-                    .copied()
-                    .map(|mut glyph| {
-                        glyph.pos.x -= row_start_x;
-                        glyph
-                    })
-                    .collect();
-
-                let section_index_at_start = glyphs[0].section_index;
-                let paragraph_min_x = glyphs[0].pos.x;
-                let paragraph_max_x = glyphs.last().unwrap().max_x();
-
-                out_rows.push(Row {
-                    section_index_at_start,
-                    glyphs,
-                    visuals: Default::default(),
-                    rect: rect_from_x_range(paragraph_min_x..=paragraph_max_x),
-                    ends_with_newline: false,
-                });
-
-                // Start a new row:
-                row_start_idx = last_kept_index + 1;
-                row_start_x = paragraph.glyphs[row_start_idx].pos.x;
-                row_break_candidates.forget_before_idx(row_start_idx);
-                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;
+        // (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 + wrap_width_margin - first_row_indentation) {
+            match row_break_candidates.get(job.wrap.break_anywhere) {
+                None => {}
+                Some(0) => {
+                    // if the best break candidate is the first char, add a newline
+                    // immediately before proceeding with any chars
+                    out_rows.push(Row {
+                        section_index_at_start: 0,
+                        glyphs: Vec::new(),
+                        rect: Rect::ZERO,
+                        visuals: Default::default(),
+                        ends_with_newline: false,
+                    });
+
+                    // Start a new row:
+                    row_start_x = paragraph.glyphs[row_start_idx].pos.x;
+                    row_break_candidates.forget_before_idx(1);
+
+                    // (bu5hm4nn) first row indentation gets consumed the first time it's used
+                    if first_row_indentation > 0.0 {
+                        first_row_indentation = 0.0;
+                    }
+                }
+                Some(last_kept_index) => {
+                    let glyphs: Vec<Glyph> = paragraph.glyphs[row_start_idx..=last_kept_index]
+                        .iter()
+                        .copied()
+                        .map(|mut glyph| {
+                            glyph.pos.x -= row_start_x;
+                            glyph
+                        })
+                        .collect();
+
+                    let section_index_at_start = glyphs[0].section_index;
+                    let paragraph_min_x = glyphs[0].pos.x;
+                    let paragraph_max_x = glyphs.last().unwrap().max_x();
+
+                    out_rows.push(Row {
+                        section_index_at_start,
+                        glyphs,
+                        visuals: Default::default(),
+                        rect: rect_from_x_range(paragraph_min_x..=paragraph_max_x),
+                        ends_with_newline: false,
+                    });
+
+                    // Start a new row:
+                    row_start_idx = last_kept_index + 1;
+                    row_start_x = paragraph.glyphs[row_start_idx].pos.x;
+                    row_break_candidates.forget_before_idx(row_start_idx);
+                    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.
             }
         }
 
@@ -590,10 +598,23 @@ fn galley_from_rows(
     mut rows: Vec<Row>,
     elided: bool,
 ) -> Galley {
+    let mut on_newline = false;
     let mut first_row_min_height = job.first_row_min_height;
     let mut cursor_y = 0.0;
     let mut min_x: f32 = 0.0;
     let mut max_x: f32 = 0.0;
+
+    // if no chars fit on the current cursor line, then layout will pass a row with
+    // a Rect::ZERO as the first row. We will remove it and set the on_newline flag instead
+    // so that calling code can handle moving the cursor (to preserve spacing, etc.)
+    // EXCEPTION: we will not remove it if it is the only row
+    if let Some(row) = rows.first() {
+        if rows.len() > 1 && row.rect == Rect::ZERO {
+            rows.remove(0);
+            on_newline = true;
+        }
+    }
+
     for row in &mut rows {
         let mut line_height = first_row_min_height.max(row.rect.height());
         let mut row_ascent = 0.0f32;
@@ -667,6 +688,7 @@ fn galley_from_rows(
     Galley {
         job,
         rows,
+        on_newline,
         elided,
         rect,
         mesh_bounds,
@@ -1123,37 +1145,49 @@ mod tests {
             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"
-        ]
-    );
+    #[test]
+    fn test_line_break_first_row_not_empty() {
+        let mut fonts = FontsImpl::new(1.0, 1024, FontDefinitions::default());
+        let mut layout_job = LayoutJob::single_section(
+            "SomeSuperLongTextThatDoesNotHaveAnyGoodBreakCandidatesButStillNeedsToBeBroken".into(),
+            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"
+            ]
+        );
+    }
+
+    // #[test]
+    // fn test_never_exceed_max_width() {}
+
+    #[test]
+    fn test_never_empty_galley() {
+        let mut fonts = FontsImpl::new(1.0, 1024, FontDefinitions::default());
+        let mut layout_job = LayoutJob::single_section("\r".into(), TextFormat::default());
+        layout_job.wrap.max_width = 0.0;
+        let galley = layout(&mut fonts, layout_job.into());
+        assert_eq!(galley.rows.len(), 1);
+    }
 }

crates/epaint/src/text/text_layout_types.rs

@@ -487,6 +487,9 @@ pub struct Galley {
     /// can be split up into multiple rows.
     pub rows: Vec<Row>,
 
+    /// Set to true if the whole galley needs to go on a newline
+    pub on_newline: bool,
+
     /// Set to true the text was truncated due to [`TextWrapping::max_rows`].
     pub elided: bool,
 

examples/wrapping-layout/src/main.rs

@@ -1,18 +1,18 @@
 use eframe::{
-    egui::{self, WidgetText},
+    egui::{self, FontSelection, TextWrapMode, ViewportBuilder, WidgetText},
     emath::Align,
     epaint::Stroke,
 };
 
 fn main() -> Result<(), eframe::Error> {
     let options = eframe::NativeOptions {
-        initial_window_size: Some(egui::vec2(380.0, 440.0)),
+        viewport: ViewportBuilder::default().with_inner_size(egui::vec2(380.0, 440.0)),
         ..Default::default()
     };
     eframe::run_native(
         "Horizontal Wrapped Layouts",
         options,
-        Box::new(|cc| Box::new(MyEguiApp::new(cc))),
+        Box::new(|cc| Ok(Box::new(MyEguiApp::new(cc)))),
     )
 }
 
@@ -37,6 +37,12 @@ impl eframe::App for MyEguiApp {
                 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.end_row();
+                ui.label(format!(
+                    "Ui Size: w: {}, h: {}",
+                    ui.available_size().x,
+                    ui.available_size().y
+                ));
             });
             ui.separator();
             ui.horizontal_wrapped(|ui| {
@@ -45,10 +51,44 @@ impl eframe::App for MyEguiApp {
                 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 );
+                let mut job = WidgetText::from(url).into_layout_job(ui.style(), egui::FontSelection::Default, Align::LEFT);
+                job.wrap.break_anywhere = true;
+                job.wrap.max_width = ui.available_size().x - ui.available_size_before_wrap().x;
+                ui.hyperlink_to( job, url );
+                ui.end_row();
+                ui.label(format!(
+                    "Ui Size: w: {}, h: {}",
+                    ui.available_size().x,
+                    ui.available_size().y
+                ));
+            });
+            ui.separator();
+            ui.horizontal_wrapped(|ui| {
+                ui.heading("Wrapping individual labels");
+                ui.end_row();
+                for i in 1..50 {
+                    let response = ui.label(format!("Label {i} "));
+                    let rect = response.rect;
+                    response.on_hover_text(format!("Cursor: {}, Label: {}", ui.cursor(), rect));
+                }
+                ui.end_row();
+                ui.label(format!(
+                    "Ui Size: w: {}, h: {}",
+                    ui.available_size().x,
+                    ui.available_size().y
+                ));
             });
        });
+        egui::TopBottomPanel::bottom("footer").show(ctx, |ui| {
+            ui.label(format!(
+                "Screen Size: w: {}, h: {}",
+                ctx.screen_rect().width(),
+                ctx.screen_rect().height()
+            ));
+            let mut debug = ctx.debug_on_hover();
+            if ui.checkbox(&mut debug, "Debug on Hover").changed() {
+                ctx.set_debug_on_hover(debug);
+            }
+        });
     }
 }