fvogt requested changes to this revision.
fvogt added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> mprisplugin.cpp:489
> + // now parse the size...
> + const QVector<QStringRef> sizeSegments =
> size.splitRef(QLatin1Char('x'), QString::SkipEmptyParts, Qt::CaseInsensitive);
> + if (sizeSegments.count() == 2) {
Why change this line? With auto it was easier to read, just the `&` should be
removed for clarity. Now it also accepts `xx42xx42xx" as valid size, while it
didn't before.
> mprisplugin.cpp:498
> +
> + auto dist = std::numeric_limits<qreal>::max() - 1;
> +
That's still equal to `std::numeric_limits<qreal>::max()` due to lacking
precision.
> mprisplugin.cpp:510
> +
> + dist = targetSamples - effectiveSamples;
> + }
This algorithm only looks at the aspect ratio of the image, not the resolution.
For 4096x4096, 1024x1024 and 512x512 you'll get the same `effectiveSamples`
value, so it will load huge thumbnails again.
REPOSITORY
R856 Plasma Browser Integration
REVISION DETAIL
https://phabricator.kde.org/D20736
To: broulik, #plasma, fvogt
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
