<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/122861/">https://git.reviewboard.kde.org/r/122861/</a>
</td>
</tr>
</table>
<br />
<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;">Looks good to me. It would be good if Eike could also have a look since he's refactoring this code right now, and his refactoring might benefit from the same changes, if not avoid conflicts.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">One stylistic remark (spread over three lines) inline.</p></pre>
<br />
<div>
<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/122861/diff/1/?file=353809#file353809line48" style="color: black; font-weight: bold; text-decoration: underline;">containments/desktop/package/contents/code/LayoutManager.js</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">44</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kd">var</span> <span class="nx">i</span> <span class="o">=</span> <span class="mi">0</span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="nx"><span class="hl">i</span></span><span class="hl"> </span><span class="o"><span class="hl"><</span></span> <span class="nx">plasmoid</span><span class="p">.</span><span class="nx">applets</span><span class="p">.</span><span class="nx">length</span><span class="p">;</span> <span class="o">++</span><span class="nx">i</span><span class="p">)</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">45</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kd">var</span> <span class="nx">i</span> <span class="o">=</span> <span class="mi">0</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="nx"><span class="hl">j</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span> <span class="nx">plasmoid</span><span class="p">.</span><span class="nx">applets</span><span class="p">.</span><span class="nx">length</span><span class="p">;</span> <span class="nx"><span class="hl">i</span></span><span class="hl"> </span><span class="o"><span class="hl"><</span></span><span class="hl"> </span><span class="nx"><span class="hl">j</span></span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="o">++</span><span class="nx">i</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">I think it's more readable to init j before the loop.</p></pre>
</div>
</div>
<br />
<div>
<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/122861/diff/1/?file=353809#file353809line58" style="color: black; font-weight: bold; text-decoration: underline;">containments/desktop/package/contents/code/LayoutManager.js</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">54</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kd">var</span> <span class="nx">i</span><span class="o"><span class="hl">=</span></span><span class="mi"><span class="hl">0</span></span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="nx"><span class="hl">i</span></span><span class="o"><span class="hl"><</span></span><span class="nx">itemsStrings</span><span class="p">.</span><span class="nx">length</span><span class="p">;</span> <span class="nx">i</span><span class="o"><span class="hl">++</span></span><span class="p">)</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">55</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kd">var</span> <span class="nx">i</span> <span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="mi"><span class="hl">0</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="nx"><span class="hl">j</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="nx">itemsStrings</span><span class="p">.</span><span class="nx">length</span><span class="p">;</span> <span class="nx">i</span><span class="hl"> </span><span class="o"><span class="hl"><</span></span><span class="hl"> </span><span class="nx"><span class="hl">j</span></span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="o"><span class="hl">++</span></span><span class="nx"><span class="hl">i</span></span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">same here</p></pre>
</div>
</div>
<br />
<div>
<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/122861/diff/1/?file=353809#file353809line146" style="color: black; font-weight: bold; text-decoration: underline;">containments/desktop/package/contents/code/LayoutManager.js</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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">function restore()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">138</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kd">var</span> <span class="nx">i</span> <span class="o">=</span> <span class="nx">row</span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="nx"><span class="hl">i</span></span><span class="hl"> </span><span class="o"><span class="hl"><</span></span> <span class="nx">row</span><span class="o"><span class="hl">+</span></span><span class="nx">availableSize</span><span class="p">.</span><span class="nx">width</span><span class="p">;</span> <span class="o">++</span><span class="nx">i</span><span class="p">)</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">140</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kd">var</span> <span class="nx">i</span> <span class="o">=</span> <span class="nx">row</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="nx"><span class="hl">j</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span> <span class="nx">row</span> <span class="o"><span class="hl">+</span></span><span class="hl"> </span><span class="nx">availableSize</span><span class="p">.</span><span class="nx">width</span><span class="p">;</span> <span class="nx"><span class="hl">i</span></span><span class="hl"> </span><span class="o"><span class="hl"><</span></span><span class="hl"> </span><span class="nx"><span class="hl">j</span></span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="o">++</span><span class="nx">i</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">init j before loop for readability</p></pre>
</div>
</div>
<br />
<p>- Sebastian Kügler</p>
<br />
<p>On March 8th, 2015, 9:08 p.m. UTC, Kai Uwe Broulik 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 Plasma.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated March 8, 2015, 9:08 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This micro-optimizes the LayoutManager by:
- using Array and Object literals rather than new Object/Array, and also creating the whole structure at once if applicable
- Store end values in for loops rather than calculating them on each iteration
- Adjust coding style here and there</p></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;">Moving applets (especially using Eike's press-and-hold when using it on a touchscreen :P) feels snappier, doesn't print any new warnings on console and seems to work as before. However, I can no longer cause plasmashell to go berserk when moving a small applet ontop of a huge one (eg. small sticky note on wide fuzzy clock) where it desperately tries to find a place and fails.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think the grid size should be based on units somehow, having a 24x24 grid on a high dpi screen also benefits the aforementioned behavior.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">One surprising discovery I made is that using Qt.point instead of a handcrafted JS Object is one order of magnitude(!) slower.</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>containments/desktop/package/contents/code/LayoutManager.js <span style="color: grey">(14d0dfc)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122861/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>