Hook up hillshade style layers, raster DEM sources to iOS/macOS #11036
Conversation
1ec5
added
feature
iOS
😄 |
There was a problem hiding this comment.
thanks for taking this on @1ec5 – this looks good overall (but I do not have experience reading/writing ObjC) just left a few comments about the docs. not "approving" bc of my lack of familiarity w ObjC + iOS dev, so I'll leave the official ✅ to someone on the iOS team but the macoss debug app "enhance terrain" works great ✨
| implementation only supports Mapbox Terrain RGB tiles | ||
| An `MGLHillshadeStyleLayer` is a style layer that renders raster <a | ||
| href="https://en.wikipedia.org/wiki/Digital_elevation_model">digital elevation | ||
| model</a> (DEM) tiles on the map. |
There was a problem hiding this comment.
Note that currently it only supports the mapbox terrain.rgb dem encoding scheme, but we plan to add support for other like Mapzen Terrarium?
There was a problem hiding this comment.
From that Wikipedia article, I gathered that digital elevation model is a generic term, encompassing other formats like USGS DEM. Would Terrarium be considered a DEM format?
There was a problem hiding this comment.
We should avoid being overly broad here, especially if it’s suggesting something we don’t currently support.
There was a problem hiding this comment.
It’s true that we should avoid overpromising, but I think it’s also important to draw a connection to the MGLRasterDEMSource class, which does use the same terminology, since the type system doesn’t otherwise guide the developer towards using MGLHillshadeStyleLayer with MGLRasterDEMSource.
| ### Example | ||
|
|
||
| ```swift | ||
| let source = MGLRasterDEMSource(identifier: "hills", tileURLTemplates: ["https://example.com/raster-rgb/{z}/{x}/{y}.png"], options: [ |
There was a problem hiding this comment.
can we use mapbox://mapbox.terrain-rgb url here so the example actually works if copied and pasted?
There was a problem hiding this comment.
Few if any of these in-header examples work when copy-pasted. But since this one was so close to working, and since specifying a configuration URL is so much easier than specifying all the TileJSON parameters manually, I went ahead and implemented your suggestion.
| let source = MGLRasterDEMSource(identifier: "hills", tileURLTemplates: ["https://example.com/raster-rgb/{z}/{x}/{y}.png"], options: [ | ||
| .minimumZoomLevel: 9, | ||
| .maximumZoomLevel: 16, | ||
| .tileSize: 512, |
There was a problem hiding this comment.
is this assuming devicePixelRatio of 2? otherwise tilesize should be 256px
There was a problem hiding this comment.
Good catch. I blindly copied this from the raster source example without thinking about tile sizes.
1ec5
force-pushed
the
1ec5-hillshade-10991
branch
from
328b984 to
38d7753
Compare
Also set the background layer icon to mirror in right-to-left locales.
1ec5
force-pushed
the
1ec5-hillshade-10991
branch
from
38d7753 to
84cabd9
Compare
| implementation only supports Mapbox Terrain RGB tiles | ||
| An `MGLHillshadeStyleLayer` is a style layer that renders raster <a | ||
| href="https://en.wikipedia.org/wiki/Digital_elevation_model">digital elevation | ||
| model</a> (DEM) tiles on the map. |
There was a problem hiding this comment.
We should avoid being overly broad here, especially if it’s suggesting something we don’t currently support.
| } | ||
|
|
||
| // Find and remove all the style layers using those sources. | ||
| NSUInteger hillshadeIndex = NSNotFound; |
| version="1.1" | ||
| inkscape:version="0.92.2 5c3e80d, 2017-08-06" | ||
| sodipodi:docname="hillshade.svg" | ||
| inkscape:export-filename="/Users/mxn/Desktop/symbol.png" |
There was a problem hiding this comment.
Not a big deal, but Inkscape sure does toss a lot of cruft into SVGs. There are a variety of SVG optimizers/minifiers out there, if you felt so inclined.
There was a problem hiding this comment.
This would be good tail work, since all the SVGs in the platform/macos/app/resources/ folder are in Inkscape SVG format.
This PR adds the MGLHillshadeStyleLayer class implemented in #10642 to the iOS and macOS map SDKs and implements a new MGLRasterDEMSource class to provide the data used by hillshade style layers. Along the way, the runtime styling code generation script had to be modified to treat hillshade style layers like raster style layers. Now MGLHillshadeStyleLayer is a subclass of MGLForegroundStyleLayer, just like MGLRasterStyleLayer.
Finally, a Debug ‣ Enhance Terrain menu item has been added to macosapp so that you can swap out any style’s Mapbox Terrain–based hillshading layers with a single Mapbox Terrain-RGB–based hillshading layer. The new layers are inserted in the same place as the old ones, though no effort is made to match the new layer’s appearance to the old layers’ appearance. That would be an interesting research project.
Enhance!
Bloopers
The Mapbox Outdoors style has a single source that composites Mapbox Terrain with Mapbox Streets. So every layer is technically based on Mapbox Terrain. The Enhance Terrain command needed to remove only those based on the
hillshadelayer in Terrain.Fixes #10991.
/cc @mollymerp @friedbunny