D5506: Don't even try to create icons with empty sizes

[Mark Gaiser]

markg requested changes to this revision.
markg added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kiconengine.cpp:87
>  
> -    if (!size.isValid()) {
> +    if (!size.isValid() || size.height() == 0 || size.width() == 0) {
>          return QPixmap();

It took me some minutes to figure this line out. I was "expecting" the isValid call to only return true if with and height are both > 0
So for a QSize(0, 1) i would have expected a false, but it returns true. 0,0 is apparently a valid size.

Then i figured out that isValid is the wrong function for the task you want to do.
You want:

  if (size.isEmpty()) {
    // ...
  }

>From the Qt docs:

> The isValid() function determines if a size is valid (a valid size has both width and height greater than or equal to zero). The isEmpty() function returns true if either of the width and height is less than, or equal to, zero, while the isNull() function returns true only if both the width and the height is zero.

And i tested it for you as well:

  QSize a(0, 0);
  QSize b(0, 1);
  QSize c(1, 1);
  QSize d(0, -1);
  
  qDebug() << a.isEmpty(); // true
  qDebug() << b.isEmpty(); // true
  qDebug() << c.isEmpty(); // false
  qDebug() << d.isEmpty(); // true

Exactly how i expect it.

REPOSITORY
  R302 KIconThemes

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

To: apol, #frameworks, markg
Cc: markg, kfunk
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170419/5156b34b/attachment.html>