[Marble-devel] Review Request: Showing/editing the coords in bookmark editor

Torsten Rahn tackat at kde.org
Wed Sep 14 21:39:45 UTC 2011



> On Sept. 14, 2011, 1:43 p.m., Torsten Rahn wrote:
> > Looks good to me. 
> > Ideally the LatLonEdit would also take the Notation into account: 
> > Check Marble's Settings dialog on the "View" tab: The Angle setting controls whether in Marble DMS or Decimal Notation is used: In Decimal Notation Marble uses a notation like 49,999° N 20,5° E while in DMS it uses 49° 59' 59" N 20° 30' 00" E. It would be great if the LatLonWidget would have a setNotation method and if inside Marble it would follow Marble settings.
> 
> Friedrich W. H. Kossebau wrote:
>     Thanks for review, pushed to master.
>     
>     Missed to see that setting so far. Agreed, the bookmark editor should then offer a widget for Decimal editing if Angle is set to that.
>     
>     Will give it a try to add such a mode to LatLonWidget.
>     
>     BTW, would it make sense to you to rename LatLonWidget to LonLatWidget? Would follow the usual convention as mentioned in the CODING file that first comes the lon parameter, then the lan one. :)
>     
>     And a more serious question: I just saw, of course after pushing, that it might not be a good idea to simply copy the old altitude to the new GeoDataCoordinates in EditBookmarkDialog::coordinates(), as that value may be pretty bogus.
>     Should that parameter perhaps be displayed as well in the bookmark editor, perhaps with an button next to it to "Get ground altitude for current lon and lat values" (button could have wizard icon or better on it).
> 
> Torsten Rahn wrote:
>     Regarding the Notation: great!
>     We have some inconsistency with the naming already due to the LatLon naming in the KML spec. So I'm not sure whether it's worth bothering to change existing occurances ... . 
>     
>     The altitude described refers to the altitude of the camera! So the coordinate is not the coordinate of the object but the coordinate of the observer above the object ... so it's not exactly bogus ... ;-)

Regarding the DMS/Decimal Notation: It would be great if the editor distinguished between both modes.

Regarding LatLon vs LonLat: We have some inconsistency with the naming already due to the LatLon naming in the KML spec. So I'm not sure whether it's worth bothering to change existing occurances ... . 

Regarding Altitude "bogus" values: The altitude described refers to the altitude of the camera! So the coordinate is not the coordinate of the object but the coordinate of the observer above the object ... so it's not exactly bogus ... ;-)


- Torsten


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


On Sept. 13, 2011, 11:09 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102608/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2011, 11:09 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Summary
> -------
> 
> This patchs adds the possibility to see and also edit also the coords of a bookmark in the bookmark editor.
> 
> Initially used a lineeddit and GeoDataCoordinates::toString, but GeoDataCoordinates::fromString cannot yet handle coords with secs it seems.
> So turned to use a LatLonEdit per coord.
> 
> Found LatLonEdit to be quite incomplete, so rewrote it to get it working for me.
> E.g. LatLonEdit::setDimension(...) was pretty broken.
> Also needed/wanted a double value for the secs editor, to enable more precision.
> No idea if 2 digits is a sane number of digits after the ".".
> 
> If LatLonEdit has to stay ABI compatible I will have to redo it then, as I changed the API
> (removed unused/useless dimensionChanged signal and changed some method signatures).
> 
> 
> Diffs
> -----
> 
>   src/lib/BookmarkManagerDialog.cpp 3f9db62 
>   src/lib/EditBookmarkDialog.h 4157726 
>   src/lib/EditBookmarkDialog.cpp 7e36ddc 
>   src/lib/EditBookmarkDialog.ui 9baf50d 
>   src/lib/LatLonEdit.h 4958805 
>   src/lib/LatLonEdit.cpp 758a167 
>   src/lib/LatLonEdit.ui 26774d3 
> 
> Diff: http://git.reviewboard.kde.org/r/102608/diff
> 
> 
> Testing
> -------
> 
> Added and viewed/edited several coords of geo bookmarks, worked all fine, same values shown after reloading or same places focussed if selected.
> 
> Also tested overflow behaviour of LatLonEdit for quite some border cases.
> 
> LatLonEdit widget still works fine in worldclock plasmoid.
> 
> 
> Thanks,
> 
> Friedrich W. H.
> 
>

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


More information about the Marble-devel mailing list