[core] Return source and layer ownership #7014
Conversation
When a source or layer is removed transfer ownership back to the caller so it can (optionally) take it.
|
@boundsj, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jfirebaugh, @brunoabinader and @kkaefer to be potential reviewers. |
|
Once this is ready, I'd like to merge it to master and then cherry-pick to the iOS (and maybe Android) release branches where the returned objects can actually be used in the platform source and layer wrappers. |
| if (impl->style) { | ||
| impl->styleMutated = true; | ||
| impl->style->removeSource(sourceID); | ||
| return impl->style->removeSource(sourceID); |
| } | ||
|
|
||
| impl->styleMutated = true; | ||
| impl->backend.activate(); | ||
|
|
||
| impl->style->removeLayer(id); | ||
| auto removedLayer = impl->style->removeLayer(id); | ||
| impl->onUpdate(Update::Classes); | ||
|
|
||
| impl->backend.deactivate(); |
There was a problem hiding this comment.
We will need to add special logic for CustomLayer. Currently, ~CustomLayer calls the CustomLayerDeinitializeFunction, which may make OpenGL calls. This is the reason that we call impl->backend.activate()/deactivate() -- the context needs to be active when initialization/deinitialization happens.
I think we need to preserve the behavior that removing a CustomLayer triggers deinitialization, even when returning ownership. Let's do this in Style::removeLayer, making it parallel Style::addLayer, which does initialization.
| @@ -43,6 +43,11 @@ void CustomLayer::Impl::initialize() { | |||
| initializeFn(context); | |||
| } | |||
|
|
|||
| void CustomLayer::Impl::deinitialize() { | |||
| assert(deinitializeFn); | |||
| deinitializeFn(context); | |||
There was a problem hiding this comment.
Also need to remove deinitialization from CustomLayer::Impl::~Impl. And from the looks of that method, it's possible that deinitializeFn may be optional? I don't remember but we should probably keep the guard.
|
@jfirebaugh can you please double check that 554abe9 looks ok to you? |
|
👍 |
When a source or layer is removed transfer ownership back to the caller so it can (optionally) take it. Preserve the behavior that removing a CustomLayer triggers deinitialization. Deinitialize all custom layers when a style is destroyed in case those layers are not explicitly removed.
When a source or layer is removed transfer ownership back to the caller so it can (optionally) take it. Preserve the behavior that removing a CustomLayer triggers deinitialization. Deinitialize all custom layers when a style is destroyed in case those layers are not explicitly removed.
When a source or layer is removed transfer ownership back to the caller so it can (optionally) take it. Preserve the behavior that removing a CustomLayer triggers deinitialization. Deinitialize all custom layers when a style is destroyed in case those layers are not explicitly removed.
When a source or layer is removed transfer ownership back to the caller so it can (optionally) take it.
ref #6959
cc @jfirebaugh