-
-
Notifications
You must be signed in to change notification settings - Fork 449
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 QFontMetrics::elidedText
over manual implementation
#4807
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
Alternate reproduction steps that don't impact others:
Requires debug build |
Thank you for mentioning that! I added it to the reproduction. |
Just realized that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
Description
There is a major performance issue when displaying replies to a message with certain Arabic characters (measuring the width for them seems to be a slow path). This PR attempts to resolve parts of that issue.
Reproduction
If you're running a debug build, do the following:
!!!
as a new splitWhen running a release build:
The manual text elision is really slow here, because it removes one character of the last overflowing word at a time. Qt has a native text elision algorithm, that seems to be faster (
QFontMetrics::elidedText
).This isn't the exact same workload (as I couldn't interact with Chatterino before) but in both cases, the same channel was shown.
Further Improvements
Even with this implementation, Chatterino will lag if many channels are open and one or more channels contain a reply to a bad message. This is because there are too many forced layouts. When many channels are open and an image gets loaded, all
ChannelViews
are forcefully laid out. Images are loaded when laying out messages (see #3929 and #3929).A
ChannelView
that's not visible shouldn't need to be laid out. I attempted a fix inperf/layout-visible
. This does have some consequences (e.g. switching channels isn't as smooth but not really noticeable - could be behind a setting [lazy vs. eager layout]). On this upside, this has the same effect as #3929 in that images won't be loaded if they're not needed, thus reducing memory usage and reducing lag when loading recent messages at the start for all open channels.Another optimization one can make is to swap
LimitedQueue
for message layouts inside aChannelView
with a rawboost::circular_buffer
, since it's only ever accessed by the GUI thread. I did that inperf/no-limited-queue
. This will reduce a lot of unnecessary copies (for all the snapshots) resulting in less memory use and lower rendering times (copying the shared pointers requires incrementing the reference count).Yet another optimization could be to remember in a
MessageLayout
if it contained any unloaded images.MessageLayout::layout
could then ignore layout requests if an image was loaded, but the current element doesn't contain any unloaded image.