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

Rework eglDisplay and eglContext construction/destruction #9412

Closed
wants to merge 1 commit into from

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Jul 3, 2017

This PR reworks the way we handle eglDisplay and eglContext creation and destruction.
Main purpose of this change is to eliminate state and resolve edge case crashes.

ezgif com-video-to-gif 56

Simplified egl state

Before this PR, a couple of code paths were possible related to egl state management.
This is now simplified and build around the SurfaceView callbacks:

    @Override
    public void surfaceCreated(SurfaceHolder holder) {
      if (nativeMapView == null) {
        nativeMapView = new NativeMapView(MapView.this);
        nativeMapView.initializeDisplay();
        nativeMapView.initializeContext();
        nativeMapView.createSurface(surface = holder.getSurface());
        nativeMapView.resizeView(getWidth(), getHeight());
        initialiseMap();
      }
    }

    @Override
    public void surfaceDestroyed(SurfaceHolder holder) {
      if (nativeMapView != null) {
        mapboxMap.onStop();
        nativeMapView.destroySurface();
        surface.release();
        surface = null;
        nativeMapView.terminateContext();
        nativeMapView.terminateDisplay();
        nativeMapView.destroy();
        nativeMapView = null;
      }
    }

This means that an application going into background will go through the same lifecycle as an Activity that is being recreated through rotation. We aren't retaining the map object, eglDisplay and eglContext anymore in the former use-case and this streamlines invoking above code blocks when a map is no longer visible on the screen.

Visibility state of MapView

We used to rely on View inflation to create the map object and trigger Surface creation. With this PR, we are relying on OnVisibilityChange which allows to have finer grain control over when callbacks in above section occur and unblocks toggling the visibility of the MapView and having it hidden at Activity startup.

Measured ViewHierarchy

Since we postpone creating the map object, we are slower in loading the style and thus slower in invoking the OnMapReady callback for the user. This has the benefit that view hierarchy has been measured and we can remove the workarounds we have in place for them.

Additional test Activities

On top of the SDK changes, 2 test activities were added to facilitate testing of this PR.

cc @ivovandongen @mapbox/android @kkaefer

…. Rework lifecycle events. Postpone map initalisation.
@tobrun tobrun added Android Mapbox Maps SDK for Android ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Jul 3, 2017
@tobrun tobrun requested a review from ivovandongen July 3, 2017 18:51
@tobrun
Copy link
Member Author

tobrun commented Jul 4, 2017

The only issue I was able to identify is that we are losing the map state when you are stopping/starting the activity (eg. press home button and reopen the app from recent apps list). The reason for this is that onSaveInstanceState isn't triggered and thus there isn't any state to recover from (the map will be started with initial configuration).

update is now fixed with introducing a saved state bundle as part of onStop.
update is not an ideal solution as we are not persisting the user state (what he normally would save in onSaveInstanceState). eg. the user adds a Polygon, rotates the device, Polygon is still there (either by state keeping or readding as part of initialisation) but if he now presses home and returns to the screen, the Polygon is removed. I'm going to revisit this PR and opting to retain NativeMapView and destroy/recreate the eglDisplay/eglContext.

@tobrun
Copy link
Member Author

tobrun commented Jul 4, 2017

With current codebase, the approach of retaining the map but destroying the display and context, doesn't seem to be a viable option. Getting it to work seems to be a lift & with GLSurfaceView on the horizon, it seems better to punt on this and focus on other issues.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant