<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/102877/">http://git.reviewboard.kde.org/r/102877/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 15th, 2011, 8:29 p.m., <b>Torsten Rahn</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;">What do you mean by unpredicted results? :-) Are you talking about the erratic behaviour when looking at the poles?
> * does the new implementation really match the intention?
I think so. Previously one of the things that distinguished rotateBy from centerOn was that rotateBy didn't cause an immediate update.However this has changed (since this can be accomplished diffently).
I also see that you correctly apply the normalization. I had removed this in the original implementation at one point since the Quaternion always gets newly created and not as a result of successive rotations applied. As I didn't notice a difference without the normalization I removed it (especiially for the rotation of actual points/nodes where it would add a costly operation).
> * are there any test rows missing?
I don't know. do you have tests with the south pole pointing upwards?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks for your fast reply!
> What do you mean by unpredicted results?
With unpredictable results I mean that resultLonLat didn't equal currentLonLat + deltaLonLat
> do you have tests with the south pole pointing upwards?
Good point. I've added such test cases.</pre>
<br />
<p>- Bernhard</p>
<br />
<p>On October 15th, 2011, 8:11 p.m., Bernhard Beschow 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 and Torsten Rahn.</div>
<div>By Bernhard Beschow.</div>
<p style="color: grey;"><i>Updated Oct. 15, 2011, 8:11 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;">This patch puts MarbleMap::rotateBy() under test. Since the current implementation produces unpredictable results, rotateBy() is modified such that the resulting LonLat equals currentLonLat + deltaLonLat, which seems to be the intention of the current implementation.
Questions:
* does the new implementation really match the intention?
* are there any test rows missing?</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;">When centering on the north pole in spherical projection, the globe is rotated around the planet axis (running through north and south pole) when the mouse arrows appear. The patch preserves this behavior (although possibly with slightly different values).</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/lib/MarbleMap.cpp <span style="color: grey">(c4b40b1)</span></li>
<li>tests/MarbleMapTest.cpp <span style="color: grey">(9a4ac8c)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102877/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>