<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123306/">https://git.reviewboard.kde.org/r/123306/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As discussed with Adam on IRC: nice patch :)
The best solution seems to be what we had in a prior implementation already (ca. Marble < 1.4): partWidth and partHeight should always at least match 1 (and never get down to 0) so that in the worst case where no bigger portion of a different level is available the least tile level is chose (likely level zero) and the single pixel in the right spot is enlarged to cover the full extent of the desired tile. So something like:
int const partWidth = qMax(1, textureLayer->tileSize().width() >> deltaLevel);
int const partHeight = qMax(1, textureLayer->tileSize().height() >> deltaLevel);
.. if that works as intended.</p></pre>
<br />
<p>- Torsten Rahn</p>
<br />
<p>On April 9th, 2015, 9:40 vorm. UTC, Adam Dabrowski wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Adam Dabrowski.</div>
<p style="color: grey;"><i>Updated April 9, 2015, 9:40 vorm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
marble
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QDoesn't change functionality.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does clarify the code as flow was very non-intuitive previously (especially in case when deltaLevel > 8 for tileSize = 256).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">if/when the scaledLowerLevelTile "for" loop reached zoom level where 2^deltaLevel > tileSize, a peculiar thing was happening.
In this situation, in part "// which rect to scale?", the image was copied first from 0 sized-image. Calling the QImage::copy function with null rectangle (or 0 width and 0 height) results in copying a whole image instead. As a result, instead of level_N tile, a level_N-9 (or later) tile was returned unscaled, resulting in curious displays (i.e. multiple copies of the same tile image).
While for textures that are downloadable this was a temporary case, offline textures would just stop there and display this curious images. Additionally it was very unclear and took me some time to figure out what's happening.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This version does clearly what previously was done due to specific QImage::copy behaviour.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Let's discuss if this behaviour is desired (or we want to display a full white opaque tile or a tile with a loading icon if there are download urls etc.).</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested with zooming in and out and two layers of texture as well as one. No change noticed (as intended).</p></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/marble/TileLoader.cpp <span style="color: grey">(01994eb)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123306/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>