<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="https://git.reviewboard.kde.org/r/115923/">https://git.reviewboard.kde.org/r/115923/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 20th, 2014, 9:13 p.m. CET, <b>Martin Gräßlin</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="https://git.reviewboard.kde.org/r/115923/diff/1/?file=245131#file245131line127" style="color: black; font-weight: bold; text-decoration: underline;">src/declarativeimports/core/svgitem.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void SvgItem::paint(QPainter *painter)</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QSGNode* SvgItem::updatePaintNode(QSGNode* oldNode, UpdatePaintNodeData* updatePaintNodeData)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">121</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">SvgItem</span><span class="o">::</span><span class="n">paint</span><span class="p">(</span><span class="n">QPainter</span> <span class="o">*</span><span class="n">painter</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">126</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">QSGNode</span><span class="o">*</span> <span class="n">SvgItem</span><span class="o">::</span><span class="n">updatePaintNode</span><span class="p">(</span><span class="n">QSGNode</span><span class="o">*</span> <span class="n">oldNode</span><span class="p">,</span> <span class="n">UpdatePaintNodeData</span><span class="o">*</span> <span class="n">updatePaintNodeData</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;">nitpick: looking at the surrounding coding style the * should be moved from type to name</pre>
</blockquote>
<p>On February 20th, 2014, 9:54 p.m. CET, <b>David Edmundson</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;">I'm really not sure which Martin is worse...
/me grumbles and fixes it.</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;">There is a coding style defined and it's there so the resulting code, shared with the whole world and worked on by many people, does not look like it went through set of earthquakes :P</pre>
<br />
<p>- Martin</p>
<br />
<p>On February 20th, 2014, 8:22 p.m. CET, David Edmundson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Plasma.</div>
<div>By David Edmundson.</div>
<p style="color: grey;"><i>Updated Feb. 20, 2014, 8:22 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-framework
</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;">The rationale behind this patch is on the mailing list in the thread "Minutes Monday"
This doesn't boost performance or save memory much, but it paves the way for texture sharing, faster resizing, and plenty of other things.
Based on Frederick's comment I have reverted my changes to use QImage everywhere, otherwise we lose out on the local QPixmap cache in KImageCache.
Changes to plasmacore are minimal.
I'm currently porting FrameSVG which is where we should see more gains, but I thought I should get this reviewed/merged in parallel.
I have only seen one regression which is in the analog clock.
Some odd code in the analog clock (by me apparently!) means the width is dependent on the current width, which due to some changes in this patch ends up in a constant spiral getting to infinitely sized and explode.
Changelog (in reverse order):
Remove manual isDirty tracking in SvgItem
Always resize the node geometry on resizes
Update to paint to fill the size of the object, not the size of texture
Fix leaking texture
Add convenient QImage image() getter in SVG
Avoid repainting if node is not changed
Render SvgItem natively rather than going through QQuickPaintedItem</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>src/declarativeimports/core/svgitem.h <span style="color: grey">(c8be7cc)</span></li>
<li>src/declarativeimports/core/svgitem.cpp <span style="color: grey">(e90751a)</span></li>
<li>src/plasma/svg.h <span style="color: grey">(01d98f8)</span></li>
<li>src/plasma/svg.cpp <span style="color: grey">(9ec2aa5)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115923/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>