[Differential] [Request, 846 lines] D2780: Introducing GeoSceneAbstractTileProjection for tile x/y <-> lonlat

[kossebau (Friedrich W. H. Kossebau)]

kossebau created this revision.
kossebau added a reviewer: Marble.

REVISION SUMMARY
  INITIAL WORKING PROTOTYPE - please give feedback on draft
  
  In the current code in some places hard assumptions are made about
  the projection used by the tile material. Also are calculations
  duplicated. 
  The new abstract class GeoSceneAbstractTileProjection and its currently
  two concrete subclasses GeoSceneEquirectTileProjection & GeoSceneMercatorTileProjection
  should allow to make most code agnostic of the tile projection details
  and also remove the code duplication (+ allow unit tests).
  
  GeoSceneAbstractTileProjection follows the concepts of AbstractProjection
  and has conversion methods in the interface, with the output vars as
  non-const references at the end of the argument list (for consistency, I personally
  prefer yielded stuff as return parameter, or have output vars at least being
  first in the argument list and as pointers for improved markup in the calling code).
  As the current two implementations need to know about the variables
  levelZeroColumns and levelZeroRows, I made these properties of the classes
  themselves, to avoid having the classes depend on GeoSceneTileDataset.
  Comes at the cost of data duplication and thus more complicated setup,
  but also avoids an indirection in the class methods using these values.
  For simplicity I put these properties onto GeoSceneAbstractTileProjection
  itself, to avoid another intermediate abstract subclass for the concept of
  levelZeroColumns and levelZeroRows.
  
  Any methods tileProjection() have been renamed to tileProjectionType(),
  to make clear those just return the type, not a projection object.
  
  The patch also proposes to make TileId more dump again and just be a
  property container, especially to not know about GeoSceneTileDataset.
  
  Open questions:
  Q1: Error/out-of-bounds handling?
  Where to handle bad input data, like out-of-bound values?
  Assume correct input values and just warn in API dox?
  If catched in the methods, how to signal bad data?
  I have not yet looked at the callees and their needs in detail,
  so no proposal yet.
  
  Q3: Always use Gudermannian function?
  The old TileId::toLatLonBox() used not the Gudermannian function, but
  
    atan( sinh( ( radius - y() - 1 ) / radius * M_PI ) );
  
  for calculating latitude from Mercator-based tiles.
  Are there situations where this is more correct, and should be traded
  against speed? For now kept the old calculation code in the method
  which replaces TileId::toLatLonBox() where it was used,
  GeoSceneMercatorTileProjection::geoCoordinates(..., GeoDataLatLonBox&).
  
  TODO:
  
  - unit tests for the new classes
  - Look into all places which have "if tileProjectionType() == x" to see how they can be made tile projection type agnostic as well

REPOSITORY
  rMARBLE Marble

BRANCH
  GeoSceneAbstractTileProjection

REVISION DETAIL
  https://phabricator.kde.org/D2780

AFFECTED FILES
  src/lib/marble/DownloadRegion.cpp
  src/lib/marble/MapThemeManager.cpp
  src/lib/marble/MapWizard.cpp
  src/lib/marble/MergedLayerDecorator.cpp
  src/lib/marble/MergedLayerDecorator.h
  src/lib/marble/ScanlineTextureMapperContext.cpp
  src/lib/marble/ScanlineTextureMapperContext.h
  src/lib/marble/ServerLayout.cpp
  src/lib/marble/StackedTileLoader.cpp
  src/lib/marble/StackedTileLoader.h
  src/lib/marble/TileId.cpp
  src/lib/marble/TileId.h
  src/lib/marble/VectorTileModel.cpp
  src/lib/marble/geodata/CMakeLists.txt
  src/lib/marble/geodata/handlers/dgml/DgmlProjectionTagHandler.cpp
  src/lib/marble/geodata/scene/GeoSceneAbstractTileProjection.cpp
  src/lib/marble/geodata/scene/GeoSceneAbstractTileProjection.h
  src/lib/marble/geodata/scene/GeoSceneEquirectTileProjection.cpp
  src/lib/marble/geodata/scene/GeoSceneEquirectTileProjection.h
  src/lib/marble/geodata/scene/GeoSceneMercatorTileProjection.cpp
  src/lib/marble/geodata/scene/GeoSceneMercatorTileProjection.h
  src/lib/marble/geodata/scene/GeoSceneTileDataset.cpp
  src/lib/marble/geodata/scene/GeoSceneTileDataset.h
  src/lib/marble/geodata/writers/dgml/DgmlTextureTagWriter.cpp
  src/lib/marble/layers/TextureLayer.cpp
  src/lib/marble/layers/TextureLayer.h
  tests/TestGeoSceneWriter.cpp
  tools/vectorosm-tilecreator/TileIterator.cpp
  tools/vectorosm-tilecreator/VectorClipper.cpp
  tools/vectorosm-tilecreator/VectorClipper.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kossebau, #marble
Cc: marble-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20160914/b0bf094b/attachment.html>