Encoding --> DEMEncoding, avoid if statement when unpacking elevation valuesupstream/mapzen-terrarium

11 files changed, 31 insertions, 25 deletions

diff --git a/include/mbgl/util/tileset.hpp b/include/mbgl/util/tileset.hpp
index d73b33c9c3..ed2b907647 100644
--- a/include/mbgl/util/tileset.hpp
+++ b/include/mbgl/util/tileset.hpp
@@ -14,13 +14,14 @@ namespace mbgl {
 class Tileset {
 public:
     enum class Scheme : bool { XYZ, TMS };
-    enum class Encoding : bool { Mapbox, Terrarium };
+    enum class DEMEncoding : bool { Mapbox, Terrarium };
 
     std::vector<std::string> tiles;
     Range<uint8_t> zoomRange;
     std::string attribution;
     Scheme scheme;
-    Encoding encoding;
+    // DEMEncoding is not supported by the TileJSON spec
+    DEMEncoding encoding;
     optional<LatLngBounds> bounds;
 
 
@@ -28,7 +29,7 @@ public:
             Range<uint8_t> zoomRange_ = { 0, util::DEFAULT_MAX_ZOOM },
             std::string attribution_ = {},
             Scheme scheme_ = Scheme::XYZ,
-            Encoding encoding_ = Encoding::Mapbox)
+            DEMEncoding encoding_ = DEMEncoding::Mapbox)
         : tiles(std::move(tiles_)),
           zoomRange(std::move(zoomRange_)),
           attribution(std::move(attribution_)),
diff --git a/src/mbgl/geometry/dem_data.cpp b/src/mbgl/geometry/dem_data.cpp
index d6b6ccbc6b..7fa98950ea 100644
--- a/src/mbgl/geometry/dem_data.cpp
+++ b/src/mbgl/geometry/dem_data.cpp
@@ -3,7 +3,7 @@
 
 namespace mbgl {
 
-DEMData::DEMData(const PremultipliedImage& _image, Tileset::Encoding encoding):
+DEMData::DEMData(const PremultipliedImage& _image, Tileset::DEMEncoding encoding):
     dim(_image.size.height),
     border(std::max<int32_t>(std::ceil(_image.size.height / 2), 1)),
     stride(dim + 2 * border),
@@ -13,15 +13,18 @@ DEMData::DEMData(const PremultipliedImage& _image, Tileset::Encoding encoding):
         throw std::runtime_error("raster-dem tiles must be square.");
     }
 
-    auto decodeRGB = [&] (const uint8_t r, const uint8_t g, const uint8_t b) {
-        if (encoding == Tileset::Encoding::Mapbox) {
-            return (r * 256 * 256 + g * 256 + b)/10 - 10000;
-        } else {
-            // encoding == Tileset::Encoding::Terrarium;
-            return ((r * 256 + g + b / 256) - 32768);
-        }
+    auto decodeMapbox = [] (const uint8_t r, const uint8_t g, const uint8_t b){
+        // https://www.mapbox.com/help/access-elevation-data/#mapbox-terrain-rgb
+        return (r * 256 * 256 + g * 256 + b)/10 - 10000;
+    };
+
+    auto decodeTerrarium = [] (const uint8_t r, const uint8_t g, const uint8_t b){
+        // https://aws.amazon.com/public-datasets/terrain/
+        return ((r * 256 + g + b / 256) - 32768);
     };
 
+    auto decodeRGB = encoding == Tileset::DEMEncoding::Terrarium ? decodeTerrarium : decodeMapbox;
+
     std::memset(image.data.get(), 0, image.bytes());
 
     for (int32_t y = 0; y < dim; y++) {
diff --git a/src/mbgl/geometry/dem_data.hpp b/src/mbgl/geometry/dem_data.hpp
index f6be10e685..817d3cc9c9 100644
--- a/src/mbgl/geometry/dem_data.hpp
+++ b/src/mbgl/geometry/dem_data.hpp
@@ -13,7 +13,7 @@ namespace mbgl {
 
 class DEMData {
 public:
-    DEMData(const PremultipliedImage& image, Tileset::Encoding encoding);
+    DEMData(const PremultipliedImage& image, Tileset::DEMEncoding encoding);
     void backfillBorder(const DEMData& borderTileData, int8_t dx, int8_t dy);
 
     void set(const int32_t x, const int32_t y, const int32_t value) {
diff --git a/src/mbgl/renderer/buckets/hillshade_bucket.cpp b/src/mbgl/renderer/buckets/hillshade_bucket.cpp
index 1785f48365..00b9536894 100644
--- a/src/mbgl/renderer/buckets/hillshade_bucket.cpp
+++ b/src/mbgl/renderer/buckets/hillshade_bucket.cpp
@@ -8,7 +8,7 @@ namespace mbgl {
 
 using namespace style;
 
-HillshadeBucket::HillshadeBucket(PremultipliedImage&& image_, Tileset::Encoding encoding): demdata(image_, encoding) {
+HillshadeBucket::HillshadeBucket(PremultipliedImage&& image_, Tileset::DEMEncoding encoding): demdata(image_, encoding) {
 }
 
 HillshadeBucket::HillshadeBucket(DEMData&& demdata_) : demdata(std::move(demdata_)) {
diff --git a/src/mbgl/renderer/buckets/hillshade_bucket.hpp b/src/mbgl/renderer/buckets/hillshade_bucket.hpp
index aeb9a7a7dc..5335f7ceda 100644
--- a/src/mbgl/renderer/buckets/hillshade_bucket.hpp
+++ b/src/mbgl/renderer/buckets/hillshade_bucket.hpp
@@ -17,8 +17,8 @@ namespace mbgl {
 
 class HillshadeBucket : public Bucket {
 public:
-    HillshadeBucket(PremultipliedImage&&, Tileset::Encoding encoding);
-    HillshadeBucket(std::shared_ptr<PremultipliedImage>, Tileset::Encoding encoding);
+    HillshadeBucket(PremultipliedImage&&, Tileset::DEMEncoding encoding);
+    HillshadeBucket(std::shared_ptr<PremultipliedImage>, Tileset::DEMEncoding encoding);
     HillshadeBucket(DEMData&&);
 
 
diff --git a/src/mbgl/style/conversion/tileset.cpp b/src/mbgl/style/conversion/tileset.cpp
index 76c726ee59..d15a054802 100644
--- a/src/mbgl/style/conversion/tileset.cpp
+++ b/src/mbgl/style/conversion/tileset.cpp
@@ -44,7 +44,9 @@ optional<Tileset> Converter<Tileset>::operator()(const Convertible& value, Error
     if (encodingValue) {
         optional<std::string> encoding = toString(*encodingValue);
         if (encoding && *encoding == "terrarium") {
-            result.encoding = Tileset::Encoding::Terrarium;
+            result.encoding = Tileset::DEMEncoding::Terrarium;
+        } else if (encoding && *encoding != "mapbox") {
+            error = { "invalid raster-dem encoding type - valid types are 'mapbox' and 'terrarium' " };
         }
     }
 
diff --git a/src/mbgl/tile/raster_dem_tile.hpp b/src/mbgl/tile/raster_dem_tile.hpp
index f09f094125..0c8dd75961 100644
--- a/src/mbgl/tile/raster_dem_tile.hpp
+++ b/src/mbgl/tile/raster_dem_tile.hpp
@@ -94,7 +94,7 @@ private:
     Actor<RasterDEMTileWorker> worker;
     
     uint64_t correlationID = 0;
-    Tileset::Encoding encoding;
+    Tileset::DEMEncoding encoding;
 
     // Contains the Bucket object for the tile. Buckets are render
     // objects and they get added by tile parsing operations.
diff --git a/src/mbgl/tile/raster_dem_tile_worker.cpp b/src/mbgl/tile/raster_dem_tile_worker.cpp
index 5f6a278c94..7338e578c7 100644
--- a/src/mbgl/tile/raster_dem_tile_worker.cpp
+++ b/src/mbgl/tile/raster_dem_tile_worker.cpp
@@ -10,7 +10,7 @@ RasterDEMTileWorker::RasterDEMTileWorker(ActorRef<RasterDEMTileWorker>, ActorRef
     : parent(std::move(parent_)) {
 }
 
-void RasterDEMTileWorker::parse(std::shared_ptr<const std::string> data, uint64_t correlationID, Tileset::Encoding encoding) {
+void RasterDEMTileWorker::parse(std::shared_ptr<const std::string> data, uint64_t correlationID, Tileset::DEMEncoding encoding) {
     if (!data) {
         parent.invoke(&RasterDEMTile::onParsed, nullptr, correlationID); // No data; empty tile.
         return;
diff --git a/src/mbgl/tile/raster_dem_tile_worker.hpp b/src/mbgl/tile/raster_dem_tile_worker.hpp
index d0e60342a6..5a8222bc2d 100644
--- a/src/mbgl/tile/raster_dem_tile_worker.hpp
+++ b/src/mbgl/tile/raster_dem_tile_worker.hpp
@@ -14,7 +14,7 @@ class RasterDEMTileWorker {
 public:
     RasterDEMTileWorker(ActorRef<RasterDEMTileWorker>, ActorRef<RasterDEMTile>);
 
-    void parse(std::shared_ptr<const std::string> data, uint64_t correlationID, Tileset::Encoding encoding);
+    void parse(std::shared_ptr<const std::string> data, uint64_t correlationID, Tileset::DEMEncoding encoding);
 
 private:
     ActorRef<RasterDEMTile> parent;
diff --git a/test/geometry/dem_data.test.cpp b/test/geometry/dem_data.test.cpp
index 9a59ed38df..b948aff5fe 100644
--- a/test/geometry/dem_data.test.cpp
+++ b/test/geometry/dem_data.test.cpp
@@ -17,7 +17,7 @@ auto fakeImage = [](Size s) {
 
 TEST(DEMData, Constructor) {
     PremultipliedImage image = fakeImage({16, 16});
-    DEMData demdata(image, Tileset::Encoding::Mapbox);
+    DEMData demdata(image, Tileset::DEMEncoding::Mapbox);
 
     EXPECT_EQ(demdata.dim, 16);
     EXPECT_EQ(demdata.border, 8);
@@ -29,7 +29,7 @@ TEST(DEMData, Constructor) {
 
 TEST(DEMData, RoundTrip) {
     PremultipliedImage image = fakeImage({16, 16});
-    DEMData demdata(image, Tileset::Encoding::Mapbox);
+    DEMData demdata(image, Tileset::DEMEncoding::Mapbox);
 
     demdata.set(4, 6, 255);
     EXPECT_EQ(demdata.get(4, 6), 255);
@@ -38,7 +38,7 @@ TEST(DEMData, RoundTrip) {
 TEST(DEMData, InitialBackfill) {
 
     PremultipliedImage image1 = fakeImage({4, 4});
-    DEMData dem1(image1, Tileset::Encoding::Mapbox);
+    DEMData dem1(image1, Tileset::DEMEncoding::Mapbox);
 
     bool nonempty = true;
     // checking that a 1 px border around the fake image has been populated
@@ -92,10 +92,10 @@ TEST(DEMData, InitialBackfill) {
 
 TEST(DEMData, BackfillNeighbor) {
     PremultipliedImage image1 = fakeImage({4, 4});
-    DEMData dem0(image1, Tileset::Encoding::Mapbox);
+    DEMData dem0(image1, Tileset::DEMEncoding::Mapbox);
 
     PremultipliedImage image2 = fakeImage({4, 4});
-    DEMData dem1(image2, Tileset::Encoding::Mapbox);
+    DEMData dem1(image2, Tileset::DEMEncoding::Mapbox);
 
     dem0.backfillBorder(dem1, -1, 0);
     for (int y = 0; y < 4; y++) {
diff --git a/test/tile/raster_dem_tile.test.cpp b/test/tile/raster_dem_tile.test.cpp
index 22c5948a3b..5a6f5a8c9a 100644
--- a/test/tile/raster_dem_tile.test.cpp
+++ b/test/tile/raster_dem_tile.test.cpp
@@ -62,7 +62,7 @@ TEST(RasterDEMTile, onError) {
 TEST(RasterDEMTile, onParsed) {
     RasterDEMTileTest test;
     RasterDEMTile tile(OverscaledTileID(0, 0, 0), test.tileParameters, test.tileset);
-    tile.onParsed(std::make_unique<HillshadeBucket>(PremultipliedImage({16, 16}), Tileset::Encoding::Mapbox), 0);
+    tile.onParsed(std::make_unique<HillshadeBucket>(PremultipliedImage({16, 16}), Tileset::DEMEncoding::Mapbox), 0);
     EXPECT_TRUE(tile.isRenderable());
     EXPECT_TRUE(tile.isLoaded());
     EXPECT_TRUE(tile.isComplete());