[Marble-devel] Review Request 118618: Render status property and notification

[Dennis Nienhüser]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118618/
-----------------------------------------------------------

Review request for Marble, Friedrich W. H. Kossebau and Torsten Rahn.


Repository: marble


Description
-------

Data needed for rendering (texture tiles, search results, routes, ...) is often download from the Internet and Marble renders preliminary results while waiting for the data to arrive. The preliminary results are created in various ways, e.g. using old (outdated) data, higher level tiles or partial data. We inform the user to some extend about pending data (e.g. progress bar for tile downloads, busy indicator for routes), but there is no sane programmatical way at the moment to determine whether all data has arrived yet.

The patch adds an enum property RenderStatus { Complete, WaitingForUpdate, WaitingForData, Incomplete } to MarbleMap and MarbleWidget along with a renderStatusChanged() notification signal. LayerManager maintains that status value. Internally it delegates the status determination to all layers and uses a tree-like data structure to record all details. Here's a string-representation of that tree (with a couple of entries removed for readability):

Marble: Waiting for data
  Stars: Complete
  Atmosphere: Complete
  Anonymous renderer: Complete
  Textures: Waiting for data
    Stacked Tiles: Waiting for data
      Tile 13/4280/2403: Complete
        mapnik_data: Complete
        hillshading: Complete
      Tile 13/4280/2404: Complete
        mapnik_data: Complete
        hillshading: Complete
      Tile 13/4280/2405: Waiting for data
        mapnik_data: Waiting for data
        hillshading: Waiting for data
      Tile 13/4281/2402: Complete
        mapnik_data: Complete
        hillshading: Complete
     [...]
  Coordinate Grid: Complete
  GeoGraphicsScene: Complete
  [...]

To keep the patch size small for a first feedback round the status determination is not implemented yet for all layers that depend on http requests. Likewise I didn't change tools and examples yet to make use of it:
- maptheme-preview-image can wait for renderStatus to become Complete before taking the screenshot (no more hacks needed)
- examples/cpp/animation-video can wait for all tiles to arrive before rendering video frames (no more blurry tiles in videos)
- calligra can generate sane (preview) images

Besides finishing the implementation for various layers depending on network queries and making use of the status property in tools and examples, there are some nice possible follow-ups:
- merge the fps determiniation (runtimeTrace) into renderStatus. Fits well and is calculated at pretty much the same places
- add a Layers panel as a tool for developers (can be hidden by default and activated by a command line option), which uses a QTreeView to visualize layer order, runtime, and status similar to the string output above

Things you might want to comment on:
- Shall the enum RenderStatus be defined in MarbleGlobal.h? I went for that to keep further #includes out of MarbleWidget/MarbleMap
- Is the detail scope fine? MarbleWidget presents just a summary as an enum value, LayerManager has the full detail tree (see above), but does not expose it via 3rd-party API yet. I went for that as I don't think 3rd-party code is really interested in those details, but will just poll/wait for renderStatusChanged(Complete).
- Is merging runtimeTrace into renderStatus a good idea?

I'll treat no feedback as "go ahead" :-P


Diffs
-----

  src/lib/marble/CMakeLists.txt 395e8bf 
  src/lib/marble/LayerInterface.h d8d9ee9 
  src/lib/marble/LayerInterface.cpp c3629de 
  src/lib/marble/LayerManager.h fa609f3 
  src/lib/marble/LayerManager.cpp 66eba7f 
  src/lib/marble/MarbleGlobal.h 53d375c 
  src/lib/marble/MarbleMap.h e7ec453 
  src/lib/marble/MarbleMap.cpp 0e1c70d 
  src/lib/marble/MarbleWidget.h e8148a9 
  src/lib/marble/MarbleWidget.cpp d58db8e 
  src/lib/marble/MergedLayerDecorator.h 5b2be58 
  src/lib/marble/MergedLayerDecorator.cpp b187dc2 
  src/lib/marble/RenderPlugin.h 11f2829 
  src/lib/marble/RenderPlugin.cpp 87989f4 
  src/lib/marble/StackedTileLoader.h 6fa8cd4 
  src/lib/marble/StackedTileLoader.cpp 1dfe1a0 
  src/lib/marble/layers/FogLayer.h 8d02bc7 
  src/lib/marble/layers/FogLayer.cpp 24a1325 
  src/lib/marble/layers/GeometryLayer.h f868f26 
  src/lib/marble/layers/GeometryLayer.cpp daea513 
  src/lib/marble/layers/GroundLayer.h 32568b3 
  src/lib/marble/layers/GroundLayer.cpp 8c43076 
  src/lib/marble/layers/MarbleSplashLayer.h f0442ad 
  src/lib/marble/layers/MarbleSplashLayer.cpp e2647f3 
  src/lib/marble/layers/PlacemarkLayer.h 7abc270 
  src/lib/marble/layers/PlacemarkLayer.cpp 863fd94 
  src/lib/marble/layers/PopupLayer.h c470def 
  src/lib/marble/layers/PopupLayer.cpp 11ba9b3 
  src/lib/marble/layers/TextureLayer.h 28bfb65 
  src/lib/marble/layers/TextureLayer.cpp 06fa006 
  src/lib/marble/routing/RoutingLayer.h 86d4244 
  src/lib/marble/routing/RoutingLayer.cpp 9840d7d 

Diff: https://git.reviewboard.kde.org/r/118618/diff/


Testing
-------


Thanks,

Dennis Nienhüser

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