Improve preview thumbnail quality
ClosedPublic

Authored by hein on Feb 1 2018, 9:13 AM.

Details

Summary

This patch does two things:

  • Instead of giving the KAbstractViewAdapter the actual icon size, we overprovision by giving it the size times two. This is because while we're deriving the grid view cell size from the icon size setting, it's not the actual size the thumbnails will be displayed at. Our IconItem is sized to almost fill the cell, and for image sources with non-square aspect ratios IconItem doesn't scale to the next icon size.
  • We set IconItem to do smooth scaling.

While this results in lovely visual fidelity, I the performance
impact is a concern. We're requesting twice as large thumbnails
now, and we're doing more scaling work. However, thumbnail
generation is async and doesn't slow down listing. There's also
the possibility IconItem::smooth could have general performance
impact unless things are smart enough not to scale when not
necessary, which is hopefully the case.

BUG:376848

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hein created this revision.Feb 1 2018, 9:13 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 1 2018, 9:13 AM
hein requested review of this revision.Feb 1 2018, 9:13 AM

Random by-passer nitpick comment: "twice as large thumbnails"... actually 4 x large in pixels, given you scale 2x in two dimensions ;)

hein added a comment.EditedFeb 1 2018, 3:00 PM

Random by-passer nitpick comment: "twice as large thumbnails"... actually 4 x large in pixels, given you scale 2x in two dimensions ;)

hein added a comment.Feb 1 2018, 3:01 PM
This comment was removed by hein.
ngraham added a subscriber: ngraham.Feb 1 2018, 4:05 PM
hein added a comment.Feb 5 2018, 11:37 AM

Before:

After:

broulik accepted this revision.Feb 5 2018, 1:35 PM
broulik added a subscriber: broulik.

Please remove the unrelated changes before pushing

containments/desktop/plugins/folder/foldermodel.cpp
1896–1900

Unrelated?

1912–1916

Unrelated?

1944–1948

Unrelated?

This revision is now accepted and ready to land.Feb 5 2018, 1:35 PM

Nice!

The visual artifacts here look similar or the same as ones we sometimes see in Discover and KInfoCenter at HiDPi scale factors:


Related? Might the same fix might work there too?

markg added a subscriber: markg.Feb 5 2018, 2:32 PM

Hmm, weird. In my eyes the knovi thumbnail in the before image looks sharper than the after one. It's just blurred in the after one, not better.
I'm guessing the QML smooth property has a fairly naive implementation (in Qt).

ngraham added a comment.EditedFeb 5 2018, 2:37 PM

@markg it's not about sharpness or blurring, but rather the "before" screenshot is pixellated and has weird artifacts around the edges.

ngraham accepted this revision.Feb 5 2018, 2:38 PM

Either way, to my mind slightly blurry is better than slightly blurry, pixellated, and bugged around the edges, so +1.

This revision was automatically updated to reflect the committed changes.
christianlopez added a subscriber: christianlopez.EditedFeb 8 2018, 6:04 PM

Hi, i was playing a bit with the code and i'm noticing you can get good looking images without activating the smooth and without making the thumbnails look blurry, also i think i know the reason why the thumbnails look distorted. At the line 239 in FolderItemDelegate.qml there's the object PlasmaCore.IconItem which has the properties width and height. At the moment: the properties are calculated the following way:

width: root.useListViewMode ? main.GridView.view.iconSize : (parent.width - 2 * units.smallSpacing)
height: main.GridView.view.iconSize

I noticed this produces numbers like: width: 81.5714 height: 48 or with: 97.8333 height, 64 or width: 64.35 height: 32
In all this cases both numbers are very different and i noticed the more different this numbers are between them the more distorted and blocky the thumbnail looks, i managed to produce perfectly looking thumbnails by putting the same number in both fields, for example: width: 80 height: 80, interestingly this doesn't affects the proportions of the thumbnails.

The problem is that if the icons are configured, for example to have an icon size of 48 they'll be much smaller if we use: width: 48 height 48, and if we use the calculated value of 81.57 in both: width: 81.57 height: 81.57 the thumbnails will be very big so there must be a way set the same number on both properties while making the thumbnails to be shown at the desired size.

Here's how it looks normally with the calculated values in the code (width: 97.8333 height: 64):

This is how it looks if i set with: 64 height: 64 :

This is how it looks if i set with: 98 height: 98 :

I don't know if this information may help, it's the first time i try to modify the code of plasma.

Edit: I forgot something important. To get this results i used

adapterIconSize: gridView.iconSize * 2;

in folderView.qml as described in the patches, otherwise it'll look ugly again.

Edit 2: i'm noticing to make the thumbnails look as desired the important thing is to put the value calculated for the width, the only problem is that this makes larger other icons like the folders too so this should be applied only to the thumbnails to keep the rest of the icons the way they used to look and to not make the grid larger.

1: This is how it looks if i use

width: root.useListViewMode ? main.GridView.view.iconSize : (parent.width - 2 * units.smallSpacing)
height: (parent.width - 2 * units.smallSpacing)

2: This is how it looks if i use

width: root.useListViewMode ? main.GridView.view.iconSize : main.GridView.view.iconSize
height: main.GridView.view.iconSize

At the moment 2 is the best option in my opinion but if i could get 2 with the size of the thumbnails in 1 it would be perfect. I would need to know how to put the same number in width and height in a way that only makes larger the thumbnail but not the rest of the icons or the grid.

hein added a comment.Feb 9 2018, 12:37 PM

Thanks for your analysis! I actually did try roughly the same thing, but I thought keeping the behavior of having larger preview thumbs is nicer overall.