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

Rewire map initialisation #9462

Merged
merged 1 commit into from
Jul 19, 2017
Merged

Rewire map initialisation #9462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 86 additions & 47 deletions platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import android.graphics.PointF;
import android.graphics.SurfaceTexture;
import android.os.Bundle;
import android.os.Handler;
import android.support.annotation.CallSuper;
import android.support.annotation.IntDef;
import android.support.annotation.NonNull;
Expand Down Expand Up @@ -42,6 +41,9 @@
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;

import timber.log.Timber;

/**
* <p>
Expand All @@ -59,17 +61,24 @@
*/
public class MapView extends FrameLayout {

private final MapCallback mapCallback = new MapCallback();
private MapboxMap mapboxMap;

private NativeMapView nativeMapView;
private boolean textureMode;
private MapboxMapOptions mapboxMapOptions;
private boolean destroyed;
private boolean hasSurface;

private MapboxMap mapboxMap;
private MapCallback mapCallback;
private MyLocationView myLocationView;
private CompassView compassView;
private ImageView attrView;
private ImageView logoView;

private MapGestureDetector mapGestureDetector;
private MapKeyListener mapKeyListener;
private MapZoomButtonController mapZoomButtonController;
private Bundle savedInstanceState;
private final CopyOnWriteArrayList<OnMapChangedListener> onMapChangedListeners = new CopyOnWriteArrayList<>();

@UiThread
public MapView(@NonNull Context context) {
Expand Down Expand Up @@ -101,21 +110,23 @@ private void initialise(@NonNull final Context context, @NonNull final MapboxMap
LayoutInflater.from(context).inflate(R.layout.mapbox_mapview_preview, this);
return;
}

// determine render surface
textureMode = options.getTextureMode();
mapboxMapOptions = options;

// inflate view
View view = LayoutInflater.from(context).inflate(R.layout.mapbox_mapview_internal, this);
CompassView compassView = (CompassView) view.findViewById(R.id.compassView);
MyLocationView myLocationView = (MyLocationView) view.findViewById(R.id.userLocationView);
ImageView attrView = (ImageView) view.findViewById(R.id.attributionView);
compassView = (CompassView) view.findViewById(R.id.compassView);
myLocationView = (MyLocationView) view.findViewById(R.id.userLocationView);
attrView = (ImageView) view.findViewById(R.id.attributionView);
logoView = (ImageView) view.findViewById(R.id.logoView);

// add accessibility support
setContentDescription(context.getString(R.string.mapbox_mapActionDescription));
setWillNotDraw(false);
}

// create native Map object
nativeMapView = new NativeMapView(this);
private void initialiseMap() {
Context context = getContext();
addOnMapChangedListener(mapCallback);

// callback for focal point invalidation
FocalPointInvalidator focalPoint = new FocalPointInvalidator(compassView);
Expand All @@ -131,7 +142,7 @@ private void initialise(@NonNull final Context context, @NonNull final MapboxMap

// setup components for MapboxMap creation
Projection proj = new Projection(nativeMapView);
UiSettings uiSettings = new UiSettings(proj, focalPoint, compassView, attrView, view.findViewById(R.id.logoView));
UiSettings uiSettings = new UiSettings(proj, focalPoint, compassView, attrView, logoView);
TrackingSettings trackingSettings = new TrackingSettings(myLocationView, uiSettings, focalPoint, zoomInvalidator);
MyLocationViewSettings myLocationViewSettings = new MyLocationViewSettings(myLocationView, proj, focalPoint);
LongSparseArray<Annotation> annotationsArray = new LongSparseArray<>();
Expand All @@ -147,6 +158,7 @@ private void initialise(@NonNull final Context context, @NonNull final MapboxMap
cameraChangeDispatcher);
mapboxMap = new MapboxMap(nativeMapView, transform, uiSettings, trackingSettings, myLocationViewSettings, proj,
registerTouchListener, annotationManager, cameraChangeDispatcher);
mapCallback.attachMapboxMap(mapboxMap);

// user input
mapGestureDetector = new MapGestureDetector(context, transform, proj, uiSettings, trackingSettings,
Expand All @@ -168,14 +180,15 @@ private void initialise(@NonNull final Context context, @NonNull final MapboxMap
setFocusableInTouchMode(true);
requestDisallowInterceptTouchEvent(true);

// allow onDraw invocation
setWillNotDraw(false);

// notify Map object about current connectivity state
nativeMapView.setReachability(ConnectivityReceiver.instance(context).isConnected(context));

// initialise MapboxMap
mapboxMap.initialise(context, options);
if (savedInstanceState == null) {
mapboxMap.initialise(context, mapboxMapOptions);
} else {
mapboxMap.onRestoreInstanceState(savedInstanceState);
}
}

//
Expand All @@ -198,16 +211,11 @@ public void onCreate(@Nullable Bundle savedInstanceState) {
if (savedInstanceState == null) {
MapboxTelemetry.getInstance().pushEvent(MapboxEventWrapper.buildMapLoadEvent());
} else if (savedInstanceState.getBoolean(MapboxConstants.STATE_HAS_SAVED_STATE)) {
mapboxMap.onRestoreInstanceState(savedInstanceState);
this.savedInstanceState = savedInstanceState;
}

initialiseDrawingSurface(textureMode);
addOnMapChangedListener(mapCallback = new MapCallback(mapboxMap));
}

private void initialiseDrawingSurface(boolean textureMode) {
nativeMapView.initializeDisplay();
nativeMapView.initializeContext();
if (textureMode) {
TextureView textureView = new TextureView(getContext());
textureView.setSurfaceTextureListener(new SurfaceTextureListener());
Expand Down Expand Up @@ -236,8 +244,10 @@ public void onSaveInstanceState(@NonNull Bundle outState) {
*/
@UiThread
public void onStart() {
mapboxMap.onStart();
ConnectivityReceiver.instance(getContext()).activate();
if (mapboxMap != null) {
mapboxMap.onStart();
}
}

/**
Expand Down Expand Up @@ -271,9 +281,6 @@ public void onStop() {
@UiThread
public void onDestroy() {
destroyed = true;
nativeMapView.terminateContext();
nativeMapView.terminateDisplay();
nativeMapView.destroySurface();
nativeMapView.destroy();
nativeMapView = null;
}
Expand Down Expand Up @@ -384,7 +391,10 @@ public void setStyleUrl(@NonNull String url) {
if (destroyed) {
return;
}

if (nativeMapView == null) {
mapboxMapOptions.styleUrl(url);
Copy link
Member Author

Choose a reason for hiding this comment

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

keeping state in mapboxMapOptions is map isn't ready yet, this will be applied to the map when started later on.

return;
}
nativeMapView.setStyleUrl(url);
}

Expand Down Expand Up @@ -412,7 +422,6 @@ public void onDraw(Canvas canvas) {
if (!hasSurface) {
return;
}

nativeMapView.render();
}

Expand All @@ -422,7 +431,7 @@ protected void onSizeChanged(int width, int height, int oldw, int oldh) {
return;
}

if (!isInEditMode()) {
if (!isInEditMode() && nativeMapView != null) {
nativeMapView.resizeView(width, height);
}
}
Expand All @@ -433,7 +442,16 @@ private class SurfaceCallback implements SurfaceHolder.Callback {

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

hasSurface = true;
}

Expand All @@ -450,6 +468,7 @@ public void surfaceDestroyed(SurfaceHolder holder) {
hasSurface = false;

if (nativeMapView != null) {
// occurs when activity goes to background
nativeMapView.destroySurface();
}
surface.release();
Expand All @@ -465,8 +484,17 @@ private class SurfaceTextureListener implements TextureView.SurfaceTextureListen
// Must do all EGL/GL ES initialization here
@Override
public void onSurfaceTextureAvailable(SurfaceTexture surface, int width, int height) {
nativeMapView.createSurface(this.surface = new Surface(surface));
nativeMapView.resizeFramebuffer(width, height);
if (nativeMapView == null) {
nativeMapView = new NativeMapView(MapView.this);
nativeMapView.createSurface(this.surface = new Surface(surface));
nativeMapView.resizeFramebuffer(width, height);
nativeMapView.resizeView(width, height);
initialiseMap();
mapboxMap.onStart();
} else {
nativeMapView.createSurface(this.surface = new Surface(surface));
}

hasSurface = true;
}

Expand Down Expand Up @@ -522,16 +550,32 @@ protected void onDetachedFromWindow() {
// Called when view is hidden and shown
@Override
protected void onVisibilityChanged(@NonNull View changedView, int visibility) {
if (isInEditMode() || mapZoomButtonController == null) {
if (isInEditMode()) {
return;
}
mapZoomButtonController.setVisible(visibility == View.VISIBLE);
if (visibility == View.VISIBLE && nativeMapView == null) {
initialiseDrawingSurface(mapboxMapOptions.getTextureMode());
}

if (mapZoomButtonController != null && nativeMapView != null) {
mapZoomButtonController.setVisible(visibility == View.VISIBLE);
}
}

//
// Map events
//

void onMapChange(int rawChange) {
for (MapView.OnMapChangedListener onMapChangedListener : onMapChangedListeners) {
try {
onMapChangedListener.onMapChanged(rawChange);
} catch (RuntimeException err) {
Timber.e("Exception (%s) in MapView.OnMapChangedListener: %s", err.getClass(), err.getMessage());
}
}
}

/**
* <p>
* Add a callback that's invoked when the displayed map view changes.
Expand All @@ -543,7 +587,7 @@ protected void onVisibilityChanged(@NonNull View changedView, int visibility) {
*/
public void addOnMapChangedListener(@Nullable OnMapChangedListener listener) {
if (listener != null) {
nativeMapView.addOnMapChangedListener(listener);
onMapChangedListeners.add(listener);
}
}

Expand All @@ -555,7 +599,7 @@ public void addOnMapChangedListener(@Nullable OnMapChangedListener listener) {
*/
public void removeOnMapChangedListener(@Nullable OnMapChangedListener listener) {
if (listener != null) {
nativeMapView.removeOnMapChangedListener(listener);
onMapChangedListeners.remove(listener);
}
}

Expand Down Expand Up @@ -919,26 +963,21 @@ private void setZoom(double zoomLevel, @Nullable PointF focalPoint, @NonNull Tra

private static class MapCallback implements OnMapChangedListener {

private final MapboxMap mapboxMap;
private MapboxMap mapboxMap;
private final List<OnMapReadyCallback> onMapReadyCallbackList = new ArrayList<>();
private boolean initialLoad = true;

MapCallback(MapboxMap mapboxMap) {
void attachMapboxMap(MapboxMap mapboxMap) {
this.mapboxMap = mapboxMap;
}

@Override
public void onMapChanged(@MapChange int change) {
if (change == DID_FINISH_LOADING_STYLE && initialLoad) {
initialLoad = false;
new Handler().post(new Runnable() {
Copy link
Member Author

Choose a reason for hiding this comment

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

this resolves a memory leak when quickly rotating the device on map start.

Copy link
Contributor

Choose a reason for hiding this comment

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

This does change the timing of the event a little though. The handler ensured that it was fired after the current loop cycle. Not sure if that is a big deal.

Copy link
Member Author

@tobrun tobrun Jul 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

afaik, we posted to the end of message queue to make sure the view was measured (to be able to initialize all components with correct width/height) . Now with this PR, we postpone map creation and thus style loading to after onResume (when the view is toggled visible). Can't really think of another reason why we needed that post there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Just noting. As long as the tests succeed it shouldn't be a big deal.

@Override
public void run() {
mapboxMap.onPreMapReady();
onMapReady();
mapboxMap.onPostMapReady();
}
});
mapboxMap.onPreMapReady();
onMapReady();
mapboxMap.onPostMapReady();
} else if (change == DID_FINISH_RENDERING_FRAME || change == DID_FINISH_RENDERING_FRAME_FULLY_RENDERED) {
mapboxMap.onUpdateFullyRendered();
} else if (change == REGION_IS_CHANGING || change == REGION_DID_CHANGE || change == DID_FINISH_LOADING_MAP) {
Expand Down
Loading