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

Ability to choose which Layers to compile #13717

Merged
merged 1 commit into from
Jan 17, 2019
Merged

Ability to choose which Layers to compile #13717

merged 1 commit into from
Jan 17, 2019

Conversation

mueschm
Copy link
Contributor

@mueschm mueschm commented Jan 10, 2019

Add two pre-processor flags for each layer that allow you to control how they will be compiled into your application. By default all layers are added and there is no change to how the SDK is currently compiled in master. This will give people who use compile the SDK by source to choose which layers they want in their build process without the need for maintaining any additional code.

LAYER_{LAYER_NAME}_DISABLE_ALL

Removes the layer completely from your application

LAYER_{LAYER_NAME}_DISABLE_RUNTIME

Adds the layer but only to the core. The goal with calling this flag RUNTIME was to allow it to both be used by Andriod and iOS so the flags would match.

Copy link
Contributor

@pozdnyakov pozdnyakov left a comment

Choose a reason for hiding this comment

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

Thanks for your patch! Few comments:

  1. We should also set annotationsEnabled to false if any of line, fill or symbol layers is disabled.
  2. Could you also update https://github.com/mapbox/mapbox-gl-native/blob/master/platform/darwin/src/MGLStyleLayerManager.mm and https://github.com/mapbox/mapbox-gl-native/blob/master/platform/default/src/mbgl/layermanager/layer_manager.cpp accordingly?

@mueschm mueschm requested a review from 1ec5 as a code owner January 15, 2019 18:37
@mueschm mueschm requested a review from a team January 15, 2019 18:37
@mueschm
Copy link
Contributor Author

mueschm commented Jan 15, 2019

@pozdnyakov I have made the suggested changes. Though was not fully clear on the parameters necessary to disable annotations, so I did this. Basically if any of the line, layer or fill layers are completely disabled then annotations is set to false. However if they are all set to core then the annotations are still enabled.

#if defined(MBGL_LAYER_LINE_DISABLE_ALL) || defined(MBGL_LAYER_SYMBOL_DISABLE_ALL) || defined(MBGL_LAYER_FILL_DISABLE_ALL)
const bool LayerManager::annotationsEnabled = false;
#else
const bool LayerManager::annotationsEnabled = true;
#endif

If that is not correct let me know and I will up date it

@mueschm
Copy link
Contributor Author

mueschm commented Jan 15, 2019

So there seems to be a test failing for node. However I cannot reproduce this locally at all. Any possible way to get some more insight on this?

@pozdnyakov
Copy link
Contributor

May I ask you to squash commits before merge?

@pozdnyakov
Copy link
Contributor

May I ask you to squash commits before merge?

To be clear, could you make a single commit out of the present 6 ones with a common description of the change (not just the commit messages of all the squashed commits)?

@pozdnyakov pozdnyakov merged commit 2446895 into mapbox:master Jan 17, 2019
@friedbunny friedbunny added iOS Mapbox Maps SDK for iOS build Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl labels Jan 22, 2019
@friedbunny friedbunny added this to the release-java milestone Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android build 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.

None yet

3 participants