<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112079/">http://git.reviewboard.kde.org/r/112079/</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 26th, 2013, 6:37 a.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112079/diff/2/?file=184119#file184119line31" style="color: black; font-weight: bold; text-decoration: underline;">staging/kguiaddons/src/util/kiconutils.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">31</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Draws the \a overlay over the \a icon in the specified \a position</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Not sure about the use of "draw" here. Those methods don't draw, right?
"Adds" sounds better to me.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">They do, they call "painter->drawPixmap(overlayIcon)" after all. I'd like to keep "draw" so it's clear it really does "draw".</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 26th, 2013, 6:37 a.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112079/diff/2/?file=184119#file184119line48" style="color: black; font-weight: bold; text-decoration: underline;">staging/kguiaddons/src/util/kiconutils.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">48</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KGUIADDONS_EXPORT</span> <span class="n">QIcon</span> <span class="nf">addOverlay</span><span class="p">(</span><span class="k">const</span> <span class="n">QIcon</span> <span class="o">&</span><span class="n">icon</span><span class="p">,</span> <span class="k">const</span> <span class="n">QHash</span><span class="o"><</span><span class="n">Qt</span><span class="o">::</span><span class="n">Corner</span><span class="p">,</span> <span class="n">QIcon</span><span class="o">></span> <span class="o">&</span><span class="n">overlays</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I tend to agree with Nicolas on the plural for naming that one.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ok</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 26th, 2013, 6:37 a.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112079/diff/2/?file=184120#file184120line30" style="color: black; font-weight: bold; text-decoration: underline;">staging/kguiaddons/src/util/kiconutils.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">30</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">paint</span><span class="p">(</span><span class="n">QPainter</span> <span class="o">*</span><span class="n">painter</span><span class="p">,</span> <span class="k">const</span> <span class="n">QRect</span> <span class="o">&</span><span class="n">rect</span><span class="p">,</span> <span class="n">QIcon</span><span class="o">::</span><span class="n">Mode</span> <span class="n">mode</span><span class="p">,</span> <span class="n">QIcon</span><span class="o">::</span><span class="n">State</span> <span class="n">state</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmmm, why did you ignore all the Q_DECL_OVERRIDE comments for that class?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Little misunderstanding, will fix.</pre>
<br />
<p>- Martin</p>
<br />
<p>On August 23rd, 2013, 9:25 a.m. UTC, Martin Klapetek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated Aug. 23, 2013, 9:25 a.m.</i></p>
<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;">As this was rejected from QIcon, I decided to implement it in frameworks. The idea is new private icon engine (KOverlayIconEngine), which paints the overlays itself, and then an utility function in KIconUtils:: namespace, which takes the base icon and the overlay icons as parameters and constructs new QIcon using KOverlayIconEngine. As QPixmapIconEngine (QIcon default) is private in Qt and couldn't be simply extended, the KOverlayIconEngine simply forwards the calls to QIcon, which then forwards those calls internally to QPixmapIconEngine (QIcon is really mostly just a wrapper around an engine).
There are two functions
QIcon KIconUtils::kIconAddOverlay(const QIcon &icon, const QIcon &overlay, Qt::Corner position);
and
QIcon KIconUtils::kIconAddOverlay(const QIcon &icon, QHash<Qt::Corner, QIcon> overlays);
The first one serves for single overlay icon, the second one is then for multiple overlays, requires the QHash to be built up before passing though.</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;">I have a small testing app with 3 QLabels, testing both ::paint() and ::pixmap() methods, all works \o/ I'm thinking about bundling the app with kguiaddons and/or additionally creating a unittest for it (I have one for Qt already), but I'll do that later as I'm off to vacation and I'd like to get comments for the code.</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>staging/kguiaddons/src/CMakeLists.txt <span style="color: grey">(fc8941a)</span></li>
<li>staging/kguiaddons/src/util/kiconutils.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>staging/kguiaddons/src/util/kiconutils.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112079/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>