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

Custom layer fixes #11239

Merged
merged 5 commits into from
Feb 23, 2018
Merged

Custom layer fixes #11239

merged 5 commits into from
Feb 23, 2018

Conversation

ivovandongen
Copy link
Contributor

Fixes #8448

Fixes two issues reported in #8448:

The issues where obscured by the fact that we don't have error checking in the custom layer functions (as it is external code). Any subsequent opengl operation would fail, hinting at unrelated points in the code. This pr adds some checks on calling into the custom layer code to render_custom_layer to makes sure these things are easier to catch.

Also added error checking code to the example custom layer to make debugging easier in the future.

@ivovandongen ivovandongen added Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl labels Feb 19, 2018
@ivovandongen ivovandongen self-assigned this Feb 19, 2018
@@ -2,11 +2,114 @@
#include <GLES2/gl2.h>

#include <mbgl/util/logging.hpp>

#include <mbgl/util/string.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably remove the logging.hpp and string.hpp dependencies. I don't think we should encourage custom layer implementations to depend on anything other than custom_layer.hpp.

@@ -46,11 +47,11 @@ void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*)
if (context != impl().context || !initialized) {
//If the context changed, deinitialize the previous one before initializing the new one.
if (context && !contextDestroyed && impl().deinitializeFn) {
impl().deinitializeFn(context);
MBGL_CHECK_ERROR(impl().deinitializeFn(context));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this going to break on Qt due to the special use of MBGL_CHECK_ERROR there? (#11106)

cc @kkaefer @tmpsantos

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really, Qt is not using MBGL_CHECK_ERROR as described by #11106

@tobrun tobrun added this to the android-v5.4.2 milestone Feb 20, 2018
@ivovandongen
Copy link
Contributor Author

@jfirebaugh Removed the dependencies on mbgl headers, expect for the custom layer specific.

@ivovandongen ivovandongen merged commit 7b38ae3 into master Feb 23, 2018
@ivovandongen ivovandongen deleted the custom-layer-fixes branch February 23, 2018 10:04
@tobrun tobrun mentioned this pull request Mar 1, 2018
23 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 Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[android] App crashes when swapping a custom layer
4 participants