D10433: Add QML support for Prison

[David Edmundson]

davidedmundson added a comment.


  Cool in principle, few nitpicks.

INLINE COMMENTS

> barcodequickitem.cpp:37
> +BarcodeQuickItem::BarcodeQuickItem(QQuickItem *parent)
> +    : QQuickPaintedItem(parent)
> +{

Is createBarcode a heavy function? It sounds like it could be.

If so I'd recommend using QQmlParserStatus so you don't generate it 4 times on startup as each property gets set.

> barcodequickitem.cpp:113
> +
> +    const auto img = m_barcode->toImage(m_barcode->minimumSize());
> +    painter->drawImage(QRectF(x, y, w, h), img, img.rect());

you have the actual up-to-date item size from inside paint. Would that be better than minimumSize?

Please also change to

img = m_barcode->toImage(size * qApp->devicePixelRatio());
img.setDevicePixelRatio(qApp->devicePixelRatio());

for high DPI support.

> barcodequickitem.cpp:131
> +        m_barcode->setBackgroundColor(m_bgColor);
> +        m_barcode->toImage(m_barcode->minimumSize());
> +        setImplicitWidth(m_barcode->minimumSize().width());

You're doing this in the paint, do you need to do this here?

REPOSITORY
  R280 Prison

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

To: vkrause, #frameworks, svuorela
Cc: davidedmundson, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180210/e22cfbfe/attachment.html>