[Marble-devel] Review Request: Separate GeoGraphicsItem from MarbleGraphicsItem and turn the latter into 2D objects

[Bernhard Beschow]

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

Review request for Marble.


Description
-------

This review request includes two patches:


1. API change: don't have GeoGraphicsItem inherit MarbleGraphicsItem; introduce GeoLabelGraphicsItem as a placeholder

GeoGraphicsItems and MarbleGraphicsItems shouldn't be mixed for the following reasons:

* paintEvent() is NOT called for real GeoGraphicsItems
Instead, paint() is called directly. As a result, "real" GeoGraphicsItems don't make use of caching.

* paint() has different semantics for ScreenGraphicsItems/AbstractDataPluginItems and GeoGraphicsItems
The GeoPainter used in "real" GeoGraphicsItems repeats items in x-direction if neccessary.
For ScreenGraphicsItems/AbstractDataPluginItems, this is done in paintEvent() presumably due to caching.

* ScreenGraphicsItems/AbstractDataPluginItems are 2D objects, whereas GeoGraphicsItems are 3D objects
ScreenGraphicsItems/AbstractDataPluginItems represent two-dimensional labels whose size() is independent of the zoom level and they always face the screen (a.k.a. billboards).
GeoGraphicsItems don't make use of size(). Instead, their size and shape changes depending on the projection.

Since AbstractDataPluginItem has coordinates(), it was derived from GeoGraphicsItem.
In order to keep a hight level of source-compatibilty, this patch introduces the placeholder class GeoLabelGraphicsItem.
It implements the methods GeoGraphicsItem used to deliver to the AbstractDataPluginItems.


2. API change: reflect in the API that MarbleGraphicsItems are now 2D objects

All MarbleGraphicsItems now take a QPainter rather than a GeoPainter, which is sufficient for 2D items.
In addition, the ViewportParams, renderPosition and GeoSceneLayer parameters have been removed from all paint() methods.

* using plain 2D API rather than GeoPainter avoids possible side effects of GeoPainter such as repeating in x-direction, which could disturb caching
* plain 2D items also shouldn't depend on the current viewport, especially on the projection
  Reacting to viewport changes is currently only possible by subclassing AbstractFloatItem and by reimplementing changeViewport(), where this method is called always before paint().
* the renderPosition and GeoSceneLayer parameters were never used and were probably overdesigned


Diffs
-----

  .reviewboardrc 285adfe926c9b544d711997e10624059252f3961 
  src/lib/AbstractDataPluginItem.h dbaeeb3b71f4f6bd649d84bdd71130d041c9104b 
  src/lib/AbstractDataPluginItem.cpp 511a11b4e23f8cf256ac849b1a5f888c6688bfe1 
  src/lib/AbstractFloatItem.cpp e54b72f05210b5ded2fda0eb33c82c4b9fef328e 
  src/lib/graphicsview/CMakeLists.txt 8d51bb841b573f5c49646965c389b593f2cf98e4 
  src/lib/graphicsview/FrameGraphicsItem.h 891512bd955ff218d17ce004e0abd4e841975c2f 
  src/lib/graphicsview/FrameGraphicsItem.cpp f91281f219d6b5141f82802eafb4fa9a2f5d563e 
  src/lib/graphicsview/GeoGraphicsItem.h 6d888c75816c4abb46771cbccb11e805d4e7b3d4 
  src/lib/graphicsview/GeoGraphicsItem.cpp 399a10a69baaac820b858f0d09931fedb548a54f 
  src/lib/graphicsview/GeoGraphicsItem_p.h 6e9657fdaa9fbfbd08a537b3c073093ae796c47a 
  src/lib/graphicsview/GeoLabelGraphicsItem.h PRE-CREATION 
  src/lib/graphicsview/GeoLabelGraphicsItem.cpp PRE-CREATION 
  src/lib/graphicsview/LabelGraphicsItem.h 187cb265549167c5c1ab02df5839f9fd4b4aa38a 
  src/lib/graphicsview/LabelGraphicsItem.cpp ca0943a6bb1e4f7ceba161dc37d9e6ee6201de5b 
  src/lib/graphicsview/MarbleGraphicsItem.h 83ca8cd5de8bf155c0eedf08b7a7ba8c306742b4 
  src/lib/graphicsview/MarbleGraphicsItem.cpp f136b7ed04305bfcfdd0953122165f84012c8a17 
  src/lib/graphicsview/MarbleGraphicsItem_p.h 2b0f50e1746ab2d852de545ee5bcdd53b1778d16 
  src/lib/graphicsview/ScreenGraphicsItem_p.h 1f84c0f013dfc90d8941d0e06890977f3df2fcce 
  src/lib/graphicsview/WidgetGraphicsItem.h d98b6320b7541077de5821e2f9f3bb02260f49c7 
  src/lib/graphicsview/WidgetGraphicsItem.cpp 0ea63460520ae590c5760fe62059e03bd3cd02af 
  src/lib/graphicsview/WidgetGraphicsItem_p.h 81eb026118a60bbea370310e8142ab00f0527b44 
  src/plugins/render/compass/CompassFloatItem.h afdad8e276b5721db598c51443b737465293cbd9 
  src/plugins/render/compass/CompassFloatItem.cpp d1b4b149ff4ba184710b4345205b950eb02a770c 
  src/plugins/render/earthquake/EarthquakeItem.h 3a23e7ab1b104ea5aaf430066ccd748a19a15ac2 
  src/plugins/render/earthquake/EarthquakeItem.cpp 305063de4f6a808cdcd88866f12429a802ae0c04 
  src/plugins/render/elevationprofilefloatitem/ElevationProfileFloatItem.h 788faaab46577b8b8816ab5312bf4498fdb4b0e3 
  src/plugins/render/elevationprofilefloatitem/ElevationProfileFloatItem.cpp eb13aee3ff654fb961a34ce79294fca43b78e330 
  src/plugins/render/elevationprofilemarker/ElevationProfileMarker.h 59f8139764a03facf6003d252463311fdc969f26 
  src/plugins/render/mapscale/MapScaleFloatItem.h b68a46b0386809166ffcc03b8ae0159908f07067 
  src/plugins/render/mapscale/MapScaleFloatItem.cpp 1c17ed05ec7c25c37ea2d29dc2b75e9b4d0b94b2 
  src/plugins/render/opendesktop/OpenDesktopItem.h 78078d91299fa899450cea25f006eac6ed7aec8a 
  src/plugins/render/opendesktop/OpenDesktopItem.cpp b7930ee3c1173bdb3331f8244ea3efaa4a068fb3 
  src/plugins/render/overviewmap/OverviewMap.h 9db547d10714a692172d4b2f1e292e9ca7bce5a1 
  src/plugins/render/overviewmap/OverviewMap.cpp bc9aea455b65381d9aef19aec70d4212186180cd 
  src/plugins/render/postalcode/PostalCodeItem.h 999f1d23e1bb3c21cf12f533f8879995fedfdf78 
  src/plugins/render/postalcode/PostalCodeItem.cpp 13702d7f3860b8244c5b3402a5c08264f475fb27 
  src/plugins/render/progress/ProgressFloatItem.h 326ae7853eccb84472b87e9db48ad8d8f21b2d89 
  src/plugins/render/progress/ProgressFloatItem.cpp 251d2402ccc35c4e27974a31dd93fd4d1db1f4fb 
  src/plugins/render/wikipedia/WikipediaItem.h 38f00a892f214819ee6760f7ab7d793785f265bd 
  src/plugins/render/wikipedia/WikipediaItem.cpp 65bdac91792b3021c8a315dac470c24fdd6730ca 

Diff: http://git.reviewboard.kde.org/r/105828/diff/


Testing
-------

Online plugins and float items still seem to be rendered properly.

Question: Should GeoLabelGraphicsItem be renamed, e.g. to BillboardGraphicsItem?


Thanks,

Bernhard Beschow

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