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

[android] Restore prior GLContext if exists #4716

Merged
merged 1 commit into from
Apr 18, 2016

Conversation

tmpsantos
Copy link
Contributor

Apparently some versions of Android, notable 4.4.4 running on my Nexus 5, creates a GLContext on the Android UIThread which is the one we are now using for rendering after #2909.

If the context is not restored, nothing gets rendered (or sometimes partially, or artifacts) because Android will try to do GL stuff on the context used by Mapbox GL and mess things up.

@tmpsantos
Copy link
Contributor Author

👀 @jfirebaugh @tobrun

@bleege bleege added this to the android-v4.1.0 milestone Apr 15, 2016
@@ -196,6 +201,14 @@ void NativeMapView::render() {
}

deactivate();

if (currentContext != context && currentContext != EGL_NO_CONTEXT) {
Copy link

Choose a reason for hiding this comment

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

Why don't you just comment out the call to deactivate instead? It's equivalent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

But I think it is better deactivate() if there is no prior GLContext. Gonna change that.

@tmpsantos tmpsantos force-pushed the tmpsantos-android_not_rendering branch from 5f247c6 to 1fdc3aa Compare April 15, 2016 15:19
throw std::runtime_error("eglMakeCurrent() failed");
}
} else {
deactivate();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record, on my Nexus 5x Android 6.0.1 deactivate() gets called but on my Nexus 5 Android 4.4.4 the context gets replaced with the prior context.

Copy link

Choose a reason for hiding this comment

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

@tmpsantos My guess is that on "Nexus 5 Android 4.4.4", either the View Framework or some other opengl view is changing the opengl state prior to call to NativeMapView::render. Is it possible to do the following experiment in your sandbox with the following two changes? If the 'screen is white bug' goes away, its an issue with not handling the opengl state correctly.

  1. Instantiate mbgl::Map with GLContextMode::Shared as the fourth argument here. This will ensure that gl state is reset every time by mbgl::Map.
  2. Call glViewPort unconditionally by commenting out if(sizeChanged) (The current code will consistently cause crashes/wrong behavior if there are multiple Opengl views in the same application. Its also missing the glError call that MBGL_ERROR_CHECK provides.).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tmpsantos My guess is that on "Nexus 5 Android 4.4.4", either the View Framework or some other opengl view is changing the opengl state prior to call to NativeMapView::render. Is it possible to do the following experiment in your sandbox with the following two changes? If the 'screen is white bug' goes away, its an issue with not handling the opengl state correctly.

Instantiate mbgl::Map with GLContextMode::Shared as the fourth argument here. This will ensure that gl state is reset every time by mbgl::Map.
Call glViewPort unconditionally by commenting out if(sizeChanged) (The current code will consistently cause crashes/wrong behavior if there are multiple Opengl views in the same application. Its also missing the glError call that MBGL_ERROR_CHECK provides.).

What really happens is there is another GL context in use that we cannot use/share because is not configured with the attributes we expect. GLContextMode::Shared didn't work in this case (I tested). We really have to use our own context and after we are done rendering, restore the old context that Android expects to be active.

@mb12 thanks for the feedback!

@tmpsantos tmpsantos force-pushed the tmpsantos-android_not_rendering branch from 1fdc3aa to 9efa5f9 Compare April 15, 2016 15:41
@jfirebaugh
Copy link
Contributor

If saving and restoring the specific display, context, and surfaces is necessary, it seems like that should be part of activate / deactivate themselves, to be consistent wherever the activate / deactivate pair is used.

@mb12
Copy link

mb12 commented Apr 15, 2016

@tmpsantos and @jfirebaugh Is there any plan to fix this? All this complexity will go away when this is fixed.
#1662

@tmpsantos
Copy link
Contributor Author

@tmpsantos and @jfirebaugh Is there any plan to fix this? All this complexity will go away when this is fixed.

I still want to work at this issue, but I don't have a timeline for that.

Now that Android renders on the same thread as the mbgl::Map object that is feasible (we need to create the mbgl::Map on the GLSurfaceView.Renderer thread). The challenge is to proxy every getter and setter from the UIThread to this renderer thread, specially because some of the getters are synchronous.

Apparently some versions of Android, notable 4.4.4 running
on my Nexus 5, creates a GLContext on the Android UIThread
which is the one we are now using for rendering after #2909.

If the context is not restored, nothing gets rendered (or
sometimes partially, or artifacts) because Android will try
to do GL stuff on the context used by Mapbox GL and mess things up.
@tmpsantos tmpsantos force-pushed the tmpsantos-android_not_rendering branch from 9efa5f9 to 7d80690 Compare April 18, 2016 15:58
@tmpsantos
Copy link
Contributor Author

it seems like that should be part of activate / deactivate themselves

👍

@tmpsantos tmpsantos merged commit 7d80690 into master Apr 18, 2016
@tmpsantos tmpsantos deleted the tmpsantos-android_not_rendering branch April 18, 2016 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants