<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/104861/">http://git.reviewboard.kde.org/r/104861/</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="http://git.reviewboard.kde.org/r/104861/diff/2/?file=62775#file62775line66" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/MarbleClock.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">qreal MarbleClock::dayFraction() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">62</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">msecdelta</span> <span class="o">=</span> <span class="mi">1000</span> <span class="o">*</span> <span class="n">m_lasttime</span><span class="p">.</span><span class="n">secsTo</span><span class="p">(</span> <span class="n">curenttime</span> <span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">66</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">msecdelta</span> <span class="o">=</span> <span class="mi">1000</span> <span class="o">*</span> <span class="n">m_lasttime</span><span class="p">.</span><span class="n">secsTo</span><span class="p">(</span> <span class="n">curenttime</span> <span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think this is not precise enough, as msec part of drift is not taken into account.</pre>
</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="http://git.reviewboard.kde.org/r/104861/diff/2/?file=62775#file62775line76" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/MarbleClock.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">qreal MarbleClock::dayFraction() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">72</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// trigger round minute update (at m_speed pace)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">76</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">sleeptime</span> <span class="o">=</span> <span class="p">(</span> <span class="mf">1000.0</span> <span class="o">*</span> <span class="n">m_updateInterval</span> <span class="p">)</span> <span class="o">/</span> <span class="n">m_speed</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">sleeptime MUST at some point depend on msecdelta, because no system garantees to wake you up at the sharp number of msecs you wanted. This is also documented on QTimer.
The issue is converting real time discrepancy of QTimer back into m_speed pace to offset the drift.
m_datetime and sleeptime were introduced before m_speed existed and calculus was correct back then.</pre>
</div>
<br />
<p>- Thibaut</p>
<br />
<p>On May 4th, 2012, 10:43 p.m., Dennis Nienhüser wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Dennis Nienhüser.</div>
<p style="color: grey;"><i>Updated May 4, 2012, 10:43 p.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;">MarbleClock currently rounds the internal clock to minutes. The implementation has rounding errors that result in the sleep timeout being calculated incorrectly and a fallback mechanism then sets it to one second. The same fallback mechanism handles negative speeds (time running backwards), which is wrong as well.
I'm not sure why the time is rounded to minutes. If it's done for aesthetic reasons, then it should rather be done in the UI code by setting the time once to the desired nice looking time. The timeout calculation however should stick to what is set from the outside. For a similar reason I'd like to remove the restriction that the timeout cannot be shorter than one second. If that is to be done for performance reasons, it should be done elsewhere (e.g. in the user interface where the clock settings can be manipulated). On my system I don't experience any problems though even when using the most extreme time settings (100x realtime at 1 second refresh = 100 fps) with the satellites plugin running.
The patch removes the rounding to minutes and timeout restriction. It also handles negative speeds correctly. This fixes the wrong signal emission timing.
</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;">Custom application using
mapWidget->model()->clock()->setSpeed( 40 );
mapWidget->model()->clock()->setUpdateInterval( 20 );
and
mapWidget->model()->clock()->setSpeed( 10 );
mapWidget->model()->clock()->setUpdateInterval( 20 );
and some other values.
Marble with satellites plugin and various time / refresh settings.
</pre>
</td>
</tr>
</table>
<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=299388">299388</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/lib/MarbleClock.cpp <span style="color: grey">(56b2fe4)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104861/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>