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
