<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/120920/">https://git.reviewboard.kde.org/r/120920/</a>
</td>
</tr>
</table>
<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/120920/diff/5-6/?file=329421#file329421line227" style="color: black; font-weight: bold; text-decoration: underline;">ksnapshot.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 5 - 6)
</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; ">KSnapshot::KSnapshot(KSnapshotObject::CaptureMode mode, QWidget *parent)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">227</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">case</span> <span class="n">KSnapshotObject</span>:<span class="o">:</span><span class="n">WindowUnderCursor</span><span class="o">:</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">227</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">case</span> <span class="n">KSnapshotObject</span>:<span class="o">:</span><span class="n">WindowUnderCursor</span><span class="o">:</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;">Used the kdevelop Edit->Reformat Source on ksnapshot.cpp. Hope this minor formatting is fine.</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/120920/diff/5-6/?file=329422#file329422line74" style="color: black; font-weight: bold; text-decoration: underline;">ksnapshotsendtoactions.h</a>
<span style="font-weight: normal;">
(Diff revisions 5 - 6)
</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">53</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">setKSnapshotForKipi</span><span class="p">(</span><span class="n">KSnapshot</span><span class="o">*</span> <span class="n">ksnapshot</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;">Introduced this method because:
1. The kipi interface needs a QWidget to work
2. ksnapshotimagecollectionshared has a dependency to ksnapshot->urlToOpen (which maybe can be removed, see comment in ksnapshotimagecollectionshared.cpp)</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/120920/diff/5-6/?file=329423#file329423line72" style="color: black; font-weight: bold; text-decoration: underline;">ksnapshotsendtoactions.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 5 - 6)
</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; ">KSnapshotSendToActions::~KSnapshotSendToActions()</pre></td>
</tr>
</tbody>
<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">72</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">auto</span> <span class="n">kipiInterface</span> <span class="o">=</span> <span class="k">new</span> <span class="n">KIPIInterface</span><span class="p">(</span><span class="n">_this</span><span class="o">-></span><span class="n">m_ksnapshot</span><span class="p">);</span> <span class="c1">// </span><span class="cs">TODO</span><span class="c1">: causes "QObject: Cannot create children for a parent that is in a different thread." warning</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;">This runtime warning should be removed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have not found out if this is the reason why the kipi plugin list remains empty.</p></pre>
</div>
</div>
<br />
<p>- Gregor Mi</p>
<br />
<p>On December 4th, 2014, 10:10 p.m. UTC, Gregor Mi 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 Graphics, KSnapshot, Aaron J. Seigo, and Dominik Haumann.</div>
<div>By Gregor Mi.</div>
<p style="color: grey;"><i>Updated Dec. 4, 2014, 10:10 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=312495">312495</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ksnapshot
</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;">Hi, I also feel slightly impaired by Bug 312495 - Very slow "Send to" menu. So here is a fix.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It consists of the following changes:
1. move long-running retrieval of SendTo items to separate method
2. fillSendToActionsCache at startup but not yet async
3. move code related to SendTo actions to own class and file
(this includes the kipi code but KIPI_FOUND was not set in my dev env so thats not tested)
4. SendTo menu is built on a separate thread to speed up populating
5. delete actions in dtor
6. Create main window on stack instead of heap</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The SendTo actions are now created in QThread which is started on startup of ksnapshot.
If the user opens the SendTo menu too soon there is a message (as single menu item) that says
that the menu is still being constructed and that he/she should try a moment later.</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;">1)
- Start ksnapshot (notice that is as fast as before)
- Immediately open SendTo menu => there is a message that the menu is not ready yet.
- Try again after 1 second: menu is populated.
- Close ksnapshot.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2)
- Start ksnapshot. Wait two seconds and then click SendTo menu
=> menu is populated right away
- Close it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">3)
Start ksnapshot. Hit the SendTo menu with insanely high button click speed.
=> No crash observed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">4)
Start ksnapshot and close it immediately.
=> No crash observed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">5) [pending]
Could no test the kipi code because KIPI_FOUND is not set yet on my environment.</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>.reviewboardrc <span style="color: grey">(PRE-CREATION)</span></li>
<li>CMakeLists.txt <span style="color: grey">(b8e665610284ae9db644e6925acae3d60e207245)</span></li>
<li>ksnapshot.h <span style="color: grey">(07100a85bbb146501ee77a9cbb9d2655edda4595)</span></li>
<li>ksnapshot.cpp <span style="color: grey">(192a0910553acf9180155ba0f409d7b15d125753)</span></li>
<li>ksnapshotsendtoactions.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>ksnapshotsendtoactions.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>main.cpp <span style="color: grey">(e7a1efb2fd8398492b96e6f21f90363d99358e12)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120920/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>