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

Advanced text #1830

Merged
merged 180 commits into from
May 11, 2023
Merged

Advanced text #1830

merged 180 commits into from
May 11, 2023

Conversation

hecrj
Copy link
Member

@hecrj hecrj commented May 5, 2023

This PR brings all the improvements that have been introduced in the advanced-text branch. Specifically:

It's hard to overstate the importance of these changes! All of these features combined make one of the biggest changesets to land in master since the inception of the library!

These efforts should lay the foundations for acquiring maturity in the long run (multi-line text input, accessibility, rendering parallelization, and more!). For more details, check out the latest roadmap diagram.

Closes #33.
Fixes #41.
Closes #1071.
Closes #1199.

hecrj added 30 commits February 24, 2023 13:17
`SwashCache` can't be easily trimmed and it's not really getting us
anything since `glyphon` already caches using a glyph atlas anyways.
We avoid recreating buffers this way, and the amount of layers should
stay relatively low anyways.
This avoids text from misteriously disappearing, even if the user uses a
`height` that isn't enough to fit the text.
@tarkah
Copy link
Member

tarkah commented May 9, 2023

image

I just noticed after testing the tour example that any longish glyph (like the gs here) is getting cut off at the bottom (tested on windows & linux). Adjusting the to_absolute function for the logical pixels of LineHeight and just adding some manual padding makes the long characters render correctly, so I'm assuming the issue is with the new LineHeight changes. Or perhaps longer glyphs go outside the "bounds" of the text?

I'm noticing the same issue on a personal project with a couple different fonts.

@hecrj
Copy link
Member Author

hecrj commented May 9, 2023

@bungoboingo @tarkah This is caused by 9a8b30d.

I have incremented the default line height to 1.3 since it seems more readable to me and should prevent this issue. It may be interesting to eventually expose a flag to control the clipping behavior on overflow.

@bungoboingo
Copy link
Contributor

bungoboingo commented May 9, 2023

@bungoboingo @tarkah This is caused by 9a8b30d.

I have incremented the default line height to 1.3 since it seems more readable to me and should prevent this issue. It may be interesting to eventually expose a flag to control the clipping behavior on overflow.

Where is line-height measured from with cosmic-text? I would assume it's measured from the bottom of the "descent" of a character, e.g. the tail of a g or p, and then divided evenly to pad the top/bottom of the line. It seems in advanced-text it's being measured from the "baseline" of the line. For example, if I set line height to 1 in Iced, I would expect something like CSS spec (line-height: 1 below).

image

** Edited to make more clear

@hecrj
Copy link
Member Author

hecrj commented May 9, 2023

@bungoboingo I would expect the same. Seems it's an issue others have encountered as well: pop-os/cosmic-text#123.

@hecrj
Copy link
Member Author

hecrj commented May 9, 2023

Hopefully, the 1.3 default line height makes the issue less common. If it still persists, I can revert 9a8b30d for now.

@i509VCB
Copy link

i509VCB commented May 11, 2023

One question I have is whether the candidate backend selection should order backends based on the most performant option. I'd think something like this:

  1. All hardware adapters for wgpu in the order wgpu enumerates
  2. Software renderer
  3. Software vulkan implementations (via wgpu)

https://github.com/iced-rs/iced/blob/advanced-text/renderer/src/compositor.rs#L155

@hecrj
Copy link
Member Author

hecrj commented May 11, 2023

@i509VCB The software renderer can't really fail to initialize and it's always bundled currently. Thus, option 3 would never be reached.

Can we reliably guarantee that our software rendering implementation is more performant than the driver virtual implementations? I don't know if this has an obvious answer or we need to run some actual numbers.

@hecrj hecrj merged commit 669f7cc into master May 11, 2023
@hecrj hecrj deleted the advanced-text branch May 11, 2023 14:45
@hecrj hecrj restored the advanced-text branch May 11, 2023 15:03
@hecrj hecrj deleted the advanced-text branch May 11, 2023 15:04
@i509VCB
Copy link

i509VCB commented May 11, 2023

Can we reliably guarantee that our software rendering implementation is more performant than the driver virtual implementations? I don't know if this has an obvious answer or we need to run some actual numbers.

I guess the answer is it depends. Lavapipe might be able to use AVX-512 if supported but then there is the issue of compiler overhead. tiny-skia does have some simd support.

Probably needs to be measured anyways.

@hecrj hecrj restored the advanced-text branch May 11, 2023 17:37
@hecrj hecrj deleted the advanced-text branch May 11, 2023 17:39
@hecrj
Copy link
Member Author

hecrj commented May 11, 2023

@gabrieldechichi I was able to reproduce and fix the issue in #1847.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
8 participants