onMapReady() callback called before style is loaded.

[ivovandongen]

When using the runtime style api to manipulate layers / sources it seems that the onMapReady callback is called to early when the style is not yet in cache. For example, in the following snippet the fill layer will not be colored when the cache is cleared for the app (and thus all cached resources, incl the style are cleared). However, once the style is cached, the example works properly.

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_lab_land_use_styling);

        mapView = (MapView) findViewById(R.id.mapView);
        mapView.onCreate(savedInstanceState);
        mapView.getMapAsync(new OnMapReadyCallback() {
            @Override
            public void onMapReady(final MapboxMap mapboxMap) {

                FillLayer schoolLayer = new FillLayer("school-layer", "composite");
                schoolLayer.setSourceLayer("landuse");
                schoolLayer.setProperties(
                        fillColor(Color.RED)
                );
                schoolLayer.setFilter(eq("class", "school"));
                mapboxMap.addLayer(schoolLayer);
            }
        });
    }

cc @jfirebaugh

[ivovandongen]

Thanks for checking @frederoni! The issue probably lies in this callback here then: https://github.com/mapbox/mapbox-gl-native/blob/master/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java#L442

We might want to refactor this a bit to make the intention clearer as well.

[frederoni]

This doesn't affect iOS and probably not macOS either.
The onMapReady iOS equivalent is -mapViewDidFinishLoadingMap which is triggered by -mbgl::View::notifyMapChange MapChangeDidFinishLoadingMap and it gets called at the right moment whether the style is cached or not.

[tobrun]

Two items impacted that implementation:

• issues with offline regions not being created
• gl markers where reloaded and flickers issue?

Need to do some digging if you need concrete ticket info but these are some items to test for.

update: related issue #4393

[ivovandongen]

Just confirmed that changing WILL_START_RENDERING_MAP to DID_FINISH_LOADING_MAP fixes the issues with runtime styling with empty cache. I'm reluctant to make changes here though as there has been some history of issue with this particular piece of code.

@jfirebaugh Can you confirm the status of this in core. Is it safe to react to DID_FINISH_LOADING_MAP, eg can we expect the callback also in offline cases?

[tobrun]

I'm taking up the testing the offline and marker issue. I kinda remember the use-cases around this.

[ivovandongen]

The offline case seems to work fine for built-in styles. I still get a onMapReady callback when going into flight mode after downloading a region.

[ivovandongen]

Noting that #6121 would result in errors when users mutate the style in the callback and it's not fired at the right time.

[ivovandongen]

Tested changing the timing of the callback to be only after the map has loaded (commit).This works correctly in all online cases, also fixing the issue with manipulating the styles directly in the callback when nothing is cached yet.

In offline cases though we need to set the expectations. With this change, the callback is only fired when all sources + the style are loaded for the current viewport. This differs from previous behaviour where the callback is fired as soon as the the map is about to render.

The main question is; do we want the callback to fire when at least the style is loaded? (see #4393 for reference on the initial change)

cc @zugaldia @jfirebaugh

[zugaldia]

On offline cases though we need to set the expectations. With this change, the callback is only fired when all sources + the style are loaded for the current viewport.

From what I understand, this is not a regression like in #4393 when no callback was fired, and it's also consistent with the current behavior on iOS. I think this is an acceptable solution, we need to make sure this is documented and pointed out in the examples.

[ivovandongen]

I added a PR (#6371) to add a signal when the style is loaded like @tmpsantos suggests in #4527. This enables us to reliably fire the onMapReady callback as soon as the api is safe to call.

[ivovandongen]

The pr in #6371 needs a slightly different timing, fixed in #6392.