Skip to content

Commit ff43954

Browse files
committed
Improve cache of drawables used for rendering location pin.
In particular, use the Glide cache, and ensure that if an error occurs and later the avatar can be retrieved, the cache will be replaced. Also limit cache size to 32. Also use UserItem as a key, instead of just the userId, so that if displayName or avatarUrl change, there will be not cache hit.
1 parent 3434687 commit ff43954

File tree

1 file changed

+25
-13
lines changed

1 file changed

+25
-13
lines changed

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package im.vector.app.features.home.room.detail.timeline.helper
1919
import android.content.Context
2020
import android.graphics.drawable.Drawable
2121
import android.graphics.drawable.LayerDrawable
22-
import androidx.annotation.ColorInt
22+
import android.util.LruCache
2323
import androidx.core.content.ContextCompat
2424
import androidx.core.graphics.drawable.DrawableCompat
2525
import com.bumptech.glide.request.target.CustomTarget
@@ -30,11 +30,17 @@ import im.vector.app.core.glide.GlideApp
3030
import im.vector.app.core.utils.DimensionConverter
3131
import im.vector.app.features.home.AvatarRenderer
3232
import org.matrix.android.sdk.api.session.getUserOrDefault
33+
import org.matrix.android.sdk.api.util.MatrixItem
3334
import org.matrix.android.sdk.api.util.toMatrixItem
3435
import timber.log.Timber
3536
import javax.inject.Inject
3637
import javax.inject.Singleton
3738

39+
private data class CachedDrawable(
40+
val drawable: Drawable,
41+
val isError: Boolean,
42+
)
43+
3844
@Singleton
3945
class LocationPinProvider @Inject constructor(
4046
private val context: Context,
@@ -43,7 +49,7 @@ class LocationPinProvider @Inject constructor(
4349
private val avatarRenderer: AvatarRenderer,
4450
private val matrixItemColorProvider: MatrixItemColorProvider
4551
) {
46-
private val cache = mutableMapOf<String, Drawable>()
52+
private val cache = LruCache<MatrixItem.UserItem, CachedDrawable>(32)
4753

4854
private val glideRequests by lazy {
4955
GlideApp.with(context)
@@ -60,23 +66,16 @@ class LocationPinProvider @Inject constructor(
6066
return
6167
}
6268

63-
if (cache.contains(userId)) {
64-
callback(cache[userId]!!)
65-
return
66-
}
67-
6869
activeSessionHolder
6970
.getActiveSession()
7071
.getUserOrDefault(userId)
7172
.toMatrixItem()
7273
.let { userItem ->
7374
val size = dimensionConverter.dpToPx(44)
74-
val bgTintColor = matrixItemColorProvider.getColor(userItem)
7575
avatarRenderer.render(glideRequests, userItem, object : CustomTarget<Drawable>(size, size) {
7676
override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
7777
Timber.d("## Location: onResourceReady")
78-
val pinDrawable = createPinDrawable(resource, bgTintColor)
79-
cache[userId] = pinDrawable
78+
val pinDrawable = createPinDrawable(userItem, resource, isError = false)
8079
callback(pinDrawable)
8180
}
8281

@@ -87,17 +86,29 @@ class LocationPinProvider @Inject constructor(
8786
}
8887

8988
override fun onLoadFailed(errorDrawable: Drawable?) {
89+
// Note: `onLoadFailed` is also called when the user has no avatarUrl
90+
// and the errorDrawable is actually the placeholder.
9091
Timber.w("## Location: onLoadFailed")
9192
errorDrawable ?: return
92-
val pinDrawable = createPinDrawable(errorDrawable, bgTintColor)
93-
cache[userId] = pinDrawable
93+
val pinDrawable = createPinDrawable(userItem, errorDrawable, isError = true)
9494
callback(pinDrawable)
9595
}
9696
})
9797
}
9898
}
9999

100-
private fun createPinDrawable(drawable: Drawable, @ColorInt bgTintColor: Int): Drawable {
100+
private fun createPinDrawable(
101+
userItem: MatrixItem.UserItem,
102+
drawable: Drawable,
103+
isError: Boolean,
104+
): Drawable {
105+
val fromCache = cache.get(userItem)
106+
// Return the cached drawable only if it is valid, or the new drawable is again an error
107+
if (fromCache != null && (!fromCache.isError || isError)) {
108+
return fromCache.drawable
109+
}
110+
111+
val bgTintColor = matrixItemColorProvider.getColor(userItem)
101112
val bgUserPin = ContextCompat.getDrawable(context, R.drawable.bg_map_user_pin)!!
102113
// use mutate on drawable to avoid sharing the color when we have multiple different user pins
103114
DrawableCompat.setTint(bgUserPin.mutate(), bgTintColor)
@@ -106,6 +117,7 @@ class LocationPinProvider @Inject constructor(
106117
val topInset = dimensionConverter.dpToPx(4)
107118
val bottomInset = dimensionConverter.dpToPx(8)
108119
layerDrawable.setLayerInset(1, horizontalInset, topInset, horizontalInset, bottomInset)
120+
cache.put(userItem, CachedDrawable(layerDrawable, isError))
109121
return layerDrawable
110122
}
111123
}

0 commit comments

Comments
 (0)