[Marble-devel] Review Request: Code cleanup: Move some static methods from TileLoaderHelper directly to GeoSceneTexture and use TileId as parameter.

[Torsten Rahn]

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


I like the general direction of this patch. I wonder though whether GeoSceneTexture is the proper place for specifying a tile layout.
GeoScene classes are rather supposed to be high level glue for the rest of the framework. So creating a an API in there which has got generic tool class character doesn't fit too well imho.
Another future problem is that we need tile layout for the implementation of GeoDataImagePyramid in the future.

I'd rather put all tile layout related stuff into a TileLayout class. That one could then get reused by GeoSceneTexture as well as GeoDataImagePyramid.

 



- Torsten


On 2010-04-13 14:59:45, Bernhard Beschow wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3588/
> -----------------------------------------------------------
> 
> (Updated 2010-04-13 14:59:45)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> This patch represents milestone 1 of a series of patches to add more (server-side) storage layouts. In this series of patches, the GeoSceneTexture::StorageLayoutMode enum will be replaced by an abstract class, allowing for easy addition of server-side storage layouts. See [1] for all patches.
> 
> [1] http://github.com/shentok/marble/commits/custom-storage
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/lib/StackedTileLoader.cpp 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoader.cpp 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.h 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.cpp 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.h 1114412 
>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.cpp 1114412 
> 
> Diff: http://reviewboard.kde.org/r/3588/diff
> 
> 
> Testing
> -------
> 
> Works for me, please test.
> 
> 
> Thanks,
> 
> Bernhard
> 
>