[Marble-devel] Review Request v2: Add "Download Region" feature
Jens-Michael Hoffmann
jensmh at gmx.de
Sun Apr 25 23:35:13 CEST 2010
Thanks for the reviews of the initial patch.
Since reviewboard is very slow at the moment, I'm sending the second version
now directly to the mailing list.
Changes since version 1:
================
- Review fix: make label "Number of tiles:" more verbose ("Number of tiles to download:").
- DownloadRegionDialog: Review fix: use QDialogButtonBox, looks much better now.
- DownloadRegionDialog: Review fix: Change "current region" to "visible region"
which is more concrete.
Also adjust corresponding qualifiers.
- MarblePart: use kDebug() instead of mDebug().
- Download region: fix 32bit overflow when calculating number of tiles to download.
- LatLonBoxWidget: instead of using 1.0 as single step value, use 10% of angle.
- DownloadRegionDialog::region: switch from tile coords to pixel coords.
When calculating tile coords, it is better to do it in pixel coords as long
as possible to not loose accuracy for no reason. With the current approach
of using the top level (of the pyramid) coords as reference this might not
give benefit, but once we switch to bottom level coords, there will be much
better accuracy.
- TileCoordsPyramid: fix potential crash.
TileCoordsPyramid has got dynamically allocated data members and so needs to
define a copy constructor and an assignment operator especially since
TileCoordsPyramid objects are copied in existing code.
- TileCoordsPyramid: improve accuracy of tile coordinates and tiles count calculations.
Use bottom level coordinates for calculations instead of top level coordinates,
so we don't loose information.
- TileLevelRangeWidget: Assure that maximum top level is not higher than then
current bottom level. Likewise for minimum bottom level: should not be
smaller than current top level.
- TileLevelRangeWidget: improve API consistency.
As this is a TileLevelRangeWidget it is not necessary to prefix level
with tile. The levels this widget deals with are inherently tile levels.
The methods topLevel and bottomLevel already reflect this.
So following methods were renamed:
setAllowedTileLevelRange -> setAllowedLevelRange
setDefaultTileLevel -> setDefaultLevel
- TileLevelRangeWidget: rename spin boxes:
minSpinBox -> topSpinBox
maxSpinBox -> bottomSpinBox
- TileLevelRangeWidget: remove obsolete comment, break long line.
- Disable copy operations on DownloadRegionDialog, LatLonBoxWidget and TileLevelRangeWidget
as these classes are indirectly derived from QObject.
- DownloadRegionDialog: set window title.
- DownloadRegionDialog: Review fix: show message box every time when the limit
of (currently) 100000 tiles to download is exceeded.
- After dropping the "Tile" infix the identifiers are shorter now and
this line break is not needed anymore.
Best regards,
Jens-Michael Hoffmann
-------------- next part --------------
A non-text attachment was scrubbed...
Name: marble-download-region-2.diff
Type: text/x-patch
Size: 47569 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/marble-devel/attachments/20100425/3bb72b6b/attachment-0001.diff
More information about the Marble-devel
mailing list