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

Reset the native renderer only when the GL thread exits #14841

Merged
merged 1 commit into from
Jun 4, 2019

Conversation

LukasPaczos
Copy link
Contributor

@LukasPaczos LukasPaczos commented Jun 4, 2019

Closes #14618.
Closes #14592.
Closes #14252.
Closes #14463.
Closes #14554. It still blocks when scrolling a map inside a recycler, which is unavoidable because we are detaching the view, but it doesn't block for #onStop anymore.
Also doesn't force tiles to reload after coming back from the background (refs #12844 (comment)).

Hopefully, this should also replace #14821. Since we are not resetting the native renderer when the GLSurfaceView is still able to render, we are not going to run into a missing pointer scenario. When running the introduced in #14821 test, I'm seeing:

Abort message: '../../../../../../../src/mbgl/gl/renderer_backend.cpp:31: void mbgl::gl::RendererBackend::assumeFramebufferBinding(const gl::FramebufferID): assertion "gl::value::BindFramebuffer::Get() == getContext<gl::Context>().bindFramebuffer.getCurrentValue()" failed'

Can this just be an issue with the test itself, where we're trying to force the surface to draw when it wouldn't in real-world case? Otherwise, I can reintroduce the hasSurface flag just to cover this one edge case, still without resetting the pointer.

All of the previously introduced integration tests are passing for me, but if we'd like to move forward with this, a device farm test run would be great.

@LukasPaczos LukasPaczos added the Android Mapbox Maps SDK for Android label Jun 4, 2019
@LukasPaczos LukasPaczos requested a review from tobrun June 4, 2019 12:27
@LukasPaczos LukasPaczos force-pushed the lp-detach-gl-thread branch 3 times, most recently from 9e7fa4f to eafc56b Compare June 4, 2019 12:47
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

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

looks good, minor nits

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.