[Emojier] Optimize performance
Needs ReviewPublic

Authored by Zren on Feb 12 2020, 12:02 AM.


Group Reviewers

BUG: 417454

The emojier fairly slow. Launching is slow. Resizing the window is slow. Changing pages is slow. Scrolling is also slow.

On launch, the sidebar widens from 0px, resizing the GridView, which resizes cellWidth as we abuse it's the width calculation to create cell spacing. Resizing 1000 emoji Labels is slow. This also happens when you resize the window. The cpu core spikes to 100% (on a 2.8Ghz cpu).

After selecting an emoji, closing, and reopening, the recent emoji's is shown first (without a search box) which opens faster since it has much less work. However switching to the all emoji page resurfaces the problem.

Scrolling is also a problem, as loading the emoji QQC2.Labels is slow. So we'll use a Loader { asynchronous: true }. I'm not sure why it loads them from bottom to top.

Here's what it looks like after this patch. Resizing no longer blocks, nor does scrolling.

Test Plan

How I compile and run emojier by itself:

Diff Detail

R119 Plasma Desktop
Lint Skipped
Unit Tests Skipped
Zren created this revision.Feb 12 2020, 12:02 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 12 2020, 12:02 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
Zren requested review of this revision.Feb 12 2020, 12:02 AM
ngraham added a subscriber: ngraham.

Very nice.

This comment was removed by davidedmundson.

I suspect DelegateRecycler doesn't do grid, otherwise this could have been an option?

What the original code was trying to do was distribute the whitespace among cells, right now if you resize to be 5.8 columns wide we want to spread that out, not have a gap on the white.

You're right on the fonts resizing though.
This is also a fix:

  • fontSizeMode: Text.Fit

+ fontSizeMode: Text.VerticalFit

Though given we have a fixed grid size, maybe we could just set a pixelSize in here. That'd be even faster.

Zren added a comment.Feb 12 2020, 10:10 AM

I used Loader as I didn't want loading glyphs to block scrollbar responsiveness. GridView reuses delegates, but I don't think it's the MouseArea + Label constructor that's slow. I think it's the "rendering the emoji glyph" in the GUI draw that is the root cause.

The cell spacing code re-renders the glyph (which is a slow process), which is why I removed it. Though I guess we can use anchor.centerIn:

MouseArea {
	id: mouse
	width: emojiView.cellWidth
	height: emojiView.cellHeight

	Label {
		anchor.centerIn: mouse
		width: emojiView.desiredSize
		height: emojiView.desiredSize

... to keep the cell spacing if that's a requirement.

Using fontSizeMode: Qt.VerticalFit would make emoji's overflow horizontally. Various "hand + skin tone" emoji's that don't yet have a custom emoji are 2x wider than they are tall.

apol added a comment.Feb 12 2020, 1:38 PM

Right, maybe we can find a better way of doing this. Or just delaying resizing of the Label since it's indeed the slow part.

apol added inline comments.Feb 12 2020, 2:00 PM

This was added so that the columns adapt to the width of the view, otherwise it was very fast (as fast as without your Loader) but there was a bit of an empty space on the right.

broulik added inline comments.Feb 12 2020, 2:02 PM

but there was a bit of an empty space on the right.

If Label resizing is the problem. Would keeping the Label square and using a padding Item around it fix the performance issue?

apol added inline comments.Feb 12 2020, 2:06 PM

Yes, or just not fitting the text I guess, which is unfortunate but maybe necessary at the moment.