Skip to content

editor: Fix soft wrap premature wrapping with certain fonts#45206

Merged
Veykril merged 5 commits intozed-industries:mainfrom
errmayank:soft-wrap-premature
Feb 13, 2026
Merged

editor: Fix soft wrap premature wrapping with certain fonts#45206
Veykril merged 5 commits intozed-industries:mainfrom
errmayank:soft-wrap-premature

Conversation

@errmayank
Copy link
Copy Markdown
Contributor

Closes #45107

Release Notes:

  • Fixed soft wrap prematurely wrapping with certain fonts

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 18, 2025
@github-actions github-actions bot added the community champion Issues filed by our amazing community champions! 🫶 label Dec 18, 2025
@github-project-automation github-project-automation bot moved this to Community Champion PRs in Quality Week – December 2025 Dec 18, 2025
@errmayank
Copy link
Copy Markdown
Contributor Author

Here's before/after:

Screenshot 2025-12-18 at 13 46 39
Example README.md
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut iaculis nisi et non.

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut urna nibh tincidunt.

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer imperdiet quam.

@Veykril Veykril self-assigned this Dec 18, 2025
Copy link
Copy Markdown
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

Thanks!

@Veykril Veykril enabled auto-merge (squash) December 18, 2025 09:03
@Veykril
Copy link
Copy Markdown
Member

Veykril commented Dec 18, 2025

Looks like this causes a test failure

cc @MrSubidubi, seems to be a test added by you, in case you have context here

@MrSubidubi
Copy link
Copy Markdown
Member

The test very much checks what the error says, so if this fails, it is very likely that we actually have minimal scrolling with soft wrap enabled, which we should never have. Test is supposed to catch just that (we regressed there too many times).

So this might change more in the editor than it perhaps should, some calculation there might be different due to this change and would result in scrolling with soft wrap

auto-merge was automatically disabled December 18, 2025 10:37

Head branch was pushed to by a user without write access

@errmayank
Copy link
Copy Markdown
Contributor Author

Thanks @/MrSubidubi. Tests should pass now 🙈

@Veykril
Copy link
Copy Markdown
Member

Veykril commented Dec 18, 2025

Can you give a bit more context on this as to why this fixes the issue now?

@errmayank
Copy link
Copy Markdown
Contributor Author

Sorry, I realized that I shouldn't have updated em_advance(), which has it's own purpose it seems.

I thought I'd make it consistent with LineWrapper::wrap_line() which uses platform_text_system.layout_line() internally for measuring rendered char width used in soft wrap calculation.

Now, I've updated the editor prepaint to use actual rendered char width instead of raw glyph advance width coming from em_advance() which could have slight differences for some fonts (looking at you, Inconsolata for Powerline)

@errmayank errmayank changed the title gpui: Fix soft wrap premature wrapping with certain fonts editor: Fix soft wrap premature wrapping with certain fonts Dec 18, 2025
Comment thread crates/editor/src/element.rs Outdated
Comment on lines +9061 to +9071
let em_width = window.text_system().em_width(font_id, font_size).unwrap();
let em_advance = window.text_system().em_advance(font_id, font_size).unwrap();
let run = TextRun {
len: 1,
font: style.text.font(),
..Default::default()
};
let em_rendered_width = window
.text_system()
.layout_line("m", font_size, &[run], None)
.width;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Feels like we should give this its own method like is done with em_width and em_advance for consistency

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, why does em_width not suffice here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

any comment on this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hey, apologies for the delay. Yeah i tried using em_width but it doesn't work and i'm not really sure why tbh. So i just made it match with the line wrapper calculation.

@errmayank errmayank force-pushed the soft-wrap-premature branch from a488c38 to 92d5901 Compare January 31, 2026 12:55
@Veykril Veykril force-pushed the soft-wrap-premature branch from 8c06eed to a5f6176 Compare February 13, 2026 10:08
@Veykril Veykril enabled auto-merge (squash) February 13, 2026 10:39
@Veykril Veykril merged commit 0c29a09 into zed-industries:main Feb 13, 2026
27 checks passed
@github-project-automation github-project-automation bot moved this from Community Champion PRs to Done in Quality Week – December 2025 Feb 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement community champion Issues filed by our amazing community champions! 🫶

Projects

Development

Successfully merging this pull request may close these issues.

Soft wrap is off by one character

5 participants