Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb: Use GlyphRun fonts, not the first available font, for more things #2796

Merged

Conversation

AtkinsSJ
Copy link
Member

@AtkinsSJ AtkinsSJ commented Dec 5, 2024

Partly addresses #2787.

I found a few places where we're calling first_available_font() for things where we actually want to know the exact font used by these particular code-points. And, I've fixed some. Specifically, selection rectangles and text-editing cursor positions.

Before

Screencast_20241205_200256.webm

After

Screencast_20241205_200448.webm

Source

<style>
html, textarea { font-family: lololol; }
</style>
<textarea>Well hello friends! 😅😅😅😅😅😅😅😅😅😅</textarea>

It is absolutely not perfect still. A couple of issues:

  1. Somehow, Noto Emoji has a space character, and since that's the first font, that's the space character we use - hence the comically large spaces between words.
  2. Selecting multiple emojis near the end of the textarea still jumps the selection around. But it's better than it was.

The remaining spots we don't handle, which I've FIXME'd:

  • SVG text paths just use the first available font, instead of identifying which font per code-point like regular text painting.
  • The text in list-item markers use the first available font too.

(There are also several places where using the first available font is appropriate as far as I can tell, so I've left those.)

We might have other places we do the wrong thing like this, but I'm sure we'll notice those eventually.

We incorrectly used the first available font to measure this before,
which may or may not be the correct font for this text.
This makes the cursor appear in the correct place when the text makes
use of multiple different fonts.
It's not clear how to address these right now, so add a FIXME to make
them easier to find and address later.
@kalenikaliaksandr kalenikaliaksandr merged commit ebc9168 into LadybirdBrowser:master Dec 6, 2024
6 checks passed
@AtkinsSJ AtkinsSJ deleted the default-font-metrics branch December 6, 2024 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants