1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
|
From: Aaron Seigo <aseigo@kde.org>
Date: Fri, 06 May 2011 13:19:09 +0000
Subject: use a QWeakPointer on the KIconLoader passed in as there are no lifetime guarantees
X-Git-Url: http://quickgit.kde.org/?p=kdelibs.git&a=commitdiff&h=a8d16682c31ef523ffebba6e19283a19cd5f5627
---
use a QWeakPointer on the KIconLoader passed in as there are no lifetime guarantees
usually KGlobal::iconLoader() is used, so this isn't an issue seen very often.
however, when a local KIconLoader is created, it is easy to get QIcons with a
KIconEngine that has a bad KIconLoader pointer in them. particularly as QIcon
is implicitly shared and easily passed around. the StatusNotifier Plasma DataEngine
was triggering this, though it would be trivial to run into this problem again
anytime a KIconLoader is created locally
thankfully, QWeakPointer does the job and is very fast and light. (confirmed
both with my own testing and confirmation from Thiago).
massive thanks to Michael Pyne for detecting the cause of the problem via Valgrind.
BUG:258706
---
--- a/kdeui/icons/kiconengine.cpp
+++ b/kdeui/icons/kiconengine.cpp
@@ -27,16 +27,16 @@
KIconEngine::KIconEngine(const QString& iconName, KIconLoader* iconLoader, const QStringList& overlays)
+ : mIconName(iconName),
+ mIconLoader(iconLoader),
+ mOverlays(overlays)
{
- mIconName = iconName;
- mIconLoader = iconLoader;
- mOverlays = overlays;
}
KIconEngine::KIconEngine(const QString& iconName, KIconLoader* iconLoader)
+ : mIconName(iconName),
+ mIconLoader(iconLoader)
{
- mIconName = iconName;
- mIconLoader = iconLoader;
}
static inline int qIconModeToKIconState( QIcon::Mode mode )
@@ -65,8 +65,12 @@ QSize KIconEngine::actualSize( const QSi
return QSize(iconSize, iconSize);
}
-void KIconEngine::paint( QPainter * painter, const QRect & rect, QIcon::Mode mode, QIcon::State state )
+void KIconEngine::paint(QPainter * painter, const QRect & rect, QIcon::Mode mode, QIcon::State state)
{
+ if (!mIconLoader) {
+ return;
+ }
+
Q_UNUSED(state)
const int kstate = qIconModeToKIconState(mode);
@@ -80,20 +84,27 @@ void KIconEngine::paint( QPainter * pain
}
const int iconSize = qMin(rect.width(), rect.height());
- const QPixmap pix = mIconLoader->loadIcon(mIconName, group, iconSize, kstate, mOverlays);
+ const QPixmap pix = mIconLoader.data()->loadIcon(mIconName, group, iconSize, kstate, mOverlays);
painter->drawPixmap(rect, pix);
}
-QPixmap KIconEngine::pixmap( const QSize & size, QIcon::Mode mode, QIcon::State state )
+QPixmap KIconEngine::pixmap(const QSize & size, QIcon::Mode mode, QIcon::State state)
{
Q_UNUSED(state)
+ if (!mIconLoader) {
+ QPixmap pm(size);
+ pm.fill(Qt::transparent);
+ return pm;
+ }
+
const int kstate = qIconModeToKIconState(mode);
const int iconSize = qMin(size.width(), size.height());
- QPixmap pix = mIconLoader->loadIcon(mIconName, KIconLoader::Desktop, iconSize, kstate, mOverlays);
+ QPixmap pix = mIconLoader.data()->loadIcon(mIconName, KIconLoader::Desktop, iconSize, kstate, mOverlays);
- if(pix.size() == size)
+ if (pix.size() == size) {
return pix;
+ }
QPixmap pix2(size);
pix2.fill(QColor(0,0,0,0));
@@ -111,7 +122,7 @@ QString KIconEngine::key() const
QIconEngineV2 *KIconEngine::clone() const
{
- return new KIconEngine(mIconName, mIconLoader, mOverlays);
+ return new KIconEngine(mIconName, mIconLoader.data(), mOverlays);
}
bool KIconEngine::read(QDataStream &in)
--- a/kdeui/icons/kiconengine_p.h
+++ b/kdeui/icons/kiconengine_p.h
@@ -75,7 +75,7 @@ class KIconEngine : public QIconEngineV2
private:
QString mIconName;
QStringList mOverlays;
- KIconLoader* mIconLoader;
+ QWeakPointer<KIconLoader> mIconLoader;
};
inline KIconEngine::~KIconEngine()
|