[Marble-devel] Review Request 110194: Remove VectorTileLayer

[Bernhard Beschow]

> On April 25, 2013, 6:34 p.m., Dennis Nienhüser wrote:
> > A 'ship it' with mixed feelings given that it's a vital part of one last years GSoC projects. However I see your point about code quality and lack of use/maintenance and agree with your conclusion. Wouldn't mind to see another patch to prove point three though :-P
> > 
> > In my opinion we should learn from this and similar GSoC experiences and do organizational changes to prevent it. That's off topic here though.
> 
> Bernhard Beschow wrote:
>     I think that the vector tiles could be added as documents to the GeoDataTreeModel as they become visible and be removed again when they disappear. This could be done by a new layer which determines the set of visible tiles based on the view's bounding box. The biggest challenge I see here is to maintain the lifecycle of the tiles inside the GeoDataTreeModel.
> 
> Thibaut Gridel wrote:
>     Hi, you are right that it's a burden to maintain unused code.
>     I think however that the effort should not be discarded on the base that no theme currently uses it.
>     Therefore i don't think this removal patch would solve the need or be nice to past working code.
>     It's also sad that the exercise has not been completed post GSoC.
>     
>     Possible remedies (no priorities or effort related):
>     - lets define the work to be made in order to put this on track (Bernard you seem to have ideas there, can we use them and break this into tasks?)
>     - should the code simply be simplified and/or continued in a separate branch for later inclusion?
>     - Could the map theme simply be added as an "experimental theme" in the site, so that we can have some exposure for it?

Yeah, I agree that it's a pity if we removed that code, particularely because it was a vital part of a last year's GSoC project. However, I don't see much code in the current solution that could be reused in an understandable and clean new implementation. The current code is just so complex and touches so many unrelated classes that I prefer to start from scratch here. In particular, this code gets in my way too often for my taste when working on the texture mapping stuff. Therefore, I prefer it to be removed. Given that this feature is currently unused, I think that user's won't miss it.


- Bernhard


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


On April 25, 2013, 4:31 p.m., Bernhard Beschow wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110194/
> -----------------------------------------------------------
> 
> (Updated April 25, 2013, 4:31 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> Reasons:
> * lack of avialable map themes, so the VectorTileLayer is currently unused in Marble
> * causes high maintainance effort when working on texture rendering stack due to its tight coupling while having very different nature
> * could be implemented much easier to provide the same features
> 
> 
> Diffs
> -----
> 
>   src/lib/CMakeLists.txt 534a7a9f221e1d414b27dc28fca2f02986e2e8b8 
>   src/lib/ElevationModel.cpp d81e58d58229dcd91fa1ca8dfdcfb1385b3e1abe 
>   src/lib/MarbleMap.cpp 14ccd36404e4efb620a9022c5d3dbdb34a1365fb 
>   src/lib/MergedLayerDecorator.h b436ca39e35fde9b52ff204a3a9f448efc078e6e 
>   src/lib/MergedLayerDecorator.cpp 7e73822c1e2d5eb94617be3a893d98f30e48e4f2 
>   src/lib/StackedTile.h cf9d5689ac7dc37bf1c853bbddd8b4de46ec06e3 
>   src/lib/StackedTile.cpp 9b449da972913af4a8cf4603a85fc8f7228b7cee 
>   src/lib/StackedTileLoader.h b4d11a0b5446ba7dc0e82f72a369fe44310ff503 
>   src/lib/StackedTileLoader.cpp 5ecc075164649093e8fefddca29a35028cac9be6 
>   src/lib/StackedTile_p.h d715e3e001dcc376fdefe1b12f93f306cb1ed27a 
>   src/lib/TextureTile.h fa80ee163536a547605e51af6384edccb10e0ee7 
>   src/lib/TextureTile.cpp 80af7c64d083581ac6134bac40607dfaac246fdd 
>   src/lib/Tile.h 8d445e8d4bbf971d9448a775c9c4a8fcb813296c 
>   src/lib/Tile.cpp b07cc5b8ec094aff26495932e1ee2219ad3cefda 
>   src/lib/TileLoader.h 72f5eaeae3234e172c6bcff95d269d0007003559 
>   src/lib/TileLoader.cpp d459d721857f3894325f0dd1c5a398642aa99627 
>   src/lib/VectorTile.h cbddd99c1c087b87d6f780af520d548c41e4d54f 
>   src/lib/VectorTile.cpp 7c72ced7dc17b4704f7276e9ef1909f97c0aa7d0 
>   src/lib/VectorTileMapper.h b6a6e4f04df540f084eaf85ba4e744315a9fbc52 
>   src/lib/VectorTileMapper.cpp 6017f29b76da2f72137bfe770917b10c4a4bb685 
>   src/lib/layers/CMakeLists.txt 4efe657038de29f86dcfde6a3d7b4c10da35a8c9 
>   src/lib/layers/TextureLayer.h 7c5f16c500d4a638efc151c20529ba8dc55b4346 
>   src/lib/layers/TextureLayer.cpp 1fe50d8db4dcfe2b8f833f19bef98ce8970f049d 
>   src/lib/layers/VectorTileLayer.h 97be1493194315f49a849d524f8855785327dc9d 
>   src/lib/layers/VectorTileLayer.cpp 5a5deed6f9a0e48fd5cd4cc5068dc046d1662f10 
> 
> Diff: http://git.reviewboard.kde.org/r/110194/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bernhard Beschow
> 
>

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