D10223: Improve preview thumbnail quality

[Eike Hein]

hein added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma, broulik, ngraham
Cc: christianlopez, markg, broulik, ngraham, kossebau, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Christian López]

christianlopez added a comment.


  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 momen:  
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 notices 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):
  F5698834: Screenshot_20180208_115655.png 

  
  This is how it looks if i set with: 64 height: 64 :
  F5698842: Screenshot_20180208_115930.png 

  
  This is how it looks if i set with: 98 height: 98 :
  F5698844: Screenshot_20180208_120034.png 

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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma, broulik, ngraham
Cc: christianlopez, markg, broulik, ngraham, kossebau, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:db42961ad4d8: Improve preview thumbnail quality (authored 
by hein).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D10223?vs=26304&id=26589#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10223?vs=26304&id=26589

REVISION DETAIL
  https://phabricator.kde.org/D10223

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderItemDelegate.qml
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/plugins/folder/foldermodel.cpp

To: hein, #plasma, broulik, ngraham
Cc: markg, broulik, ngraham, kossebau, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.12

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma, broulik, ngraham
Cc: markg, broulik, ngraham, kossebau, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Nathaniel Graham]

ngraham added a comment.


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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.12

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma, broulik
Cc: markg, broulik, ngraham, kossebau, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Mark Gaiser]

markg added a comment.


  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).

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.12

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma, broulik
Cc: markg, broulik, ngraham, kossebau, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Nathaniel Graham]

ngraham added a comment.


  Nice!
  
  The visual artifacts here look similar or the same as ones we sometimes see 
in Discover and KInfoCenter at HiDPi scale factors:
  
  F5693185: Screenshot_20180122_143515.png 

  F5693186: Pixellated logo.png 
  
  - https://bugs.kde.org/show_bug.cgi?id=388253
  - https://bugs.kde.org/show_bug.cgi?id=388633
  
  Related? Might the same fix might work there too?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.12

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma, broulik
Cc: broulik, ngraham, kossebau, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Kai Uwe Broulik]

broulik accepted this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  Please remove the unrelated changes before pushing

INLINE COMMENTS

> foldermodel.cpp:1896-1900
> +if (m_appletInterface) {
> +Plasma::Applet *applet = 
> m_appletInterface->property("_plasma_applet").value();
> +QQuickItem* gObj = qobject_cast *>(applet->property("_plasma_graphicObject").value());
> +uiDelegate.setQWindow(gObj->window());
> +}

Unrelated?

> foldermodel.cpp:1918-1922
> +if (m_appletInterface) {
> +Plasma::Applet *applet = 
> m_appletInterface->property("_plasma_applet").value();
> +QQuickItem* gObj = qobject_cast *>(applet->property("_plasma_graphicObject").value());
> +uiDelegate.setQWindow(gObj->window());
> +}

Unrelated?

> foldermodel.cpp:1956-1960
> +if (m_appletInterface) {
> +Plasma::Applet *applet = 
> m_appletInterface->property("_plasma_applet").value();
> +QQuickItem* gObj = qobject_cast *>(applet->property("_plasma_graphicObject").value());
> +uiDelegate.setQWindow(gObj->window());
> +}

Unrelated?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.12

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma, broulik
Cc: broulik, ngraham, kossebau, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Eike Hein]

hein added a comment.


  Before:
  
  F5692985: Screenshot_20180205_203453.png 

  
  After:
  
  F5692983: Screenshot_20180205_203401.png 


REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma
Cc: ngraham, kossebau, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Eike Hein]

hein added a comment.


  In https://phabricator.kde.org/D10223#199103, @hein wrote:
  
  > In https://phabricator.kde.org/D10223#199099, @kossebau wrote:
  >
  > > Random by-passer nitpick comment: "twice as large thumbnails"... actually 
4 x large in pixels, given you scale 2x in two dimensions ;)
  >
  >
  > https://www.youtube.com/watch?v=RbTUTNenvCY&t=9s

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma
Cc: kossebau, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Eike Hein]

hein added a comment.


  In https://phabricator.kde.org/D10223#199099, @kossebau wrote:
  
  > Random by-passer nitpick comment: "twice as large thumbnails"... actually 4 
x large in pixels, given you scale 2x in two dimensions ;)
  
  
  https://www.youtube.com/watch?v=RbTUTNenvCY&t=9s

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma
Cc: kossebau, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Friedrich W . H . Kossebau]

kossebau added a comment.


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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D10223

To: hein, #plasma
Cc: kossebau, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10223: Improve preview thumbnail quality

[Eike Hein]

hein created this revision.
hein added a reviewer: Plasma.
Restricted Application added a project: Plasma.
hein requested review of this revision.

REVISION 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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.12

REVISION DETAIL
  https://phabricator.kde.org/D10223

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderItemDelegate.qml
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/plugins/folder/foldermodel.cpp

To: hein, #plasma
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart