broulik added a comment.
Not really, I recently wanted to update this patch to include support for
Icon Scale definition but ran into a dead-end of having to pass through the
scale in 50 places... I think I'll give it a go once more but only special
casing SVGs and leaving everything
acrouthamel added a comment.
Hey there, any movement on this? I've been submitting some patches to fix
icon scaling in apps and have noticed how they switch from monochrome to
colored (hires) versions when fixed. Getting the underlying issue fixed would
be great as more people buy
mart added a comment.
In https://phabricator.kde.org/D6313#118285, @kvermette wrote:
> Behaviorally speaking there's justification for ensuring SVGs are treated
the same as PNGs in this case. Looking at the code we aren't shimming the SVGs
specifically (unless I'm missing something),
kvermette added a comment.
In https://phabricator.kde.org/D6313#118250, @davidedmundson wrote:
> For SVG icons this is fine.
>
> For pixmap icons this is only part of the needed changes.
>
> We don't want to load the 16px image and then resize it, I think that's
what this would
cfeck added inline comments.
INLINE COMMENTS
> cfeck wrote in kiconloader.cpp:863
> Please use some rounding here. Scaling factors such as 1.4 cannot be
> represented exactly.
>
> Either add some formatting specifiers, e.g. for three decimal places, or use
> qRound(scale * 1000).
Just
cfeck added inline comments.
INLINE COMMENTS
> kiconloader.cpp:1299
>
> -if (d->findCachedPixmapWithPath(key, pix, path)) {
> +if (d->findCachedPixmapWithPath(key, pix, path)) {// skip cache
> if (path_store) {
Is this a left-over change from disabling this check?
REPOSITORY
cfeck added inline comments.
INLINE COMMENTS
> kiconloader.cpp:863
> + % QLatin1Char('@')
> + % QString::number(scale)
> % QLatin1Char('_')
Please use some rounding here. Scaling factors such as 1.4 cannot be
represented exactly.
Either add some formatting
davidedmundson added a comment.
For SVG icons this is fine.
For pixmap icons this is only part of the needed changes.
We don't want to load the 16px image and then resize it, I think that's what
this would do? That would be an unacceptable regression.
We would need a folder
broulik created this revision.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.
REVISION SUMMARY
Since Qt 5.9 there's a ScaledPixmapHook in QIconEngine which is called when
device pixel ratio is > 1 and it wants a
