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

[Bernhard Beschow]

> On 2010-05-01 11:36:07, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/lib/ServerLayout.h, line 12
> > <http://reviewboard.kde.org/r/3588/diff/2/?file=25103#file25103line12>
> >
> >     Include guards are named MARBLE_STORAGELAYOUT_H while the class is named ServerLayout already. I don't have anything against a name change but was there a reason to change it to "server"layout instead of "storage"layout? I'm just curious.

The naming of the include guards is fixed in the commit.

The reason why I changed the name to *Server*Layout is that I see a distinction between the tile layout on the server and the (local) layout of the Marble-client. While Marble basically has an (unsigned) X-Y-Z layout, many Servers implement a different - but isomorphic - layout like quad-trees or signed X-Y-Z. So instead of copying the server's layout 1:1 locally, it seems more elegant to translate between them.


> On 2010-05-01 11:36:07, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/lib/ServerLayout.h, line 26
> > <http://reviewboard.kde.org/r/3588/diff/2/?file=25103#file25103line26>
> >
> >     Hm, I feel there is a bit of documentation missing here. What is the Url provided by the first parameter? What is the Url returned? Maybe something like serverUrl, or rootDirUrl, or something even better?

Doxygen comments added in the commit.


> On 2010-05-01 11:36:07, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/lib/ServerLayout.cpp, line 29
> > <http://reviewboard.kde.org/r/3588/diff/2/?file=25104#file25104line29>
> >
> >     What does "prototype" mean in this case? :-)

This is explained in the doxygen comments. :-)


- Bernhard


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


On 2010-04-30 10:36:41, Bernhard Beschow wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3588/
> -----------------------------------------------------------
> 
> (Updated 2010-04-30 10:36:41)
> 
> 
> 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, an abstract base class will be used to create download URLs, 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/CMakeLists.txt 1121091 
>   trunk/KDE/kdeedu/marble/src/lib/ServerLayout.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/lib/ServerLayout.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/lib/StackedTileLoader.cpp 1121091 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoader.cpp 1121091 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.h 1121091 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoaderHelper.cpp 1121091 
>   trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/dgml/DgmlStorageLayoutTagHandler.cpp 1121091 
>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.h 1121091 
>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.cpp 1121091 
>   trunk/KDE/kdeedu/marble/src/tilecreator/CMakeLists.txt 1121091 
> 
> Diff: http://reviewboard.kde.org/r/3588/diff
> 
> 
> Testing
> -------
> 
> Works for me, please test.
> 
> 
> Thanks,
> 
> Bernhard
> 
>