D14308: [Exe Thumbnailer] Improve icon selection algorithm

Stefan BrĂ¼ns noreply at phabricator.kde.org
Tue Jul 24 15:42:02 BST 2018


bruns added inline comments.

INLINE COMMENTS

> icoutils_common.cpp:28
> +{
> +    // compute difference of areas
> +    const qreal desiredAspectRatio = (desiredHeight > 0) ? desiredWidth / static_cast<qreal>(desiredHeight) : 0;

Can not see an area calculation here ...

> broulik wrote in icoutils_common.cpp:33
> No idea, really.

I think the formula is a little bit to ad-hoc, and actually wrong.

We have 2 different notable cases. `desiredAspectRatio` obviously is a constant, this leaves us with `candidateAspectRatio` and `delta`

1. all candiates have exactly the same aspect ratio. The difference below is constant, thus has no influence on the sorting. Sorting is determined by `delta`alone (good).
2. candidates have different aspect ratios. Any candidate with a slightly different aspect ratio is immediately heavily penalized, and thus discarded (bad).

Think of e.g. a requested size of 96x96, and two candidates, 32x32 and 128x64. The small one has a `distance` of 128, while the second one has a distance 25032. I don't think that's wanted here.

REPOSITORY
  R320 KIO Extras

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

To: broulik, #frameworks, dfaure, ngraham, pali, vonreth, antlarr
Cc: bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180724/4be36a33/attachment.html>


More information about the Kde-frameworks-devel mailing list