<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/110194/">http://git.reviewboard.kde.org/r/110194/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 25th, 2013, 6:34 p.m. UTC, <b>Dennis Nienhüser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On April 25th, 2013, 6:42 p.m. UTC, <b>Bernhard Beschow</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
<br />
<p>- Thibaut</p>
<br />
<p>On April 25th, 2013, 4:31 p.m. UTC, Bernhard Beschow wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Bernhard Beschow.</div>
<p style="color: grey;"><i>Updated April 25, 2013, 4:31 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/lib/CMakeLists.txt <span style="color: grey">(534a7a9f221e1d414b27dc28fca2f02986e2e8b8)</span></li>
<li>src/lib/ElevationModel.cpp <span style="color: grey">(d81e58d58229dcd91fa1ca8dfdcfb1385b3e1abe)</span></li>
<li>src/lib/MarbleMap.cpp <span style="color: grey">(14ccd36404e4efb620a9022c5d3dbdb34a1365fb)</span></li>
<li>src/lib/MergedLayerDecorator.h <span style="color: grey">(b436ca39e35fde9b52ff204a3a9f448efc078e6e)</span></li>
<li>src/lib/MergedLayerDecorator.cpp <span style="color: grey">(7e73822c1e2d5eb94617be3a893d98f30e48e4f2)</span></li>
<li>src/lib/StackedTile.h <span style="color: grey">(cf9d5689ac7dc37bf1c853bbddd8b4de46ec06e3)</span></li>
<li>src/lib/StackedTile.cpp <span style="color: grey">(9b449da972913af4a8cf4603a85fc8f7228b7cee)</span></li>
<li>src/lib/StackedTileLoader.h <span style="color: grey">(b4d11a0b5446ba7dc0e82f72a369fe44310ff503)</span></li>
<li>src/lib/StackedTileLoader.cpp <span style="color: grey">(5ecc075164649093e8fefddca29a35028cac9be6)</span></li>
<li>src/lib/StackedTile_p.h <span style="color: grey">(d715e3e001dcc376fdefe1b12f93f306cb1ed27a)</span></li>
<li>src/lib/TextureTile.h <span style="color: grey">(fa80ee163536a547605e51af6384edccb10e0ee7)</span></li>
<li>src/lib/TextureTile.cpp <span style="color: grey">(80af7c64d083581ac6134bac40607dfaac246fdd)</span></li>
<li>src/lib/Tile.h <span style="color: grey">(8d445e8d4bbf971d9448a775c9c4a8fcb813296c)</span></li>
<li>src/lib/Tile.cpp <span style="color: grey">(b07cc5b8ec094aff26495932e1ee2219ad3cefda)</span></li>
<li>src/lib/TileLoader.h <span style="color: grey">(72f5eaeae3234e172c6bcff95d269d0007003559)</span></li>
<li>src/lib/TileLoader.cpp <span style="color: grey">(d459d721857f3894325f0dd1c5a398642aa99627)</span></li>
<li>src/lib/VectorTile.h <span style="color: grey">(cbddd99c1c087b87d6f780af520d548c41e4d54f)</span></li>
<li>src/lib/VectorTile.cpp <span style="color: grey">(7c72ced7dc17b4704f7276e9ef1909f97c0aa7d0)</span></li>
<li>src/lib/VectorTileMapper.h <span style="color: grey">(b6a6e4f04df540f084eaf85ba4e744315a9fbc52)</span></li>
<li>src/lib/VectorTileMapper.cpp <span style="color: grey">(6017f29b76da2f72137bfe770917b10c4a4bb685)</span></li>
<li>src/lib/layers/CMakeLists.txt <span style="color: grey">(4efe657038de29f86dcfde6a3d7b4c10da35a8c9)</span></li>
<li>src/lib/layers/TextureLayer.h <span style="color: grey">(7c5f16c500d4a638efc151c20529ba8dc55b4346)</span></li>
<li>src/lib/layers/TextureLayer.cpp <span style="color: grey">(1fe50d8db4dcfe2b8f833f19bef98ce8970f049d)</span></li>
<li>src/lib/layers/VectorTileLayer.h <span style="color: grey">(97be1493194315f49a849d524f8855785327dc9d)</span></li>
<li>src/lib/layers/VectorTileLayer.cpp <span style="color: grey">(5a5deed6f9a0e48fd5cd4cc5068dc046d1662f10)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110194/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>