D11319: [KStandardItemListWidget] Avoid needless image resizing

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kstandarditemlistwidget.cpp:1013
>  
> -    m_scaledPixmapSize = m_pixmap.size();
> -    m_scaledPixmapSize.scale(maxScaledIconWidth, maxScaledIconHeight, Qt::KeepAspectRatio);
> +    // Scale the original pixmap size to avoid rounding errors
> +    m_scaledPixmapSize = m_pixmap.size() / m_pixmap.devicePixelRatio();

It's best to avoid the term "scale".

It could be scaling from logical to device pixels, or scailng from device to logical. 
So it tells me literally nothing.

Sticking to either the term "logical" or "device" seems to work best.

> kstandarditemlistwidget.cpp:1380
> +        if (qAbs(m_scaledPixmapSize.width() - pixmapDprSize.width()) > 1
> +                || qAbs(m_scaledPixmapSize.height() - pixmapDprSize.height()) > 1) {
> +            KPixmapModifier::scale(scaledPixmap, m_scaledPixmapSize * qApp->devicePixelRatio());

This is overly complex.

We have the pixmap at size    in device pixels (logical size * DPR)

When we divide back to logical pixels, we lose resolution. So....don't.

You can do it all in device pixels, which is the part that's relevant here.

if (m_scaledPixmapSize * pixmap.devicePixelRatio !=  pixmap.size()) {

}

REPOSITORY
  R318 Dolphin

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

To: broulik, #dolphin, elvisangelaccio, davidedmundson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180320/6eb0acbc/attachment.htm>