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

[android] - expose source layer identifier #8709

Merged
merged 2 commits into from
Apr 19, 2017
Merged

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Apr 11, 2017

Closes #8663, need to fixup some whitespacing in the .ejs template file before reviewing.
This PR add a getter for source layer identifier on the layer classes that support it. Exposing this allows to iterate all layers and check for a specfic source layer identifiers, eg. road.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Apr 11, 2017
@tobrun tobrun added this to the android-v5.1.0 milestone Apr 11, 2017
@tobrun tobrun self-assigned this Apr 11, 2017
@tobrun tobrun force-pushed the 8663-source-layer-identifier branch 5 times, most recently from 0b1bc30 to da28a2f Compare April 13, 2017 14:18
@tobrun
Copy link
Member Author

tobrun commented Apr 13, 2017

Whitespacing has been adressed + fixed a couple of nit's in the existing code.
@ivovandongen want tor review?

device-2017-04-13-143701

@@ -99,7 +108,7 @@ public class <%- camelize(type) %>Layer extends Layer {
return this;
}

<% } -%>
<% } -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: probably accidental?

@@ -144,6 +144,25 @@ namespace android {
}
}

jni::String Layer::getSourceLayer(jni::JNIEnv& env) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I set a bad example here (my apologies), but a more extensible way of doing this would be something like:

struct Evaluator {
    std::string operator()(mbgl::style::BackgroundLayer&) { return {}; }
    std::string operator()(mbgl::style::RasterLayer&) { return {}; }
    std::string operator()(mbgl::style::CustomLayer&) { return {}; }

    template <class VectorLayer>
    std::string operator()(VectorLayer& layer) {
        return layer.getSourceLayer();
    }
};

...

        std::string sourceLayerId = layer.accept(Evaluator());

maybe it's better though to clean this up separately in a pr and do the others in this class at the same time (might be quite some overlap).

Copy link
Member Author

Choose a reason for hiding this comment

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

Follow up ticket in #8758

@tobrun tobrun force-pushed the 8663-source-layer-identifier branch from da28a2f to e8b17b1 Compare April 18, 2017 11:07
@tobrun
Copy link
Member Author

tobrun commented Apr 18, 2017

Once CI approves, going to merge

@tobrun tobrun merged commit 553ec88 into master Apr 19, 2017
@tobrun tobrun deleted the 8663-source-layer-identifier branch April 19, 2017 13:05
@tobrun tobrun mentioned this pull request May 2, 2017
12 tasks
@tobrun tobrun mentioned this pull request Jun 9, 2017
12 tasks
@tobrun tobrun mentioned this pull request Jun 21, 2017
11 tasks
@tobrun tobrun mentioned this pull request Jun 30, 2017
16 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose sourceLayerIdentifier on Layer
2 participants