android-maps-utils icon indicating copy to clipboard operation
android-maps-utils copied to clipboard

DefalutClusterRender crash sometimes in some Devices

Open AnD010 opened this issue 4 years ago • 14 comments

Hi! I have in my app a painful error

image

image

I can´t reproduce this error, either in the emulators or in my physical devices. My app has 20% users with this crash about google maps

I have my app made in kotlin. Two days ago update to the last version maps-utils, but the error persists. Now I use this maps utils version com.google.maps.android:android-maps-utils:1.3.3

My DefaultClusterRender is

    override fun shouldRenderAsCluster(cluster: Cluster<T>): Boolean {
        return cluster!!.size >= 2
    }

    override fun onBeforeClusterItemRendered(item: T, markerOptions: MarkerOptions) {
        item?.let {item ->
            val  markerItem =  item as? ProfileClusterMarker
            markerItem?.let {
                markerOptions.icon(it.icon)
                markerOptions.title(it.title)
                markerOptions.title(it.snippet)

            }
        }
    }

In previous maps utils version, I had to override the method with the nullable property because always get any crash in my app (in the new version is not possible this)

//    override fun onBeforeClusterItemRendered(item: T?, markerOptions: MarkerOptions?) {
//        item?.let {item ->
//            val  markerItem =  item as? ProfileClusterMarker
//            markerItem?.let {
//                markerOptions?.icon(it.icon)
//                markerOptions?.title(it.title)
//                markerOptions?.title(it.snippet)
//
//            }
//        }
//
//    }

//    override fun onClusterItemUpdated(item: T, marker: Marker?) {
//        var changed = false
//        // Update marker text if the item text changed - same logic as adding marker in CreateMarkerTask.perform()
//        // Update marker text if the item text changed - same logic as adding marker in CreateMarkerTask.perform()
//        if (item!!.title != null && item!!.snippet != null) {
//            if (!item!!.title.equals(marker!!.title)) {
//                marker!!.title = item!!.title
//                changed = true
//            }
//            if (!item!!.snippet.equals(marker!!.snippet)) {
//                marker!!.snippet = item!!.snippet
//                changed = true
//            }
//        } else if (item!!.snippet != null && !item!!.snippet.equals(marker!!.title)) {
//            marker!!.title = item!!.snippet
//            changed = true
//        } else if (item!!.title != null && !item!!.title.equals(marker!!.title)) {
//            marker!!.title = item!!.title
//            changed = true
//        }
//        // Update marker position if the item changed position
//        // Update marker position if the item changed position
//        if (!marker!!.position.equals(item!!.position)) {
//            marker!!.position = item!!.position
//            changed = true
//        }
//        if (changed && marker!!.isInfoWindowShown) {
//            // Force a refresh of marker info window contents
//            marker!!.showInfoWindow()
//        }
//    }

AnD010 avatar Jun 10 '20 08:06 AnD010

Hi @AnD010, a reproducible sample will certainly help us resolve this issue. If you are updating Cluster and/or ClusterItem icon/text, note that since V1 you need to override onClusterItemUpdated as well. See: https://github.com/googlemaps/android-maps-utils#clustering

Perhaps this StackOverflow post might help: https://stackoverflow.com/a/47057898

arriolac avatar Jun 10 '20 16:06 arriolac

Hi @arriolac , I think that I don´t need onClusterItemUpdated. My app functionality paint X elements on the map, then when you move over the map and X distance is passed then I clear all elements over the map,

  clusterManager?.clearItems()
  map?.clear() 

And then call to my server and paint again the new elements. But once the elements were painted, we don't change it. Only show info when markers or minimum cluster are clicked. We need implements onClusterItemUpdated ??? I will try to reproduce this issue but as I said... in my emulator or real device works perfectly...

Thanks

AnD010 avatar Jun 10 '20 16:06 AnD010

Were you able to reproduce the error? Also, based on the snippet you shared, I think what you actually want is to do the following when you are clearing the map:

clusterManager.clearItems()
clusterManager.cluster()

Basically, try not invoking map.clear() and only rely on ClusterManager APIs to accomplish what you need.

arriolac avatar Jun 18 '20 17:06 arriolac

Hi @arriolac I can´t reproduce in my devices the error but still happened to others users in my production code. I will change my code and remove clear the map and call only clear items and cluster with clusterManager. I upload a new version for testers and check if the error persists

AnD010 avatar Jun 22 '20 19:06 AnD010

Hello! I'm getting a similar crash from our production app with the following devices:

  • LG Stylo 5
  • Galaxy S9
  • Galaxy A10e
  • Motorola E6
  • Unimax
  • Tinno Mobile

The stack trace is below:

Fatal Exception: java.lang.IllegalArgumentException: Unmanaged descriptor
       at com.google.maps.api.android.lib6.common.m.d(m.java)
       at com.google.maps.api.android.lib6.impl.q.c(q.java:1)
       at com.google.maps.api.android.lib6.impl.cy.a(cy.java:20)
       at com.google.android.gms.maps.model.internal.o.a(o.java:19)
       at cy.onTransact(cy.java:4)
       at android.os.Binder.transact(Binder.java:667)
       at com.google.android.gms.internal.maps.zza.zzb(zza.java:20)
       at com.google.android.gms.internal.maps.zzv.zzg(zzv.java:80)
       at com.google.android.gms.maps.model.Marker.setIcon(Marker.java:30)
       at com.google.maps.android.clustering.view.DefaultClusterRenderer.onClusterUpdated(DefaultClusterRenderer.java:882)
       at com.google.maps.android.clustering.view.DefaultClusterRenderer$CreateMarkerTask.perform(DefaultClusterRenderer.java:998)
       at com.google.maps.android.clustering.view.DefaultClusterRenderer$CreateMarkerTask.access$2000(DefaultClusterRenderer.java:937)
       at com.google.maps.android.clustering.view.DefaultClusterRenderer$MarkerModifier.performNextTask(DefaultClusterRenderer.java:654)
       at com.google.maps.android.clustering.view.DefaultClusterRenderer$MarkerModifier.handleMessage(DefaultClusterRenderer.java:623)
       at android.os.Handler.dispatchMessage(Handler.java:106)
       at android.os.Looper.loop(Looper.java:193)
       at android.app.ActivityThread.main(ActivityThread.java:6725)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:911)

I am using clusterManager.clear() for removing clusters and though it seems redundant but I'm doing map.clear() for removing polygons and individual markers (not included in the cluster). I will add the onBeforeClusterRendered(), onClusterUpdated() and onClusterItemUpdated() methods. and see if it updates anything about the issue though I am not quite sure why it is needed as it is working fine without overriding those methods.

Greeeeyyyss avatar Jun 23 '20 03:06 Greeeeyyyss

Hi @Greeeeyyyss you´re right. I need map.clear() to remove individual markers, I check this point in my code. Actually only I have override

    override fun shouldRenderAsCluster(cluster: Cluster<T>): Boolean {
        return cluster!!.size >= 2
    }

    override fun onBeforeClusterItemRendered(item: T, markerOptions: MarkerOptions) {
        item?.let {item ->
            val  markerItem =  item as? ProfileClusterMarker
            markerItem?.let {
                markerOptions?.icon(it.icon)
                markerOptions?.title(it.title)
                markerOptions?.title(it.snippet)

            }
        }
    }

@arriolac @Greeeeyyyss are you think it is necessary to override these methods (I only manipulate cluster for change icon when painting it, no update or change cluster once painted) ?? onBeforeClusterRendered onClusterUpdated

AnD010 avatar Jun 23 '20 08:06 AnD010

@Greeeeyyyss at what point are you calling clear() on the map? Based on the stack trace you and @AnD010 shared, it seems like there is a cached BitmapDescriptor that is no longer managed as the marker has been removed from the map. Are you able to share a reproducible sample or at least describe the order on when you are calling map.clear() and/or clusterManager.cluster()?

One possible workaround here I think is to override onClusterUpdated and set a custom icon on the cluster rather than using a cached BitmapDescriptor in the library. You can take a look at CustomMarkerClusteringDemoActivity as an example.

arriolac avatar Jun 23 '20 16:06 arriolac

@barbeau do you think this line can be removed? https://github.com/googlemaps/android-maps-utils/blob/acfdec032aee78e76741aa4221bd8c386ee1dd32/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java#L870 Or does it always need to be called since the cluster size may have changed?

arriolac avatar Jun 23 '20 16:06 arriolac

@arriolac It needs to stay, because as you say the contents of the cluster (and size) may have changed. The default implementation of onClusterUpdated() needs to match the default implementation of onBeforeClusterRendered() for the icons to always match the internal data model given the current caching implementation.

As you mentioned in https://github.com/googlemaps/android-maps-utils/issues/746#issuecomment-646211580, my guess is that the underlying issue here is that the call directly to GoogleMap.clear() is removing a marker that's being managed/cached by the ClusterManager/Renderer, and then when the ClusterRenderer tries to update the state of that marker (here, update the icon) it throws this exception. So the workaround/solution would be to use the APIs on the ClusterManager and, if other markers are on the map, the MarkerManager rather than interacting with the GoogleMap directly.

barbeau avatar Jun 23 '20 17:06 barbeau

@arriolac We were not able to reproduce this issue on our available devices but the order would be like this:

clusterManager.clearItems()
clusterManager.cluster()
map.clear()

We also don't use a custom icon for cluster just the default. I think I'll try to use the MarkerManager for clearing individual markers and calling remove() on individual polygons instead of GoogleMap.clear() as a workaround.

Greeeeyyyss avatar Jun 24 '20 00:06 Greeeeyyyss

Hi! @arriolac @barbeau it is not completely clear to me If I have for example 5k individual markers and 10k into clusters (because zoom level) around the world, I need to save it in a list, and then when I need to clean the map need to iterate my list to remove one by one these markers? Because I see that function removes from MarkerManager is public boolean remove(O object). And if the zoom change and some individual marker are now into a ClusterManager? How control it? I was able to talk to two users, the first happened just after opening the map, the second happened after using the map for 10min. They both tell me it doesn't always happen to them.

My delete implementation now is the same as @Greeeeyyyss (I didn't call before clusterManager.cluster() when the map was clear) but I use custom icons for clusters

AnD010 avatar Jun 24 '20 06:06 AnD010

@Greeeeyyyss great! Please keep us posted if that resolves your issue.

arriolac avatar Jun 24 '20 15:06 arriolac

@AnD010 in that hypothetical scenario, have both 5k markers and 10k markers been added to the ClusterManager via addItems/addItem (i.e. 5k are not clustered, and 10k are because of the zoom level)? Or, are only 10k markers added to ClusterManager and the other 5k are always non-clustered items added via a separate MarkerManager from the internal MarkerManager in ClusterManager?

If you can share some more code snippets that would be helpful.

To answer your question though, yes, you will need to invoke remove() on the MarkerManager one by one since that is currently the API. It wouldn't be unreasonable to add a removeAll method though.

arriolac avatar Jun 24 '20 15:06 arriolac

Hi, I got similar crash before. In my case, I used Fragment and added cluster items at onMapReady() and called map.clear() at onDestroyView(). However, there might be some case that onMapReady() is called after onDestroyView() and mCreateMarkerTasks in DefaultClusterRenderer still executes perform() which causes the crash.

Here, as a workaround, I have to check Fragment isAdded && !isRemoving && !isDetached && view != null at onMapReady() callback.

Hope this helps.

imunique-ZJ avatar Jul 07 '20 10:07 imunique-ZJ

Is this still an issue? It looks like nobody has reported this crash in 3 years so I am closing it as obsolete if nobody is observing problems with recent versions.

wangela avatar Nov 17 '23 19:11 wangela