D29526: Thumbnails: make thumbnail generation dpr-aware

Harald Sitter noreply at phabricator.kde.org
Wed May 13 14:43:40 BST 2020


sitter added a comment.


  You do seem to calculate the deviceWidth and height an awful lot, it makes reading a bit clunky. I'd much rather have the multiplication done once and then always use the var instead. In fact, perhaps it'd make sense to have createV3 feed the values into the implementations? Currently they all repate the same two lines over and over.

INLINE COMMENTS

> djvucreator.cpp:61
>    QByteArray sizearg, fnamearg;
> -  sizearg = QByteArray::number(width) + 'x' + QByteArray::number(height);
> +  sizearg = QByteArray::number(width * devicePixelRatio) + 'x' + QByteArray::number(height * devicePixelRatio);
>    fnamearg = QFile::encodeName( path );

Surely multiplication results need converting to int. That being said, you do multiply below again, so maybe just move maxWidth/Height up here.

> thumbnail.cpp:774
>  
>  void ThumbnailProtocol::scaleDownImage(QImage& img, int maxWidth, int maxHeight)
>  {

Var naming is a bit inconsistent across the code base now. In the implementations there are maxWidth/H that are device-adjusted but in here they are not. Not a huge concern, just noticed.

REPOSITORY
  R320 KIO Extras

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

To: meven, #frameworks, dfaure, broulik, sitter, ngraham
Cc: kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200513/3abc2f11/attachment.htm>


More information about the kfm-devel mailing list