davidedmundson added inline comments.
INLINE COMMENTS
> iconitem.cpp:325
> + const QSize &paintedSize =
> m_iconPixmap.size().scaled(actualContainerSize, Qt::KeepAspectRatio);
> + return QSize(Units::roundToIconSize(paintedSize.width()),
> Units::roundToIconSize(paintedSize.height()));
> }
This makes no sense.
You can't round it to icon sizes *after* scaling, it means the shorter size is
always just wrong.
If we do merge this patch, you want:
m_iconPixmap.size().scaled(QSize(Units.roundtoIconSize(actualContainerSize.width),
Units.roundToIconSize(actualContainerSize.height)
> iconitem.cpp:377
> if (m_sizeChanged) {
> - const int iconSize =
> Units::roundToIconSize(qMin(boundingRect().size().width(),
> boundingRect().size().height()));
> - const QRect destRect(QPointF(boundingRect().center() -
> QPointF(iconSize/2, iconSize/2)).toPoint(),
> - QSize(iconSize, iconSize));
> -
> + const QSize &newSize = paintedSize();
> + const QRect destRect(QPointF(boundingRect().center() -
> QPointF(newSize.width(), newSize.height()) / 2).toPoint(), newSize);
You're using references a lot in a few patches that don't make any sense.
paintedSize returns a new QSize object; you're keeping a reference to a value
that would just be immediately discarded. This doesn't result in an
optimisation but instead doing something that would crash if it wasn't for a
magic C++ feature where it lengthens the lifespan of the object.
REPOSITORY
R242 Plasma Frameworks
REVISION DETAIL
https://phabricator.kde.org/D3892
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: broulik, #plasma, hein
Cc: davidedmundson, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas
