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

Use QStaticText to boost text rendering performance #301

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

blahgeek
Copy link
Contributor

@blahgeek blahgeek commented Oct 6, 2019

Currently, text rendering in qtermwidget is feature-rich (unicode, font ligatures, etc.) but not fast enough. This change uses QStaticText to cache layout information of each text fragment. Since that for typical terminal use case (consider scolling in vim), same text fragments are being rendered repeatedly, caching layout information can save a lot of time. Also, this change does not break font ligatures or unicode.

You can easily notice the performance improvement using vim or some other similar terminal applications.

I've been using my terminal with this change as my primary terminal for several weeks now and haven't found any issues.

However there's some changes in the code that may require more discussion:

  • The size of the cache is now set to a fixed value (4096)
  • QStaticText doesn't work well with underline and strikeout, so I draw them individually instead
  • drawStaticText starts in the top-left corner instead of the baseline, so I didn't find any reliable way to correctly align characters from different fonts (fallback fonts, especially those CJK characters). I now assumes that the height from top to baseline of fallback fonts is 80% of the its total height.
  • I removed LTR_OVERRIDE_CHAR because, I don't know, it somewhat make the behavior of QStaticText incorrect (alignment issue). I don't write RTL text so unfortunately I cannot verify if it breaks anything.

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

Successfully merging this pull request may close these issues.

2 participants