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

[Android] Best practice to use MarkerViews? #5161

Closed
Schumi09 opened this issue May 26, 2016 · 19 comments
Closed

[Android] Best practice to use MarkerViews? #5161

Schumi09 opened this issue May 26, 2016 · 19 comments
Labels
Android Mapbox Maps SDK for Android annotations Annotations on iOS and macOS or markers on Android

Comments

@Schumi09
Copy link

Hi,

good job on the MarkerViews. I guess this will help me a lot and it already looks promising :)
However, I'm not quite sure yet what's be best or intended way to use it.

My first expectation was that I could use it in the very same way as the normal Markers, e.g.:

IconFactory iconFactory = IconFactory.getInstance(SimpleMapViewActivity.this);
Drawable iconDrawable = ContextCompat.getDrawable(SimpleMapViewActivity.this, R.drawable.purple_marker);
Icon icon = iconFactory.fromDrawable(iconDrawable);
mapboxMap.addMarker(new MarkerViewOptions()
  .position(new LatLng(-33.8500000, 18.4158234))
  .icon(icon)
  .rotation(45)).setAlpha(0.5f);

But no marker was added to the map.
After taking a look into the TestApp I saw the use of Adapters. So I defined some XML view similar to the CountryMarkerView and and extended the TextAdapter class like

viewHolder.icon.setImageResource(R.drawable.arrow_black);
convertView.setAlpha(marker.getAlpha());
convertView.setRotation(marker.getRotation());

and after setting

final MarkerViewManager markerViewManager = mapboxMap.getMarkerViewManager();
markerViewManager.addMarkerViewAdapter(new TextAdapter(getApplicationContext()));

it finally worked. However, for my application I will need to remove/add new MarkerViews contiously so i set up a little test like

final Handler handler = new Handler();
handler.postDelayed(new Runnable() {
                    @Override
                    public void run() {
                        mapboxMap.removeAnnotations();
                        markerViewManager.invalidateViewMarkers();
                        markerViewManager.getMarkerViewAdapters().removeAll(markerViewManager.getMarkerViewAdapters());
                        handler.postDelayed(new Runnable() {
                            @Override
                            public void run() {
                                mapboxMap.addMarker(new MarkerViewOptions()
                                        .position(new LatLng(-33.8500000, 18.4158234))
                                        .rotation(45)).setAlpha(0.5f);
                                mapboxMap.addMarker(new MarkerViewOptions()
                                        .position(new LatLng(-33.848546, 18.435047))
                                        .rotation(90)).setAlpha(0.9f);
                                markerViewManager.addMarkerViewAdapter(new TextAdapter(getApplicationContext()));
                            }
                        }, 2000);
                        handler.postDelayed(this, 4000);
                    }
                }, 4000);

Without

markerViewManager.invalidateViewMarkers(); markerViewManager.getMarkerViewAdapters().removeAll(markerViewManager.getMarkerViewAdapters());

and adding the Adapter afterwards again I did not have success to add the Markers again.

So in conclusion, what is the easiest way if you only want to add markers to the app that can be rotated and having different alpha level, like I thought it would be possible in the very beginning. Do I really have to set up XML Views and Adapter Classes or even write an Extended MarkerView class on my own? At least there is no default fallback I think. I ran my tests on a Nexus 5 with Android 6.0.1.

@tobrun
Copy link
Member

tobrun commented May 26, 2016

@Schumi09 thank you for providing feedback and iterating on this feature so quickly.

My first expectation was that I could use it in the very same way as the normal Markers with an Icon

Currently the Icon field is pretty much ignored and we had to keep it around since MarkerView extends Marker. I currently see 2 possibilities:

  • look into limiting the visibility of the Icon implementation
  • automatically create an ImageViewAdapter if an Icon is provided
viewHolder.icon.setImageResource(R.drawable.arrow_black);
convertView.setAlpha(marker.getAlpha());
convertView.setRotation(marker.getRotation());

Small note here is that alpha and rotation are automatically updated by the adapter itself (all the fields of Marker are linked to the MapView so it should automatically call invalidate). I initially though about implementing it in the way you are proposing but I wanted to make it as easy as possible for the end developer. eg. Calling marker.setAlpha(0) will result in hiding the view with an alpha animation.

and adding the Adapter afterwards again I did not have success to add the Markers again.

I will need to look into attaching/removing the adapters and how to invalidate them correctly. What is the use case of adding/removing adapters? Small side note here is instead of adding/removing markers,you can also call marker.setVisible(true/false)?

Thanks again for providing this feedback, I will look into integrating what was mentioned above to make the ViewMarker API more clear and logical.

@tobrun tobrun added Android Mapbox Maps SDK for Android annotations Annotations on iOS and macOS or markers on Android labels May 26, 2016
@Schumi09
Copy link
Author

Thanks for your fast reply 👍
I'll consider the notes about alpha and rotation, didn't try it without it yet.

What is the use case of adding/removing adapters

It has been the only way for me to remove the markers and add new ones afterwards.

Small side note here is instead of adding/removing markers,you can also call marker.setVisible(true/false)?

I've already seen it. In my project/master thesis im dealing with different visualizations of off-screen landmarks. For this purpose I calculate locations that are on-screen. So the new locations depend on the current mapview/user location and the landmarks locations (angle, distance). Like this it's pretty easy to use classic map annotations as base for my visualizations that are anyway nothing else than markers, polygons or lines. Moreover, I can't update existing polylines or polygons, only markers. Therefore, in my current implementation I just replace the current visualization with a new one. If i wanted to implement a way to update the previous visualization it would be basically the same way like it currently is, the only difference would be updating markers only ;)
I see that this is not a usual use case but removing and adding new markers should work somehow.
But i think I'm going to use marker.setVisible(true/false) for on-screen landmarks, so far I also replace the markers because it was handy ;)

@Schumi09
Copy link
Author

Schumi09 commented May 27, 2016

Little update:
So i actually found out that
addMarkers(); mMapboxMap.removeAnnotations(); addMarkers();
works. addmarkers() contains the loop you've written in the TestApp to test TextMarkers and I used the TextAdapter and view_text_marker.
So the readded MarkerViews are not shown when you use Handler (that I only used to visually check whether the removing and adding actually works). In my application I'm not doing this anyway.

Log.d("Markers", mMapboxMap.getMarkers().size() + "" );
provided the right amount of markers though, they were just not visible/rendered.

@Schumi09
Copy link
Author

Next little update:

At start I do addMarkers() and set an onMyLocationChanged listener

final MarkerViewManager markerViewManager = mapboxMap.getMarkerViewManager();
markerViewManager.addMarkerViewAdapter(new TextAdapter(getApplicationContext()));
LocationServices.getLocationServices(MainActivity.this).toggleGPS(true);
addMarkers();
mMapboxMap.setOnMyLocationChangeListener(new MapboxMap.OnMyLocationChangeListener() {
                    @Override
                    public void onMyLocationChange(@Nullable final Location location) {
                        if (location != null) {
                            mMapboxMap.removeAnnotations();
                            addMarkers();
                        }
                    }
                });

when the location changes the markers disappear but are not added again. However, when I store a reference to a marker in order to move it it works.

if (currentPositionMarker != null) {
 currentPositionMarker.setPosition(new LatLng(location.getLatitude(), location.getLongitude()));
} else {
  MarkerViewOptions options = new MarkerViewOptions().position(new LatLng(location.getLatitude(), location.getLongitude()));
  currentPositionMarker = mMapboxMap.addMarker(options);
}

I guess it should be possible to remove like all annotations on a triggered event and just add new ones ;) It seems a bit strange that it works when you just do add - remove - add but when there are Events or Handlers used it does not.

@bleege
Copy link
Contributor

bleege commented May 27, 2016

@Schumi09 Thanks for the feedback. Can you confirm what version of the SDK the app is using? We just launched the Android 4.1.0-beta.1 yesterday if you haven't seen that yet.

@Schumi09
Copy link
Author

Hi,
I've seen that you released a beta and I'm still on the Snapshot :) But there should be no difference I guess

@tobrun
Copy link
Member

tobrun commented Jun 2, 2016

One of the features of ViewAnnotations is that the following is already managed by the system:

In my project/master thesis im dealing with different visualizations of off-screen landmarks. For this purpose I calculate locations that are on-screen.

When you pan a map and markers are no longer visible, the views will no longer be updated. So don't worry to much about adding them all at once and leaving them there (side note here is that you need to set the initial camera position first else you will try showing them all at once which will not work well).

@Schumi09
Copy link
Author

Schumi09 commented Jun 2, 2016

Let me explain it better:
I got a map view that is obviously restricted by its bounding box. But i want to show indicators to the user that there are (global) landmarks around that are not in the bounding box. So i calculate positions within map view to place a marker for example that points to the off screen object. As I do it as experiment for navigation systems you are obv. always moving and that's why I recalculate everything when the location has changed (maybe using the locationtracking soon and then put it on the camerachange event). So removing and overwriting the visualizations is much more simple than providing ways to update them (It's like you also don't provide a way to update a polyline because it is not really needed)
For now I don't deal with a large set a of landmarks, so they are limited and preselected.
This is somer older screen of one of the visualizations:)
tp-transparent

So the issue i experienced last week when i set up a basic demo app was:

  • Adding a MarkerView to the map
  • Call removeAnnotations inside a looping handler or onLocationChanged event
  • Add a new MarkerView again
  • MarkerView isn't visible, mapview/or map object (don't know atm) knows that there is a marker

If i remember correctly, I did not change/move the camera position with the handlers, but with the location updates. Tho i used setCameraPosition, already noticed the markers disappear while panning so moving the camera and setting up a MarkerView at once might cause troubles.

@Schumi09
Copy link
Author

Schumi09 commented Jun 5, 2016

This is some code snippet that might help you reproducing it:

final MarkerViewOptions options = new MarkerViewOptions();
options.icon(icon);
options.flat(true);
mapboxMap.setOnMyLocationChangeListener(new MapboxMap.OnMyLocationChangeListener() {
                    @Override
                    public void onMyLocationChange(@Nullable Location location) {
                        if (location != null) {
                            Log.d("Location changed", location.toString());
                            mCurrenLocationLatLng = new LatLng(location.getLatitude(), location.getLongitude());
                            CameraPosition cameraPosition = new CameraPosition.Builder().target(mCurrenLocationLatLng).build();
                            mapboxMap.removeAnnotations();
                            mapboxMap.setCameraPosition(cameraPosition);
                            mapboxMap.addMarker(options).setPosition(mCurrenLocationLatLng);
                        }
                    }
                });

So when above is called I get

06-05 18:01:16.754 15223-15223/ifgi.mapbox_playground D/Location changed: Location[network 51.968395,7.608740 acc=38 et=+22h18m16s56ms {Bundle[mParcelledData.dataSize=708]}]
06-05 18:01:16.779 15223-15257/ifgi.mapbox_playground I/mbgl: [Sprite]: Can't find sprite named 'markerViewSettings'

When I pan the map a little bit the markerview instantly shows up while panning. Maybe there is some issue with the animation of fading a markerview out earlier? (Is there/will there be a way to disable the animation anyway?). Doing the 'update' in the onCameraChanged event did not help either. I also tried delaying the adding with a Handler for some milliseconds, no success.

Recently, this also happened frequently to me with the above code:
06-05 17:51:31.464 9616-9616/ifgi.mapbox_playground E/AndroidRuntime: FATAL EXCEPTION: main Process: ifgi.mapbox_playground, PID: 9616 java.lang.IllegalStateException: Already in the pool! at android.support.v4.util.Pools$SimplePool.release(Pools.java:113) at com.mapbox.mapboxsdk.annotations.MarkerViewManager$1.onAnimationEnd(MarkerViewManager.java:213) at com.mapbox.mapboxsdk.utils.AnimatorUtils$3.onAnimationEnd(AnimatorUtils.java:79) at android.animation.ValueAnimator.endAnimation(ValueAnimator.java:1239) at android.animation.ValueAnimator$AnimationHandler.doAnimationFrame(ValueAnimator.java:766) at android.animation.ValueAnimator$AnimationHandler$1.run(ValueAnimator.java:801) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:858) at android.view.Choreographer.doCallbacks(Choreographer.java:670) at android.view.Choreographer.doFrame(Choreographer.java:603) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:844) at android.os.Handler.handleCallback(Handler.java:739) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:148) at android.app.ActivityThread.main(ActivityThread.java:5417) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)

@tobrun
Copy link
Member

tobrun commented Jun 6, 2016

When I pan the map a little bit the markerview instantly shows up while panning. Maybe there is some issue with the animation of fading a markerview out earlier? (Is there/will there be a way to disable the animation anyway?). Doing the 'update' in the onCameraChanged event did not help either. I also tried delaying the adding with a Handler for some milliseconds, no success.

This issue comes from not invalidating the marker when adding a Marker. This is fixed in #5250.

Recently, this also happened frequently to me with the above code java.lang.IllegalStateException: Already in the pool! at android.support.v4.util.Pools$SimplePool.release(Pools.java:113)

Would you able to retest this when we release another beta build? I have been doing some fixes with this system in #5250. Would love to hear if this issue still exists.

@Schumi09
Copy link
Author

Schumi09 commented Jun 6, 2016

This issue comes from not invalidating the marker when adding a Marker. This is fixed in #5250.

Sounds great, can't wait for the new snapshot/beta 👍
So far the new MarkerView look pretty promising. I don't have to mess around with bitmap stuff anymore for setting alpha and rotation and it also looks like the issue about disappearing markers is fixed :) So the biggest issues for me are solved for me so far...

Would you able to retest this when we release another beta build? I have been doing some fixes with this system in #5250. Would love to hear if this issue still exists.

Yea, I'll try it then.

@Schumi09
Copy link
Author

Schumi09 commented Jun 7, 2016

Hi @tobrun,
can you confirm that your changes from yesterday made it into the last snapshot (from yesterday evening) ? I deleted the cache files and also invalidated the cache in android studio so it got redownloaded. Because the options.roation() still takes an integer it looks like there is something missing.

@bleege
Copy link
Contributor

bleege commented Jun 7, 2016

@Schumi09 It looks like the code that @tobrun was working on in #5076 / PR #5250 was merged into the release-android-v4.1.0 branch yesterday morning. We're using this branch to produce SNAPSHOTS and related builds for Android 4.1.0 right now and I created the last SNAPSHOT yesterday evening (Number 46) that should have this included for you. Getting the latest SNAPSHOT can be tricky sometimes, although your process sounds like it should have done this. Can you try the following on the command line to ensure that the latest SNAPSHOT downloads (it'll get logged in the shell as Gradle downloads the files)?

./gradlew clean --refresh-dependencies

https://github.com/mapbox/mapbox-gl-native/commits/release-android-v4.1.0

screen shot 2016-06-07 at 9 52 34 am

@Schumi09
Copy link
Author

Schumi09 commented Jun 7, 2016

Thanks @bleege 👍
Usually I don't run into these issues. Although I have deleted the cache folder yesterday it just redownloaded the wrong snapshot again. But now it did the trick it seems :)
Unfortunately, it's already evening over here in germany so I can't provide you much feedback right now.

So first of all the invalidating seems fine.

However, the above exception is still thrown for one of my vizualizations. It's happening for the MarkerViews that all use the same Icon with new MarkerViewOptions tho.

Additionally, when pressing the device's home button I currently get

06-07 17:53:07.444 3372-3372/ifgi.wayto_navigation E/AndroidRuntime: FATAL EXCEPTION: main
                                                                     Process: ifgi.wayto_navigation, PID: 3372
                                                                     java.lang.NullPointerException: Attempt to invoke virtual method 'void android.view.View.setVisibility(int)' on a null object reference
                                                                         at com.mapbox.mapboxsdk.maps.MapboxMap$MarkerViewAdapter.releaseView(MapboxMap.java:1921)
                                                                         at com.mapbox.mapboxsdk.annotations.MarkerViewManager.invalidateViewMarkersInBounds(MarkerViewManager.java:332)
                                                                         at com.mapbox.mapboxsdk.maps.MapboxMap.addMarker(MapboxMap.java:708)
                                                                         at ifgi.wayto_navigation.model.Landmark$TangiblePointer.setArrow(Landmark.java:467)
                                                                         at ifgi.wayto_navigation.model.Landmark$TangiblePointer.setLine(Landmark.java:448)
                                                                         at ifgi.wayto_navigation.model.Landmark$TangiblePointer.<init>(Landmark.java:397)
                                                                         at ifgi.wayto_navigation.model.Landmark.drawTangiblePointer(Landmark.java:477)
                                                                         at ifgi.wayto_navigation.model.Landmark.visualize(Landmark.java:161)
                                                                         at ifgi.wayto_navigation.MainActivity.landmarkVisualization(MainActivity.java:347)
                                                                         at ifgi.wayto_navigation.MainActivity.access$100(MainActivity.java:77)
                                                                         at ifgi.wayto_navigation.MainActivity$1$2.onCameraChange(MainActivity.java:231)
                                                                         at com.mapbox.mapboxsdk.maps.MapboxMap.invalidateCameraPosition(MapboxMap.java:475)
                                                                         at com.mapbox.mapboxsdk.maps.MapboxMap.moveCamera(MapboxMap.java:283)
                                                                         at com.mapbox.mapboxsdk.maps.MapboxMap.moveCamera(MapboxMap.java:266)
                                                                         at com.mapbox.mapboxsdk.maps.MapboxMap.setCameraPosition(MapboxMap.java:254)
                                                                         at ifgi.wayto_navigation.MainActivity.onLocationChanged(MainActivity.java:336)
                                                                         at ifgi.wayto_navigation.MainActivity$1$1.onMyLocationChange(MainActivity.java:220)
                                                                         at com.mapbox.mapboxsdk.maps.MapView$5.onLocationChanged(MapView.java:2400)
                                                                         at com.mapbox.mapboxsdk.location.LocationServices.onLocationChanged(LocationServices.java:138)
                                                                         at com.mapzen.android.lost.internal.FusedLocationProviderApiImpl.reportLocation(FusedLocationProviderApiImpl.java:96)
                                                                         at com.mapzen.android.lost.internal.MockEngine.setLocation(MockEngine.java:66)
                                                                         at com.mapzen.android.lost.internal.FusedLocationProviderApiImpl.setMockLocation(FusedLocationProviderApiImpl.java:82)
                                                                         at ifgi.wayto_navigation.MainActivity.mock(MainActivity.java:586)
                                                                         at ifgi.wayto_navigation.MainActivity.access$1100(MainActivity.java:77)
                                                                         at ifgi.wayto_navigation.MainActivity$4$1.run(MainActivity.java:571)
                                                                         at android.os.Handler.handleCallback(Handler.java:739)
                                                                         at android.os.Handler.dispatchMessage(Handler.java:95)
                                                                         at android.os.Looper.loop(Looper.java:148)
                                                                         at android.app.ActivityThread.main(ActivityThread.java:5417)
                                                                         at java.lang.reflect.Method.invoke(Native Method)
                                                                         at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
                                                                         at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)

Looks like a new behaviour at least, at this point I cannot assure that it's not related to my code ;)

@tobrun
Copy link
Member

tobrun commented Jun 8, 2016

@Schumi09 thank you for that feedback. looking at the stacktrace I'm guessing this has to do with Activity destroy and at the same time still doing operations related to ViewMarkers. The solution for this is making MarkerAdapter Activity context aware.(in normal cases this was onPause/onResume but with the new multi window support in Android N. I would look into onStart/onStop instead)

@Schumi09
Copy link
Author

Schumi09 commented Jun 8, 2016

@tobrun thanks for the feedback, I switched the testing device only some weeks ago and didn't focus on those things yet.

I made a mistake, so don't consider below
To point this out:

However, the above exception is still thrown for one of my vizualizations. It's happening for the MarkerViews that all use the same Icon with new MarkerViewOptions tho.
java.lang.IllegalStateException: Already in the pool! at android.support.v4.util.Pools$SimplePool.release(Pools.java:113)

I set up several MarkerViews one after another via a For-Loop basically. They all share the same Icon that I have globally stored and the difference between the MarkerViews are position, rotation, alpha so I do new MarkerViewOptions for each. This already crashes the app at first execution of this code. I had another type of vizualization with MarkerViews running properly, but there I always build entirely new drawables that become icons for every execution.

@Schumi09
Copy link
Author

Schumi09 commented Jun 8, 2016

Oh my bad, I missed that I have accidentally added the same MarkerView twice to the map that has caused the exception!
So far everything is running as expected which is amazing, so many new opportunities 👍

@Schumi09
Copy link
Author

I think all my issues here are solved, so I closed the issue 👍

@bleege
Copy link
Contributor

bleege commented Jun 27, 2016

Excellent! Glad things are working for you @Schumi09!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android annotations Annotations on iOS and macOS or markers on Android
Projects
None yet
Development

No branches or pull requests

3 participants