Skip to content

Conversation

@RndUsr123
Copy link
Contributor

@RndUsr123 RndUsr123 commented Jan 4, 2026

This is a fix/improvement that makes all kinds of alignments work in TextEdit when a custom LayoutJob with halign is used.

I used the simplest approach possible to avoid unwanted bugs as I wasn't sure what's safe to change, but there's potentially better ways to achieve this. In particular, I'm not sure I fully understand the rationale behind aligning rows in a Galley based on the latter's leftmost border, considering the size is what's ultimately used in widgets (in TextEdit at least).

Regardless, here's a demo of this PR:

text_edit.mp4
  • I have followed the instructions in the PR template

@github-actions
Copy link

github-actions bot commented Jan 4, 2026

Preview available at https://egui-pr-preview.github.io/pr/7831-TextEdit-fix
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

View snapshot changes at kitdiff

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice; thank you! Could you please also add a snapshot test for this? 🙏

See tests/egui_tests/tests/regression_tests.rs for a template to follow (put yours in tests/egui_tests/tests/test_text_edit.rs)

@emilk emilk added bug Something is broken egui labels Jan 5, 2026
@RndUsr123
Copy link
Contributor Author

RndUsr123 commented Jan 5, 2026

That could be tricky because there's no way to determine where the text has been rendered after the fact from what I can gather. The main issue stems from the fact TextEdit relies on the Galley's rect size rather than its alignment; I'm a bit wary of changing how Galley is computed as that could introduce other bugs so I just visually shifted the text instead.
Here's what I mean for reference (the boxes show the Galley rects):
comparison

I've also just found another bug in LayoutJob which prevents the cursor from moving forward after a trailing space when halign is RIGHT or Center, so that would probably need to be fixed before a proper test about cursor position can be written. Should I open another issue for it or should it be part of this PR?

Edit: this is what it looks like (it was a thing before my fix)

demo.mp4

@emilk
Copy link
Owner

emilk commented Jan 5, 2026

You don't need to test text editing (unless you want to). Just adding your nice 3x3 matrix and snapshot-testing that would be great. See for instance this:

#[test]
fn text_edit_rtl() {
let mut text = "hello ".to_owned();
let mut harness = Harness::builder().with_size((200.0, 50.0)).build_ui(|ui| {
ui.with_layout(Layout::right_to_left(Align::Min), |ui| {
_ = ui.button("right");
ui.add(
egui::TextEdit::singleline(&mut text)
.desired_width(10.0)
.clip_text(false),
);
_ = ui.button("left");
});
});
harness.get_by_role(Role::TextInput).focus();
harness.step();
harness.snapshot("text_edit_rtl_0");
harness.get_by_role(Role::TextInput).type_text("world");
for i in 1..3 {
harness.step();
harness.snapshot(format!("text_edit_rtl_{i}"));
}
}

@RndUsr123
Copy link
Contributor Author

Oh I see... sure I'll do it!

Btw I think I've found out how to fix the LayoutJob issue: it was trimming them out even when the row didn't need to be justified so changing that apparently fixed the problem and justify still works:
fix2

This is what I'm going for:

let [original_min_x, original_max_x];

if justify {
    original_min_x = row.glyphs[glyph_range[0]].logical_rect().min.x;
    original_max_x = row.glyphs[glyph_range[1] - 1].logical_rect().max.x;
}
else {
    original_min_x = row.glyphs[0].logical_rect().min.x;
    original_max_x = row.glyphs[row.glyphs.len() - 1].logical_rect().max.x;
}

let original_width = original_max_x - original_min_x;

would you prefer a direct assignment or something else style-wise?

@RndUsr123
Copy link
Contributor Author

I've added the regression test! Ended up settling for a single snapshot, which should be good enough to showcase both visual alignment in a few scenarios as well as correct cursor placement.
The LayoutJob whitespace trim situation turned out to be not as trivial so I'll open a new issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is broken egui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants