<table><tr><td style="">kossebau created this revision.<br />kossebau added a reviewer: Marble.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D2780" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>REVISION SUMMARY</strong><div><p>INITIAL WORKING PROTOTYPE - please give feedback on draft</p>

<p>In the current code in some places hard assumptions are made about<br />
the projection used by the tile material. Also are calculations<br />
duplicated. <br />
The new abstract class GeoSceneAbstractTileProjection and its currently<br />
two concrete subclasses GeoSceneEquirectTileProjection & GeoSceneMercatorTileProjection<br />
should allow to make most code agnostic of the tile projection details<br />
and also remove the code duplication (+ allow unit tests).</p>

<p>GeoSceneAbstractTileProjection follows the concepts of AbstractProjection<br />
and has conversion methods in the interface, with the output vars as<br />
non-const references at the end of the argument list (for consistency, I personally<br />
prefer yielded stuff as return parameter, or have output vars at least being<br />
first in the argument list and as pointers for improved markup in the calling code).<br />
As the current two implementations need to know about the variables<br />
levelZeroColumns and levelZeroRows, I made these properties of the classes<br />
themselves, to avoid having the classes depend on GeoSceneTileDataset.<br />
Comes at the cost of data duplication and thus more complicated setup,<br />
but also avoids an indirection in the class methods using these values.<br />
For simplicity I put these properties onto GeoSceneAbstractTileProjection<br />
itself, to avoid another intermediate abstract subclass for the concept of<br />
levelZeroColumns and levelZeroRows.</p>

<p>Any methods tileProjection() have been renamed to tileProjectionType(),<br />
to make clear those just return the type, not a projection object.</p>

<p>The patch also proposes to make TileId more dump again and just be a<br />
property container, especially to not know about GeoSceneTileDataset.</p>

<p>Open questions:<br />
Q1: Error/out-of-bounds handling?<br />
Where to handle bad input data, like out-of-bound values?<br />
Assume correct input values and just warn in API dox?<br />
If catched in the methods, how to signal bad data?<br />
I have not yet looked at the callees and their needs in detail,<br />
so no proposal yet.</p>

<p>Q3: Always use Gudermannian function?<br />
The old TileId::toLatLonBox() used not the Gudermannian function, but</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">atan( sinh( ( radius - y() - 1 ) / radius * M_PI ) );</pre></div>

<p>for calculating latitude from Mercator-based tiles.<br />
Are there situations where this is more correct, and should be traded<br />
against speed? For now kept the old calculation code in the method<br />
which replaces TileId::toLatLonBox() where it was used,<br />
GeoSceneMercatorTileProjection::geoCoordinates(..., GeoDataLatLonBox&).</p>

<p>TODO:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">unit tests for the new classes</li>
<li class="remarkup-list-item">Look into all places which have "if tileProjectionType() == x" to see how they can be made tile projection type agnostic as well</li>
</ul></div></div><br /><div><strong>REPOSITORY</strong><div><div>rMARBLE Marble</div></div></div><br /><div><strong>BRANCH</strong><div><div>GeoSceneAbstractTileProjection</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2780" rel="noreferrer">https://phabricator.kde.org/D2780</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>src/lib/marble/DownloadRegion.cpp<br />
src/lib/marble/MapThemeManager.cpp<br />
src/lib/marble/MapWizard.cpp<br />
src/lib/marble/MergedLayerDecorator.cpp<br />
src/lib/marble/MergedLayerDecorator.h<br />
src/lib/marble/ScanlineTextureMapperContext.cpp<br />
src/lib/marble/ScanlineTextureMapperContext.h<br />
src/lib/marble/ServerLayout.cpp<br />
src/lib/marble/StackedTileLoader.cpp<br />
src/lib/marble/StackedTileLoader.h<br />
src/lib/marble/TileId.cpp<br />
src/lib/marble/TileId.h<br />
src/lib/marble/VectorTileModel.cpp<br />
src/lib/marble/geodata/CMakeLists.txt<br />
src/lib/marble/geodata/handlers/dgml/DgmlProjectionTagHandler.cpp<br />
src/lib/marble/geodata/scene/GeoSceneAbstractTileProjection.cpp<br />
src/lib/marble/geodata/scene/GeoSceneAbstractTileProjection.h<br />
src/lib/marble/geodata/scene/GeoSceneEquirectTileProjection.cpp<br />
src/lib/marble/geodata/scene/GeoSceneEquirectTileProjection.h<br />
src/lib/marble/geodata/scene/GeoSceneMercatorTileProjection.cpp<br />
src/lib/marble/geodata/scene/GeoSceneMercatorTileProjection.h<br />
src/lib/marble/geodata/scene/GeoSceneTileDataset.cpp<br />
src/lib/marble/geodata/scene/GeoSceneTileDataset.h<br />
src/lib/marble/geodata/writers/dgml/DgmlTextureTagWriter.cpp<br />
src/lib/marble/layers/TextureLayer.cpp<br />
src/lib/marble/layers/TextureLayer.h<br />
tests/TestGeoSceneWriter.cpp<br />
tools/vectorosm-tilecreator/TileIterator.cpp<br />
tools/vectorosm-tilecreator/VectorClipper.cpp<br />
tools/vectorosm-tilecreator/VectorClipper.h</div></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>kossebau, Marble<br /><strong>Cc: </strong>marble-devel<br /></div>