<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124811/">https://git.reviewboard.kde.org/r/124811/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 19th, 2015, 5:52 p.m. UTC, <b>Volker Krause</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry, this does not change the KMail case, it's still hitting the disk for the first icon of each type requested via QIcon::fromTheme, after that the in-memory cache kicks in.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">IIUC the problem is that on the pure QIcon::fromTheme path, the icon never actually makes it into the persistent cache. If it is in there already, it is properly retrieved it seems.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ah, I think I see one. hasIcon() emulates loading the icon in size "Desktop", while kmail probably loads all its icons in size Toolbar or Small (menuitems).
So it doesn't match, the on-disk cache of the small-size icon doesn't help hasIcon().
Sounds like hasIcon needs to have its own code to lookup the cache in various sizes before hitting the disk for one of them.
Another patch, for another day...</p></pre>
<br />
<p>- David</p>
<br />
<p>On August 19th, 2015, 7:25 a.m. UTC, David Faure wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, Christoph Feck and Volker Krause.</div>
<div>By David Faure.</div>
<p style="color: grey;"><i>Updated Aug. 19, 2015, 7:25 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kiconthemes
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This makes QIcon::fromTheme() much faster, for all cases where we have loaded the
icon once before (including in another process).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The unittest has a way to see if KIconLoader used the cache or searched on disk, with the newly exported global int. I couldn't think of a better way.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'll let Volker do the real-world testing and measure the overall impact as he did previously.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>autotests/kiconloader_unittest.cpp <span style="color: grey">(6b60e7ebfc970c94ae865d56c4e33a8034b4fcee)</span></li>
<li>src/kiconloader.cpp <span style="color: grey">(db3aa8f8fd6f8fb706edcb27cce073c36831934d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124811/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>