> On May 3, 2015, 10:10 a.m., Dennis Nienhüser wrote: > > src/lib/marble/geodata/data/GeoDataCoordinates.h, line 380 > > <https://git.reviewboard.kde.org/r/123579/diff/2/?file=365494#file365494line380> > > > > These are not used outside GeoDataCoordinates, right? I'd move them to > > GeoDataCoordinatesPrivate instead and call them via d->lonLatToUTMString() > > etc then. > > Alejandro García Montoro wrote: > I'm thinking about reordering all the UTM-related code. My idea is: > delete all `{lon,lat,lonLat}ToUTMString()` I've done and implement four main > functions for obtaining the four main values of UTM (zone, latitude band, > easting and northing). These four functions would be implemented based on all > the mathematical stuff that was placed in GeoDataUTM. Hence, we can move all > those mathematical functions to GeoDataCoordinatesPrivate and make them > static as you suggested in the other issue. > > Futhermore, the four main functions should have two versions: one static > and one non-static. My idea es: implement them in GeoDataCoordinatesPrivate > as static (maybe something like `lonLatToZone(qreal lon, qreal lat)`, > `lonLatToLatitudeBand(qreal lon, qreal lat)`, ...) and then declare the other > version in GeoDataCoordinates (they could maybe named just `zone()`, > `latitudeBand()` and so on). The idea behind making a public non-static > version is just to give the possibility of obtaining the UTM values from a > GeoDataCoordinates object, as it is possible now for the lon/lat units with > `longitude()` or `latitude()`. It's not critical but I think it could help to > improve the coordinate system support. > > Implementing the static versions is mandatory if we want to use them from > lonToString and latToString (which are static). However, these two functions > need a bigger refactoring as you said in another issue, as this proposal does > not solve the real problem: we have to guess the UTM values from just one > component of the pair (lon,lat), when we really need both of them. > > If you think this is a good way to advance, I can finish this > implementation (I have it nearly done) and update the review request. And, > please, feel free to comment anything you does not like. > > Thank you! :)
Sounds all fine to me. Wrt to zone(), latitudeBand() and similar methods I'd rather go for utmZone(), utmLatitudeBand() etc since GeoDataCoordinates::zone seems too generic to me. - Dennis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123579/#review79789 ----------------------------------------------------------- On May 2, 2015, 6:57 p.m., Alejandro García Montoro wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123579/ > ----------------------------------------------------------- > > (Updated May 2, 2015, 6:57 p.m.) > > > Review request for Marble. > > > Repository: marble > > > Description > ------- > > Two different tasks have been accomplished concerning UTM coordinate system: > > 1. The UTM grid rendering has been fixed. Now the grid and labels are drawn > as expected (see > http://upload.wikimedia.org/wikipedia/commons/e/ed/Utm-zones.jpg for > comparison) > 2. Some UTM functions have been added in order to properly calculate and > display the coordinates on the status bar. Those functions have been wrapped > in the new GeoDataUTM namespace inside GeoDataCoordinates.cpp If you prefer > to see the functions integrated in the GeoDataCoordinates class, please, let > me know. > > **Still to be fixed**: The UTM string showed in the status bar is too long > for the text field in which it is shown. I have not been able to change its > size, as I have not found the relevant .ui file; where in the code should I > check to change that? > > **Note**: The lat/lon <-> UTM conversion algorithm has been adapted from this > JavaScript code > http://home.hiwaay.net/~taylorc/toolbox/geography/geoutm.html. The author > says in the page "The JavaScript source code in this document may be copied > and reused without restriction", and I have linked the website in the code. > However, as I am not sure about all the licenses and authoring stuff, I would > like you to review it. > > > Diffs > ----- > > src/apps/marble-kde/marble_part.cpp > 283ad87a39f91e01c391a4dd8f4c9ddbeef41481 > src/apps/marble-qt/QtMainWindow.cpp > c4280c62b4534b7d10387b6895d142920f957bb7 > src/lib/marble/geodata/data/GeoDataCoordinates.h > 6efabd047f8f3aff63381facb78cb371c0740bf4 > src/lib/marble/geodata/data/GeoDataCoordinates.cpp > fe74a2e42db921b152a29ef04246ceaaa16c2325 > src/plugins/render/graticule/GraticulePlugin.cpp > a5e6d14a921c66d55cf01e39b16564dedcf6532b > > Diff: https://git.reviewboard.kde.org/r/123579/diff/ > > > Testing > ------- > > Changes tested in master branch @ 1st May. > Grid and coordinates work well (except for the displaying issue explained > above) and are consistent with the UTM coordinate system. > > > File Attachments > ---------------- > > UTM grid exceptions around Norway > > https://git.reviewboard.kde.org/media/uploaded/files/2015/05/01/505df756-87f2-47bb-a6c1-d5b52629547a__NorwayScreenshot.png > > > Thanks, > > Alejandro García Montoro > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
