Adds support for Mapzen terrain #6024

ibesora · 2018-01-19T15:42:03Z

Launch Checklist

briefly describe the changes in this PR
Implements Support additional encoding formulas for RasterDEM sources #5824 and adds support for the Mapzen terrarium terrain format by adding a new encoding parameter to the raster-dem source type that selects which unpack function should be used.
Also adds a new debug map that shows this new terrain format.
write tests for all new functionality
document any changes to public APIs
Adds a new parameter called encoding to the raster-dem source type. Defaults to mapbox to use the mapbox terrain encoding
post benchmark scores
manually test the debug page
Works as expected

Update fork to last version

andrewharvey · 2018-01-21T01:47:15Z

src/style-spec/reference/v8.json

@@ -231,7 +231,7 @@
      "type": "enum",
      "values": {
        "raster-dem": {
-            "doc": "A raster DEM source using Mapbox Terrain RGB"
+            "doc": "A raster DEM source"


I think we should still leave in the Terrain RGB part as it's still limited to sources of that type. For example it can't handle single band 16bit GeoTIFF tiles which are raster DEMs.

Corrected it in the new commit.
I also expanded the declaration of the encoding parameter to explicitly tell the supported values

…r one

mollymerp

Thank you @ibesora for the contribution! We definitely want to support other encoding schemes for our raster-dem type. We have not yet discussed as a team how customizable we want our raster-dem encoding to be but this is a step in the right direction!

Due to the recent Mapzen shutdown, I think we should link to the AWS documentation and maybe call the encoding type "terrarium" since that's the only format of theirs we support: https://aws.amazon.com/public-datasets/terrain/

Also, I don't think we need a separate debug page for this – maybe you could add a toggle on the existing hillshade.html page to use the mapzen tiles instead (and remove your mapzen key and probably use the AWS endpoint)

mollymerp · 2018-01-22T23:10:26Z

src/data/dem_data.js

-                level.set(x, y, this.scale * ((pixels[j] * 256 * 256 + pixels[j + 1] * 256.0 + pixels[j + 2]) / 10.0 - 10000.0));
-            }
-        }
+        this.unpackData(level, data, encoding);


needs to be data.data

mollymerp · 2018-01-22T23:11:55Z

src/style-spec/reference/v8.json

@@ -272,6 +272,10 @@
      "type": "string",
      "doc": "Contains an attribution to be displayed when the map is shown to a user."
    },
+    "encoding": {
+      "type": "string",


this should be "type": "enum" with a values object listing the possible types.

mollymerp · 2018-01-22T23:17:27Z

src/source/raster_dem_tile_source.js

    constructor(id: string, options: RasterDEMSourceSpecification, dispatcher: Dispatcher, eventedParent: Evented) {
        super(id, options, dispatcher, eventedParent);
        this.type = 'raster-dem';
        this.maxzoom = 22;
        this._options = util.extend({}, options);
+        this.encoding = "mapbox";


options.encoding || "mapbox";

mollymerp · 2018-01-22T23:18:52Z

src/data/dem_data.js

+            for (let x = 0; x < level.dim; x++) {
+                const i = y * level.dim + x;
+                const j = i * 4;
+                // decoding per https://mapzen.com/documentation/terrain-tiles/formats/#types-of-terrain-tiles


link to AWS docs

…arium

ibesora · 2018-01-23T10:13:07Z

Hi @mollymerp .
When looking at your fixes I saw that I had not pushed my last commit that corrected some of the things you told me :'(
I've pushed my changes, added the AWS endpoint, changed the encoding type to an enum and added a toggle to the debug page

mollymerp

thanks for the updates @ibesora – made a couple more small comments!

mollymerp · 2018-01-27T00:06:15Z

src/source/raster_dem_tile_source.js

@@ -15,11 +15,14 @@ import type {Callback} from '../types/callback';


 class RasterDEMTileSource extends RasterTileSource implements Source {
+    encoding: string;


you can explicitly type this as encoding: "mapbox" | "terrarium";

mollymerp · 2018-01-27T00:06:20Z

src/data/dem_data.js

@@ -95,6 +87,40 @@ class DEMData {
        this.loaded = true;
    }

+    unpackData(level: Level, pixels: Uint8Array | Uint8ClampedArray, encoding: string) {


to reduce code duplication, you could refactor to something like

unpackMapbox(x, y, r, g, b) { level.set(x, y, this.scale * ((r * 256 * 256 +g * 256.0 + b) / 10.0 - 10000.0)); } unpackTerrarium(x, y, r, g, b) { level.set(x, y, this.scale * ((r * 256 + g+ b / 256) - 32768.0)); } const unpackFunctions = {"mapbox": unpackMapbox, "terrarium": unpackTerrarium}; const unpack = unpackFunctions[encoding]; for (let y = 0; y < level.dim; y++) { for (let x = 0; x < level.dim; x++) { const i = y * level.dim + const j = i * 4; unpack(x, y, pixels[j], pixels[j+1], pixels[j+2]); } }

mollymerp · 2018-01-27T00:06:59Z

src/source/worker_source.js

@@ -28,7 +28,8 @@ export type WorkerTileParameters = TileParameters & {

 export type WorkerDEMTileParameters = TileParameters & {
    coord: { z: number, x: number, y: number, w: number },
-    rawImageData: RGBAImage
+    rawImageData: RGBAImage,
+    encoding: string


explicitly type for enum values (as above)

mollymerp · 2018-01-27T00:08:01Z

src/style-spec/reference/v8.json

+          "doc": "Terrarium format PNG tiles. See https://aws.amazon.com/es/public-datasets/terrain/ for more info."
+        },
+        "mapbox": {
+          "doc": "Mapbox Terrain RGB tiles. See https://www.mapbox.com/blog/terrain-rgb/ for more info."


I just found out we have official documentation for this now, so we can link to https://www.mapbox.com/help/access-elevation-data/#mapbox-terrain-rgb

…arium

…thout params

ibesora · 2018-01-29T11:19:17Z

Hi again @mollymerp I've just pushed some changes that incorporate your comments.

mollymerp

Just a couple of small nits that I missed earlier, but I think this is almost ready to merge!
Sorry about the delay in following up.

mollymerp · 2018-02-07T00:58:13Z

src/data/dem_data.js

@@ -57,22 +57,14 @@ class DEMData {
        this.loaded = !!data;
    }

-    loadFromImage(data: RGBAImage) {
+    loadFromImage(data: RGBAImage, encoding: ?string) {


this typing can also be "mapbox | "terrarium" – sorry I missed it earlier!

mollymerp · 2018-02-07T00:59:09Z

src/data/dem_data.js

@@ -95,6 +87,26 @@ class DEMData {
        this.loaded = true;
    }

+    unpackMapbox(r: number, g: number, b: number) {


I think we can use _unpack* for these functions to signify that they're private to the DEMData class

mollymerp · 2018-02-08T01:31:10Z

@ibesora thanks for your patience and great work here! to save you the trouble I have rebased and fixed conflicts and added some tests. I had to push to a new branch so I've made a PR for that, but I think this is good to go overall! 👉 #6110

mollymerp · 2018-02-08T19:03:17Z

closing here in favor of #6110

Isaac Besora and others added 4 commits June 27, 2017 15:40

Small correction on the Marker documentation

b73833c

Merge pull request #4 from mapbox/master

eb8d6a0

Update fork to last version

Merge remote-tracking branch 'mapbox/master'

dac8bed

Added the encoding parameter to the raster-dem source type

5c6189a

andrewharvey reviewed Jan 21, 2018

View reviewed changes

ibesora added 2 commits January 21, 2018 10:33

Changed the raster-dem description and expanded the encoding paramete…

1a398f2

…r one

Changed the description in the style spec reference

4d4a4c3

mollymerp reviewed Jan 22, 2018

View reviewed changes

ibesora added 2 commits January 23, 2018 11:07

Changes proposed by @mollymerp

d427624

Merge remote-tracking branch 'origin/mapzen-terrain' into mapzen-terr…

11b58d8

…arium

ibesora added 3 commits January 25, 2018 16:11

Fixes linter errors

92f0c74

Changed ts type

2b5b4ce

Merge branch 'master-mapbox' into mapzen-terrarium

cdee3b3

mollymerp reviewed Jan 27, 2018

View reviewed changes

ibesora added 6 commits January 27, 2018 14:35

Proposed changes

aa01a65

Bug

381fcff

Merge branch 'master-mapbox' into mapzen-terrarium

b9f7977

Merge remote-tracking branch 'origin/mapzen-terrain' into mapzen-terr…

cfc3b46

…arium

Added a default value when the RasterDEMTileWorkerSource is called wi…

fdfe9b3

…thout params

Adds default value to pass unit tests

2a16718

mollymerp mentioned this pull request Feb 7, 2018

add maxzoom uniform to support external tilesets mapbox/mapbox-gl-native#11134

Merged

mollymerp reviewed Feb 7, 2018

View reviewed changes

Explicit encoding type and _ to private functions

ec6562f

mollymerp mentioned this pull request Feb 8, 2018

Adds support for mapzen terrain #6110

Merged

mollymerp closed this Feb 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for Mapzen terrain #6024

Adds support for Mapzen terrain #6024

ibesora commented Jan 19, 2018

andrewharvey Jan 21, 2018

ibesora Jan 21, 2018

mollymerp left a comment •

edited

Loading

mollymerp Jan 22, 2018

mollymerp Jan 22, 2018

mollymerp Jan 22, 2018

mollymerp Jan 22, 2018

ibesora commented Jan 23, 2018

mollymerp left a comment

mollymerp Jan 27, 2018

mollymerp Jan 27, 2018

mollymerp Jan 27, 2018

mollymerp Jan 27, 2018

ibesora commented Jan 29, 2018

mollymerp left a comment

mollymerp Feb 7, 2018

ibesora Feb 7, 2018

mollymerp Feb 7, 2018

ibesora Feb 7, 2018

mollymerp commented Feb 8, 2018

mollymerp commented Feb 8, 2018

		@@ -15,11 +15,14 @@ import type {Callback} from '../types/callback';


		class RasterDEMTileSource extends RasterTileSource implements Source {
		encoding: string;

Adds support for Mapzen terrain #6024

Adds support for Mapzen terrain #6024

Conversation

ibesora commented Jan 19, 2018

Launch Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollymerp left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibesora commented Jan 23, 2018

mollymerp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibesora commented Jan 29, 2018

mollymerp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollymerp commented Feb 8, 2018

mollymerp commented Feb 8, 2018

mollymerp left a comment •

edited

Loading