Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios] Add iOS bindings and example for missing icons event. #14302

Merged
merged 4 commits into from
Apr 3, 2019

Conversation

jmkiley
Copy link
Contributor

@jmkiley jmkiley commented Apr 2, 2019

Adds iOS bindings for #14253.

cc @julianrex @ansis

@jmkiley jmkiley added iOS Mapbox Maps SDK for iOS Core The cross-platform C++ core, aka mbgl labels Apr 2, 2019
@jmkiley jmkiley added this to the release-liquid milestone Apr 2, 2019
@jmkiley jmkiley self-assigned this Apr 2, 2019

void onStyleImageMissing(const std::string& img) override {
NSString *missing = [NSString stringWithCString:img.c_str() encoding:[NSString defaultCStringEncoding]];
[nativeView mapViewMissingStyleImage:missing];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to assign the returned image to a variable. Then use the returned image name to load the resource and pass this back to the renderer.

@@ -275,6 +275,8 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (void)mapView:(MGLMapView *)mapView didFinishLoadingStyle:(MGLStyle *)style;


- (NSString *)mapViewMissingStyleImage:(NSString *)image;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something along the lines of:
-(NSString *)mapView:(MGLMapView *)mapView didFailLoadingImage:(NSString *)missingImage;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...didFailToLoadImage: ?

@jmkiley jmkiley force-pushed the jmkiley-cp-14253 branch from 47fb41f to 827a83f Compare April 2, 2019 22:40
@jmkiley jmkiley changed the title [WIP, ios] Cherry-pick 14253 to release-liquid, add bindings [WIP, ios] Add iOS bindings and example for onStyleImageMissing Apr 3, 2019
@jmkiley
Copy link
Contributor Author

jmkiley commented Apr 3, 2019

I started to implement this, but I was not able to finish porting the example. I may have misunderstood what was supposed to happen, sorry!

@fabian-guerra fabian-guerra changed the base branch from release-liquid to master April 3, 2019 17:34
@fabian-guerra fabian-guerra self-assigned this Apr 3, 2019
@fabian-guerra fabian-guerra marked this pull request as ready for review April 3, 2019 18:21
@fabian-guerra fabian-guerra requested a review from a team April 3, 2019 18:21
@fabian-guerra fabian-guerra changed the title [WIP, ios] Add iOS bindings and example for onStyleImageMissing [ios] Add iOS bindings and example for missing icons event. Apr 3, 2019
@fabian-guerra fabian-guerra requested a review from ansis April 3, 2019 18:22
Copy link
Contributor

@ansis ansis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me but I think this should get another review from someone on iOS because I don't understand that part well. This is an "approve" from my side.

If possible, it might be good to expand this test to cover a case where an icon is missing but you don't want to provide it. I've added another couple layers to the style json to test this case:

Expand to see new style json ```json { "version": 8, "name": "Mapbox Streets", "sprite": "mapbox://sprites/mapbox/streets-v8", "glyphs": "mapbox://fonts/mapbox/{fontstack}/{range}.pbf", "sources": { "point": { "type": "geojson", "data": { "type": "Feature", "properties": {}, "geometry": { "type": "Point", "coordinates": [0, 0] } } }, "skip-point": { "type": "geojson", "data": { "type": "Feature", "properties": {}, "geometry": { "type": "Point", "coordinates": [0, 40] } } } }, "layers": [{ "id": "bg", "type": "background", "paint": { "background-color": "#f00" } }, { "id": "point", "type": "circle", "source": "point", "paint": { "circle-radius": 100 } }, { "id": "icon", "type": "symbol", "source": "point", "layout": { "icon-image": "missing-icon" } }, { "id": "show-skip-point", "type": "circle", "source": "skip-point", "paint": { "circle-radius": 100, "circle-color": "#fff" } }, { "id": "skip-icon", "type": "symbol", "source": "skip-point", "layout": { "icon-image": "skip-this-missing-icon" } }] } ```

My code for this in the glfw app looks like this:

    if (id != "skip-this-missing-icon") {
        map->getStyle().addImage(makeImage(id, 22, 22, 1));
    }

It should look something like this (no icon above the white circle):
image

Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iOS parts LGTM.
@ansis for whether the callback to GL is correct.

@fabian-guerra
Copy link
Contributor

This is how it's rendered on iOS with the substitute image:
cc @ansis @julianrex
missing-icon

@fabian-guerra fabian-guerra merged commit 1aa705c into master Apr 3, 2019
@fabian-guerra fabian-guerra deleted the jmkiley-cp-14253 branch April 3, 2019 19:38
fabian-guerra pushed a commit that referenced this pull request Apr 3, 2019
Added a new mapView delegate method that allows reload a style icon in case it couldn't load it from the style.

Added an iosapp test example.
fabian-guerra pushed a commit that referenced this pull request Apr 3, 2019
Added a new mapView delegate method that allows reload a style icon in case it couldn't load it from the style.

Added an iosapp test example.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants