Is there a way of reusing a QIcon in differents QPushButtons?

JonB

@leonardo-M-B said in Is there a way of reusing a QIcon in differents QPushButtons?:

I'm setting the Icon of each one of the QPushButtons to the same image, but its using a lot of memory.

I wonder what evidence you have for this? However your are setting icons on pushbuttons the image is shared in Qt, so it probably doesn't much matter how you do it I wouldn't expect much memory to be used. You can use a single QIcon if you want, just create it once, I would not have thought this would be significant.

JoeCFD

@leonardo-M-B I guess not. setIcon uses a copy of your icon. Each button has its own copy.
https://doc.qt.io/qt-5.15/qabstractbutton.html#icon-prop

JonB

@JoeCFD
But how much is anyone saying that costs? Implicitly Shared Classes. There are a bunch of buttons all using the same icon images. Else there wouldn't be much point in having shared data classes?

JoeCFD

@JonB It is a copy.
https://codebrowser.dev/qt5/qtbase/src/widgets/widgets/qabstractbutton.cpp.html#_ZN15QAbstractButton7setIconERK5QIcon

JonB

@JoeCFD
I don't see how we are talking about the same thing. What does your link show? Yes it is a copy of a QIcon. The question is how much is that? The point is it does not contain a copy of the image (unless something odd is going on, OP should only need one pixmap). You have not commented on the shared data, so where is this going?

As for the original question, since setIcon() takes a copy of the QIcon passed to it there is the answer.

Is there any evidence that multiple push buttons having the same image is using multiple times the image's size?

JoeCFD

@JonB There is some sharing mechanism there when I look at QIcon class. I have not got how it is done. Will look more into the code. I am wrong at the copy thing. It looks like a copy, but actually is not.

Chris Kawa

As @JonB mentioned QIcon is implicitly shared, so copying it does not copy the underlying image data. Only some basic data like number of images contained and a pointer to the pixmap is actually duplicated (a couple bytes at most).

Something like this will not duplicate the image data:

QIcon ico(path);
QPushButton btn1(ico, text);
QPushButton btn2(ico, text);

Thanks to QPixmap also being implicitly shared this won't either:

QPixmap pix(path);
QPushButton btn1(pix, text); // using QIcon(const QPixmap&) implicit constructor
QPushButton btn2(pix, text);

And not even this, because QPixmap uses QPixmapCache underneath and only loads image once, based on its path and modification date:

QPushButton btn1(QPixmap (path), text);
QPushButton btn2(QPixmap (path), text);

You have to actually do some work to force it to duplicate, e.g.

QPixmap pix(path);
QPushButton btn1(pix, text);
pix.detach(); // force pixmap to clone shared data
QPushButton btn2(pix, text);

Another way this can happen is if your image does not fit in the cache (image is too big or there are a lot of smaller images loaded). The default is 10MB. If you're blowing past the cache size you can increase it via QPixmapCache::setCacheLimit(int). It's a bit counterintuitive, but increasing that size can reduce your overall memory usage if you have a lot of images in your app, because the sharing takes place more often.

JonB

@JoeCFD
When that referenced code goes d->icon = icon; this invokes QIcon's copy constructor. That is https://codebrowser.dev/qt5/qtbase/src/gui/image/qicon.cpp.html#_ZN5QIconaSERKS_

QIcon &QIcon::operator=(const QIcon &other)
{
    if (other.d)
        other.d->ref.ref();
    if (d && !d->ref.deref())
        delete d;
    d = other.d;
    return *this;
}

See how that works on incrementing/decrementing reference count rather than actually copying the icon's image pixmap.

The upshot is that copying QIcon neither uses much space nor much time. Qt does this for most of its "BLOB" classes (images, byte arrays, strings, ...). Have a read of Implicit Sharing, it's useful to know about

Many C++ classes in Qt use implicit data sharing to maximize resource usage and minimize copying. Implicitly shared classes are both safe and efficient when passed as arguments, because only a pointer to the data is passed around, and the data is copied only if and when a function writes to it, i.e., copy-on-write.

leonardo M B

@Chris-Kawa @JonB
In Practice I have this situation

button1->setIcon(QIcon(":/close.png"));
button2->setIcon(QIcon(":/close.png"));
button3->setIcon(QIcon(":/close.png"));
.
.
button911->setIcon(QIcon(":/close.png"));

This is also not a problem? I think I understood yours argumentations but I would like to confirm

JonB

@leonardo-M-B
Hmm, I wondered if you might have something like this.... I am not sure, but those may [well] be creating separate images for each icon (which might explain the surprise I found in your report). I would try:

QIcon close(QIcon(":/close.png"));
button1->setIcon(close);
button2->setIcon(close);
button3->setIcon(close);

Does that perchance make your report of "lots of memory" go away?

Chris Kawa

@leonardo-M-B Huh, unfortunately QIcon(const QString&) uses a different path in code. It does lookup for scaled images and ultimately does not use pixmap cache, so it will duplicate.

That's a bit of a shame actually. QPixmapIconEngine used in that constructor seems to be doing a roundtrip through QImage instead of directly using QPixmap, which bypasses the QPixmapCache optimization.

So either use a QPixmap constructor or like @JonB suggested create a single instance of QIcon and reuse it. Copying it will not duplicate data.

JoeCFD

@JonB d->icon = icon; this invokes QIcon's equal operator =(). You are right: QIcon has some sort of reference count. I will have a detailed look at it.