Skip to content

Commit

Permalink
Fix issue #2578
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Bu5hm4nn committed Mar 9, 2023
1 parent f94187a commit 462f6db
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 17 deletions.
8 changes: 8 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

65 changes: 48 additions & 17 deletions crates/epaint/src/text/text_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -198,28 +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,
});
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()
Expand All @@ -243,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.
}
Expand Down Expand Up @@ -763,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()
Expand Down Expand Up @@ -856,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"
]
);
}
12 changes: 12 additions & 0 deletions examples/wrapping-layout/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "wrapping-layout"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
eframe = { path = "../../crates/eframe", features = [
"__screenshot", # __screenshot is so we can dump a screenshot using EFRAME_SCREENSHOT_TO
] }
tracing-subscriber = "0.3"
53 changes: 53 additions & 0 deletions examples/wrapping-layout/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use eframe::{
egui::{self, WidgetText},
emath::Align,
epaint::Stroke,
};

fn main() -> Result<(), eframe::Error> {
let options = eframe::NativeOptions {
initial_window_size: Some(egui::vec2(380.0, 440.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.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 );
});
});
}
}

0 comments on commit 462f6db

Please sign in to comment.