[Marble-devel] Review Request: Show "Download Region" dialog non modal

[jensmh at gmx.de]

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

Review request for marble.


Summary
-------

This consists of the following individual changes:
==================================================
- Show DownloadRegionDialog non modal.
  Works as before. For now it does not react to changes of the visible region.
- DownloadRegionDialog: Add slot setVisibleRegion.
  This is needed for making the dialog non-modal and the slot will
  be connected to the visibleLatLonAltBoxChanged signal.
- Update tiles count in DownloadRegionDialog when the visible lat/lon box changes.
  Map theme changes (involving eventually a change of tile size) are not yet handled.
- DownloadRegionDialog: Add (for now empty) slot for map theme changes and
  connect it to MarbleWidgets themeChanged signal.
- DownloadRegionDialog: Add "Apply" button which also triggers the download,
  but leaves the dialog open to allow another download.
- DownloadRegionDialog: When the map theme changes, update the texture layer
  and tiles count, as the tile layout might be different.
  Also use the active texture mapper, not the one that was active when
  creating the dialog (leftover from modal mode).
- DownloadRegionDialog: Fix crash when switching to a map theme without texture layer.
  The problem was that d->m_textureLayer is used in the tiles count calculation. If
  there is no texture layer, the tiles count is simply 0.
- Refactor MarblePart::showDownloadRegionDialog.
  Move code to new method connectDownloadRegionDialog and provide corresponding
  method disconnectDownloadRegionDialog. As the slot invocations are completely
  unnecessary when the dialog is closed/hidden we want to have it disconnected
  when unused.
- Pass the kpart's widget as parent to DownloadRegionDialog's constructor.
- Download Region: Disconnect the expensive slots when the dialog is hidden.
- DownloadRegionDialog: Also update the lat/lon widget, but only if it is not enabled,
  to prevent that users loose their entered values.
- DownloadRegionDialog: When the selection method changes from "specify region"
  to "visible region", update the (then read only) lat/lon values.

Remaining issues:
=================
- I observed one crash when changing to a map theme which required initial creation of tiles,
  but did not yet investigate.
- Dennis Nienhüser pointed out that the maximum-tiles-count-message should be rephrased,
  that's something I'd like to fix, but not in this patch, as I see it independent of non-modal.


Diffs
-----

  /trunk/KDE/kdeedu/marble/src/lib/DownloadRegionDialog.h 1124675 
  /trunk/KDE/kdeedu/marble/src/lib/DownloadRegionDialog.cpp 1124675 
  /trunk/KDE/kdeedu/marble/src/marble_part.h 1124675 
  /trunk/KDE/kdeedu/marble/src/marble_part.cpp 1124675 

Diff: http://reviewboard.kde.org/r/3940/diff


Testing
-------

I've been using it for some hours and now it seems more or less stable with my usage patterns.


Thanks,

jmho