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

Update Marker Icon doesn't result in AnnotationManager::removeAnnotationIcon #7343

Closed
AlexeyIke opened this issue Dec 8, 2016 · 4 comments · Fixed by #9643
Closed

Update Marker Icon doesn't result in AnnotationManager::removeAnnotationIcon #7343

AlexeyIke opened this issue Dec 8, 2016 · 4 comments · Fixed by #9643
Labels
Android Mapbox Maps SDK for Android good first issue Good for newcomers

Comments

@AlexeyIke
Copy link

Platform:Android
Mapbox SDK version:android-v4.2.0-beta.4

Steps to trigger behavior

  1. Create many markers on map by using MapboxMap.addMarkers(List<? extends BaseMarkerOptions> markerOptionsList); - for example 200 markers;
  2. Change all markers position and icon every second (icon - new bitmap)

Expected behavior

Markers change icon and position

Actual behavior

Memory leak - used memory grows on every icon change.

When i use mapboxMap.addMarker(new MarkerViewOptions().position(position)) for each marker add - everything works well.

For my project i need to draw many markers (about 200 - 300) and periodicaly change its position and icon.
With mapboxMap.addMarker(new MarkerViewOptions().position(position)) func adding i has memory leak. May be not using addMarkers method?

Example with memory leak i've make base on mapbox demo // AnimatedIconActivity:

package com.mapbox.mapboxandroiddemo.examples.annotations;

import android.animation.ObjectAnimator;
import android.animation.TypeEvaluator;
import android.animation.ValueAnimator;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.Paint;
import android.graphics.Rect;
import android.os.Bundle;
import android.os.Handler;
import android.support.annotation.NonNull;
import android.support.v7.app.AppCompatActivity;

import com.mapbox.mapboxandroiddemo.R;
import com.mapbox.mapboxsdk.MapboxAccountManager;
import com.mapbox.mapboxsdk.annotations.Icon;
import com.mapbox.mapboxsdk.annotations.IconFactory;
import com.mapbox.mapboxsdk.annotations.Marker;
import com.mapbox.mapboxsdk.annotations.MarkerOptions;
import com.mapbox.mapboxsdk.annotations.MarkerViewOptions;
import com.mapbox.mapboxsdk.geometry.LatLng;
import com.mapbox.mapboxsdk.maps.MapView;
import com.mapbox.mapboxsdk.maps.MapboxMap;
import com.mapbox.mapboxsdk.maps.OnMapReadyCallback;

import java.util.ArrayList;
import java.util.List;

public class AnimatedMarkerActivity extends AppCompatActivity {

private MapView mapView;
Handler handler = new Handler();
List mlist = new ArrayList<>();
Paint paint;
static int i=0;
Runnable runnable;
MapboxMap mmapboxMap;

@OverRide
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);

runnable = new Runnable() {
  @Override
  public void run() {
    i++;

    for (Marker marker: mmapboxMap.getMarkers()) {

      marker.setIcon(getIcon(i, paint));

      LatLng position = marker.getPosition();
      position.setLatitude(position.getLatitude() + 0.001);
      position.setLongitude(position.getLongitude() + 0.001);
      marker.setPosition(position);
    }

    handler.postDelayed(runnable, 2000);

  }
};

// Mapbox access token is configured here. This needs to be called either in your application
// object or in the same activity which contains the mapview.
MapboxAccountManager.start(this, getString(R.string.access_token));

// This contains the MapView in XML and needs to be called after the account manager
setContentView(R.layout.activity_annotation_animated_marker);

mapView = (MapView) findViewById(R.id.mapView);
mapView.onCreate(savedInstanceState);

paint = new Paint(Paint.ANTI_ALIAS_FLAG);
paint.setColor(Color.WHITE);
paint.setTextAlign(Paint.Align.LEFT);
paint.setTextSize(10 * getResources().getDisplayMetrics().density);

mapView.getMapAsync(new OnMapReadyCallback() {
  @Override
  public void onMapReady(MapboxMap mapboxMap) {
    mmapboxMap = mapboxMap;

    final ArrayList<MarkerOptions> buffer = new ArrayList<>();

    for (int i=0;i< 200; i++) {
      LatLng position = new LatLng(64.900932, -18.167040);
      position.setLatitude(position.getLatitude() - 0.2 * i);
      position.setLongitude(position.getLongitude() + 0.2 * i);

      MarkerOptions marker = new MarkerOptions()
              .title("")
              .position(position);

      buffer.add(marker);


    }

    mapboxMap.addMarkers(buffer);
    handler.postDelayed(runnable, 3000);
  }
});

}

@OverRide
public void onResume() {
super.onResume();
mapView.onResume();
}

@OverRide
public void onPause() {
super.onPause();
mapView.onPause();
}

@OverRide
public void onLowMemory() {
super.onLowMemory();
mapView.onLowMemory();
}

@OverRide
protected void onDestroy() {
super.onDestroy();
mapView.onDestroy();
}

@OverRide
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
mapView.onSaveInstanceState(outState);
}

private static class LatLngEvaluator implements TypeEvaluator {
// Method is used to interpolate the marker animation.

private LatLng latLng = new LatLng();

@Override
public LatLng evaluate(float fraction, LatLng startValue, LatLng endValue) {
  latLng.setLatitude(startValue.getLatitude()
    + ((endValue.getLatitude() - startValue.getLatitude()) * fraction));
  latLng.setLongitude(startValue.getLongitude()
    + ((endValue.getLongitude() - startValue.getLongitude()) * fraction));
  return latLng;
}

}

private Icon getIcon(int i, Paint paint){
Bitmap bitmap;

  bitmap = BitmapFactory.decodeResource(
          getResources(),R.drawable.green_marker);
Icon icon = IconFactory.getInstance(this).fromBitmap(drawTextOnBitmap(String.valueOf(i), paint, bitmap));
return icon;

}

public static Bitmap drawTextOnBitmap(String text, Paint paint, Bitmap source) {
Bitmap mutableBitmap = source.copy(Bitmap.Config.ARGB_8888, true);
Canvas canvas = new Canvas(mutableBitmap);

// draw text to the Canvas center
Rect boundsText = new Rect();
paint.getTextBounds(text, 0, text.length(), boundsText);
int x = (mutableBitmap.getWidth() - boundsText.width()) / 2;
int y = (mutableBitmap.getHeight() + boundsText.height()) / 2;
canvas.drawText(text, x, y, paint);
return Bitmap.createBitmap(mutableBitmap, 0, 0, mutableBitmap.getWidth(), mutableBitmap.getHeight());

}
}

@tobrun
Copy link
Member

tobrun commented Dec 8, 2016

@AlexeyIke
thank you for reaching out and using our products! Can you explain me what you mean with memory leak in this situation? From my perspective a memory leak and memory growing/churning aren't the same thing.

Looking at the code you posted I'm seeing that you are constantly creating new bitmaps/icons. These are memory intensive tasks and you should optimise your code to avoid creating new objects if not needed.

Could you provide us so more information on this by profiling the code an seeing where the large amounts of memory allocations are occurring?

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Dec 8, 2016
@AlexeyIke
Copy link
Author

AlexeyIke commented Dec 9, 2016

Hello,
in my application i've show city buses (about 200 markers), it positions, route number and its direction, and update it every 8 seconds
(that's why i generate new icon if bus drive direction has been changed)
(in my app overflow happen after some minutes - no fail, but markers became invisible, example that i made on mapbox_demo fail in a minute)

I mean memory leak beacause if i change icon for marker previous icon should be delete by GC, but no mem free happen for markers - used memory grows up and even when i init GC from AndroidStudio not free memory.

Screenshot - https://drive.google.com/open?id=0B3U64R1No4k8NmdIcjNpcUNfT2c
dump scr - https://drive.google.com/open?id=0B3U64R1No4k8NmdIcjNpcUNfT2c
dump file - https://drive.google.com/open?id=0B3U64R1No4k8T2JNdm14NTZvdVk
most allocation for icons with it bitmap (2401 objects on )
(all for example that i've send early)

Another way for my project - instead if changind icon just delete and add markers. So, i need to know keeping changed markers icons in memory normal or bug?

@tobrun
Copy link
Member

tobrun commented Dec 9, 2016

Thank you for clarifying those statements, very helpful! The reason why you don't see memory being freed with invoking a java gc is that the Bitmap is passed through jni as a bytebuffer and is than is placed in a SpriteAtlas with AnnotationManager::addIcon. I will not be possible to let the java gc clean that up. The only way would be to explicitly call AnnotationManager::removeIcon, while this is supported in the C++ we still need to expose the proper binding for it. We also need to check if we can automatically hook this up when you are replacing an icon.

Re. your use-case, I would highly advice you to look at runtime styling API instead of using the marker API. This API is much more flexible and you should be able to handle your use-case without creating any bitmaps at all. I think this cluster example should help you get started.

Thanks again for taking the time to write this up and making our SDK better.

@tobrun tobrun changed the title Memory leak on using MapboxMap.addMarkers(List<? extends BaseMarkerOptions> ) Update Marker Icon doesn't result in AnnotationManager::removeAnnotationIcon Dec 9, 2016
@tobrun tobrun added this to the android-v5.0.0 milestone Dec 9, 2016
@tobrun tobrun added the good first issue Good for newcomers label Dec 9, 2016
@AlexeyIke
Copy link
Author

AlexeyIke commented Jan 11, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants