[Marble-devel] Review Request: New dgml tag <vectortile>, with its parser and new GeoScenes

Dennis Nienhüser earthwings at gentoo.org
Tue Aug 14 12:06:04 UTC 2012 

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


I wonder if GeoSceneTiled shouldn't be called GeoSceneTile, GeoSceneBaseTile or GeoSceneTileBase.

The inheritance of Tiled, VectorTile and TextureTile looks a bit unbalanced yet (nearly everything is defined in the base class). Are there more tags to be added to <vectortile> in the future? Otherwise it could be simplified to just have a boolean or enum member in Tiled that tells whether we have a vector or texture tile. That's merely for discussion, can go in as is in my opinion.

You'll find a couple of comments addressing improper copyright headers. I'm aware that this originates from the existing code, but it would be great if you could fix it while you're at it. Thanks.



src/lib/geodata/handlers/dgml/DgmlVectortileTagHandler.h
<http://git.reviewboard.kde.org/r/106009/#comment13582>

    KDE/Marble
    
    Can you replace this header with the shorter one we use in most other files?
    



src/lib/geodata/handlers/dgml/DgmlVectortileTagHandler.cpp
<http://git.reviewboard.kde.org/r/106009/#comment13584>

    See above



src/lib/geodata/scene/GeoSceneMap.cpp
<http://git.reviewboard.kde.org/r/106009/#comment13585>

    not needed



src/lib/geodata/scene/GeoSceneTextureTile.h
<http://git.reviewboard.kde.org/r/106009/#comment13586>

    See above



src/lib/geodata/scene/GeoSceneTextureTile.cpp
<http://git.reviewboard.kde.org/r/106009/#comment13587>

    See above



src/lib/geodata/scene/GeoSceneTextureTile.cpp
<http://git.reviewboard.kde.org/r/106009/#comment13594>

    This seems to be used as an alternative way to determine the class type to differentiate between VectorTile and TextureTile. What about instead using an enum TileType { TextureTile, VectorTile } defined in TextureTiled?
    



src/lib/geodata/scene/GeoSceneTiled.h
<http://git.reviewboard.kde.org/r/106009/#comment13588>

    See above



src/lib/geodata/scene/GeoSceneTiled.h
<http://git.reviewboard.kde.org/r/106009/#comment13593>

    Can this one move down to TextureTile?



src/lib/geodata/scene/GeoSceneTiled.h
<http://git.reviewboard.kde.org/r/106009/#comment13592>

    Can this one move down to TextureTile?



src/lib/geodata/scene/GeoSceneTiled.cpp
<http://git.reviewboard.kde.org/r/106009/#comment13589>

    See above



src/lib/geodata/scene/GeoSceneVectorTile.h
<http://git.reviewboard.kde.org/r/106009/#comment13590>

    See above



src/lib/geodata/scene/GeoSceneVectorTile.cpp
<http://git.reviewboard.kde.org/r/106009/#comment13591>

    See above


- Dennis Nienhüser


On Aug. 14, 2012, 11:24 a.m., Ander Pijoan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106009/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2012, 11:24 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> For GSoC 2012 vector tile rendering, a new tag <vectortile> has been created for the dgml format and the handler for it in the dgml parser. In te begining it was thought to call it <vectorTile> as some composed words tags have also camel casing. But through the code this tile is compared with the backend="vectortile" .tolower() tag and it didn't match vectortile != vectorTile. So for consistency the tag will be <vectortile>.
> 
> Now that there are two possible tiled layers (Image tile layers with <texture> tag and Vector tile layers with <vectortile> tag) GeoSceneTexture has been turned to GeoSceneTextureTile which extends a GeoSceneTiled abstract class. Also a GeoSceneVectorTile class has been created.
> 
> GeoSceneTiled stores all the common data for layers containing tiles and then GeoSceneTextureTile and GeoSceneVectorTile can manage specific data related to them. Currently booth have the same dgml structure so this two classes have no specific attribute but they are needed afterwards for MarbleMap to know if its a VectorTile or a ImageTile layer.
> 
> 
> Diffs
> -----
> 
>   src/lib/geodata/handlers/dgml/DgmlAuxillaryDictionary.h 54693e9 
>   src/lib/geodata/handlers/dgml/DgmlAuxillaryDictionary.cpp d1452ae 
>   src/lib/geodata/handlers/dgml/DgmlBlendingTagHandler.cpp 6570a6e 
>   src/lib/geodata/handlers/dgml/DgmlDownloadPolicyTagHandler.cpp 765be23 
>   src/lib/geodata/handlers/dgml/DgmlDownloadUrlTagHandler.cpp 5fe2251 
>   src/lib/geodata/handlers/dgml/DgmlElementDictionary.h 1171392 
>   src/lib/geodata/handlers/dgml/DgmlElementDictionary.cpp c2bba42 
>   src/lib/geodata/handlers/dgml/DgmlInstallMapTagHandler.cpp d9e5e12 
>   src/lib/geodata/handlers/dgml/DgmlProjectionTagHandler.cpp d4130bf 
>   src/lib/geodata/handlers/dgml/DgmlSourceDirTagHandler.cpp 129799a 
>   src/lib/geodata/handlers/dgml/DgmlStorageLayoutTagHandler.cpp 7619175 
>   src/lib/geodata/handlers/dgml/DgmlTextureTagHandler.cpp 6d033d2 
>   src/lib/geodata/handlers/dgml/DgmlTileSizeTagHandler.cpp 07ab101 
>   src/lib/geodata/handlers/dgml/DgmlVectortileTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/dgml/DgmlVectortileTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/dgml/DgmlVisibleTagHandler.cpp a6da77d 
>   src/lib/geodata/parser/GeoSceneTypes.h 48e90e2 
>   src/lib/geodata/parser/GeoSceneTypes.cpp b076509 
>   src/lib/geodata/scene/GeoSceneMap.cpp ffa2006 
>   src/lib/geodata/scene/GeoSceneTexture.h a5d97f2 
>   src/lib/geodata/scene/GeoSceneTexture.cpp 14cb61d 
>   src/lib/geodata/scene/GeoSceneTextureTile.h PRE-CREATION 
>   src/lib/geodata/scene/GeoSceneTextureTile.cpp PRE-CREATION 
>   src/lib/geodata/scene/GeoSceneTiled.h PRE-CREATION 
>   src/lib/geodata/scene/GeoSceneTiled.cpp PRE-CREATION 
>   src/lib/geodata/scene/GeoSceneVectorTile.h PRE-CREATION 
>   src/lib/geodata/scene/GeoSceneVectorTile.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106009/diff/
> 
> 
> Testing
> -------
> 
> Testing done and works OK.
> 
> 
> Thanks,
> 
> Ander Pijoan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20120814/85a5a892/attachment-0001.html>

More information about the Marble-devel mailing list