[Marble-devel] Review Request: Put MarbleMap::rotateBy() under test

[Bernhard Beschow]

> On Oct. 15, 2011, 8:29 p.m., Torsten Rahn wrote:
> > 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?

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.


- Bernhard


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102877/#review7355
-----------------------------------------------------------


On Oct. 15, 2011, 8:11 p.m., Bernhard Beschow wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102877/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2011, 8:11 p.m.)
> 
> 
> Review request for Marble and Torsten Rahn.
> 
> 
> Description
> -------
> 
> 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?
> 
> 
> Diffs
> -----
> 
>   src/lib/MarbleMap.cpp c4b40b1 
>   tests/MarbleMapTest.cpp 9a4ac8c 
> 
> Diff: http://git.reviewboard.kde.org/r/102877/diff/diff
> 
> 
> Testing
> -------
> 
> 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).
> 
> 
> Thanks,
> 
> Bernhard Beschow
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20111016/ee8822f7/attachment.html>