[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