Optimize Glide emoji targets (#4467)
The current code loads emojis using Glide into basic custom `Target`s and doesn't keep a hard reference to the Target. This creates a few problems: - Unlike images loaded into `ImageViewTarget`s, Emoji animations are not paused when the Activity/Fragment becomes invisible. GIF decoding use resources in the background. - When `TextView`s get recycled in a RecyclerView, the loading of emojis for the previous bind are not canceled when binding the new text and starting the load of the new emojis. This is also handled automatically when using `ImageViewTarget` but not for custom targets. Also, when the obsolete emojis complete loading, the `TextView` will be unnecessarily invalidated and redrawn. - Since Glide's `RequestManager` doesn't keep hard references to Targets after they are loaded and the emoji Target is currently not stored in any View, emojis don't get an opportunity to clean up (at least stop their animation) when the Activity/Fragment is destroyed. Depending on the Drawable implementation, animations may run forever in the background and cause memory leaks. This pull request aims to properly track the lifecycle of emoji Targets, cancel their loading an stop animations when appropriate. It also reimplements `emojify()` to be more efficient. - Add extension functions `View.setEmojiTargets()` and `View.clearEmojiTargets()` to store and clear lists of emoji targets in View tags, keeping a hard reference to them as long as the View is used. When clearing emoji targets, pending requests will be canceled and animations will be stopped to free memory. This is similar to what `ImageViewTarget` does, except here multiple Targets are stored for a single View instead of one. - Add helper extension function `View.updateEmojiTargets()` to automatically clear the View emoji targets, then allowing to call `emojify()` one or more times in the `EmojiTargetScope` of that View, and finally store all the pending targets of the `EmojiTargetScope` in the View. - Reimplement `CharSequence.emojify()` using `View.updateEmojiTargets()`. This is used in RecyclerViews as well and will automatically cancel previous emoji loadings for the same View and stop animations. - The main logic of `emojify()` has been moved to `EmojiTargetScope`. Replace usage of slow regex `Matcher` with faster `CharSequence.indexOf()`. Use `SpannableString` (with `toSpannable()`) instead of `SpannableStringBuilder` to store the `EmojiSpan`s. - Rename `EmojiSpan.getTarget()` to `EmojiSpan.createGlideTarget()` and improve the target to stop/resume the animation according to the parent component lifecycle, and stop the animation when clearing the target. Use a hard reference to the view instead of a weak reference, since the lifecycle of the Target now matches the one of the View and the Target will be cleared at the latest when the View is destroyed. - Use `View.updateEmojiTargets()` in `ReportNotificationViewHolder` in order to store the targets of 2 separate sets of emojis into the same TextView. - Fix: reimplement the code to merge the 2 emoji sets into a single `CharSequence` in `ReportNotificationViewHolder` using `TextUtils.expandTemplate()`. The current code uses `String.format()` which returns a String instead of a Spannable so the computed emojis are lost. - Store the emoji targets in `AnnouncementAdapter` in the parent view after clearing the previous ones. It is a better location than storing one emoji target in each child `Tooltip` view because tooltips are not recycled when refreshing the data and the previous targets would not be canceled properly. - Bonus: update `ViewVideoFragment` to use `CustomViewTarget` instead of `CustomTarget` to load the default artwork into `PlayerView`. The loading will also automatically be canceled when the fragment view is detached.
This commit is contained in:
parent
2cbd629150
commit
c668cdc633
|
@ -16,8 +16,9 @@
|
|||
package com.keylesspalace.tusky.components.announcements
|
||||
|
||||
import android.annotation.SuppressLint
|
||||
import android.graphics.drawable.Drawable
|
||||
import android.os.Build
|
||||
import android.text.SpannableStringBuilder
|
||||
import android.text.SpannableString
|
||||
import android.view.ContextThemeWrapper
|
||||
import android.view.LayoutInflater
|
||||
import android.view.View
|
||||
|
@ -25,6 +26,7 @@ import android.view.ViewGroup
|
|||
import androidx.core.view.size
|
||||
import androidx.recyclerview.widget.RecyclerView
|
||||
import com.bumptech.glide.Glide
|
||||
import com.bumptech.glide.request.target.Target
|
||||
import com.google.android.material.chip.Chip
|
||||
import com.keylesspalace.tusky.R
|
||||
import com.keylesspalace.tusky.databinding.ItemAnnouncementBinding
|
||||
|
@ -33,9 +35,11 @@ import com.keylesspalace.tusky.interfaces.LinkListener
|
|||
import com.keylesspalace.tusky.util.AbsoluteTimeFormatter
|
||||
import com.keylesspalace.tusky.util.BindingHolder
|
||||
import com.keylesspalace.tusky.util.EmojiSpan
|
||||
import com.keylesspalace.tusky.util.clearEmojiTargets
|
||||
import com.keylesspalace.tusky.util.emojify
|
||||
import com.keylesspalace.tusky.util.parseAsMastodonHtml
|
||||
import com.keylesspalace.tusky.util.setClickableText
|
||||
import com.keylesspalace.tusky.util.setEmojiTargets
|
||||
import com.keylesspalace.tusky.util.visible
|
||||
|
||||
interface AnnouncementActionListener : LinkListener {
|
||||
|
@ -94,6 +98,11 @@ class AnnouncementAdapter(
|
|||
// hide button if announcement badge limit is already reached
|
||||
addReactionChip.visible(item.reactions.size < 8)
|
||||
|
||||
val requestManager = Glide.with(chips)
|
||||
|
||||
chips.clearEmojiTargets()
|
||||
val targets = ArrayList<Target<Drawable>>(item.reactions.size)
|
||||
|
||||
item.reactions.forEachIndexed { i, reaction ->
|
||||
(
|
||||
chips.getChildAt(i)?.takeUnless { it.id == R.id.addReactionChip } as Chip?
|
||||
|
@ -109,13 +118,14 @@ class AnnouncementAdapter(
|
|||
} else {
|
||||
// we set the EmojiSpan on a space, because otherwise the Chip won't have the right size
|
||||
// https://github.com/tuskyapp/Tusky/issues/2308
|
||||
val spanBuilder = SpannableStringBuilder(" ${reaction.count}")
|
||||
val spannable = SpannableString(" ${reaction.count}")
|
||||
val span = EmojiSpan(this)
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
|
||||
span.contentDescription = reaction.name
|
||||
}
|
||||
spanBuilder.setSpan(span, 0, 1, 0)
|
||||
Glide.with(this)
|
||||
val target = span.createGlideTarget(this, animateEmojis)
|
||||
spannable.setSpan(span, 0, 1, 0)
|
||||
requestManager
|
||||
.asDrawable()
|
||||
.load(
|
||||
if (animateEmojis) {
|
||||
|
@ -124,8 +134,9 @@ class AnnouncementAdapter(
|
|||
reaction.staticUrl
|
||||
}
|
||||
)
|
||||
.into(span.getTarget(animateEmojis))
|
||||
this.text = spanBuilder
|
||||
.into(target)
|
||||
targets.add(target)
|
||||
this.text = spannable
|
||||
}
|
||||
|
||||
isChecked = reaction.me
|
||||
|
@ -144,11 +155,18 @@ class AnnouncementAdapter(
|
|||
chips.removeViewAt(item.reactions.size)
|
||||
}
|
||||
|
||||
// Store Glide targets for later cancellation
|
||||
chips.setEmojiTargets(targets)
|
||||
|
||||
addReactionChip.setOnClickListener {
|
||||
listener.openReactionPicker(item.id, it)
|
||||
}
|
||||
}
|
||||
|
||||
override fun onViewRecycled(holder: BindingHolder<ItemAnnouncementBinding>) {
|
||||
holder.binding.chipGroup.clearEmojiTargets()
|
||||
}
|
||||
|
||||
override fun getItemCount() = items.size
|
||||
|
||||
fun updateList(items: List<Announcement>) {
|
||||
|
|
|
@ -16,15 +16,16 @@
|
|||
package com.keylesspalace.tusky.components.notifications
|
||||
|
||||
import android.content.Context
|
||||
import android.text.TextUtils
|
||||
import androidx.recyclerview.widget.RecyclerView
|
||||
import com.keylesspalace.tusky.R
|
||||
import com.keylesspalace.tusky.databinding.ItemReportNotificationBinding
|
||||
import com.keylesspalace.tusky.interfaces.AccountActionListener
|
||||
import com.keylesspalace.tusky.util.StatusDisplayOptions
|
||||
import com.keylesspalace.tusky.util.emojify
|
||||
import com.keylesspalace.tusky.util.getRelativeTimeSpanString
|
||||
import com.keylesspalace.tusky.util.loadAvatar
|
||||
import com.keylesspalace.tusky.util.unicodeWrap
|
||||
import com.keylesspalace.tusky.util.updateEmojiTargets
|
||||
import com.keylesspalace.tusky.viewdata.NotificationViewData
|
||||
|
||||
class ReportNotificationViewHolder(
|
||||
|
@ -41,10 +42,16 @@ class ReportNotificationViewHolder(
|
|||
val report = viewData.report!!
|
||||
val reporter = viewData.account
|
||||
|
||||
val reporterName = reporter.name.unicodeWrap().emojify(reporter.emojis, binding.notificationTopText, statusDisplayOptions.animateEmojis)
|
||||
val reporteeName = report.targetAccount.name.unicodeWrap().emojify(report.targetAccount.emojis, binding.notificationTopText, statusDisplayOptions.animateEmojis)
|
||||
binding.notificationTopText.updateEmojiTargets {
|
||||
val reporterName = reporter.name.unicodeWrap().emojify(reporter.emojis, statusDisplayOptions.animateEmojis)
|
||||
val reporteeName = report.targetAccount.name.unicodeWrap().emojify(report.targetAccount.emojis, statusDisplayOptions.animateEmojis)
|
||||
|
||||
binding.notificationTopText.text = itemView.context.getString(R.string.notification_header_report_format, reporterName, reporteeName)
|
||||
// Context.getString() returns a String and doesn't support Spannable.
|
||||
// Convert the placeholders to the format used by TextUtils.expandTemplate which does.
|
||||
val topText =
|
||||
view.context.getString(R.string.notification_header_report_format, "^1", "^2")
|
||||
view.text = TextUtils.expandTemplate(topText, reporterName, reporteeName)
|
||||
}
|
||||
binding.notificationSummary.text = itemView.context.getString(R.string.notification_summary_report_format, getRelativeTimeSpanString(itemView.context, report.createdAt.time, System.currentTimeMillis()), report.statusIds?.size ?: 0)
|
||||
binding.notificationCategory.text = getTranslatedCategory(itemView.context, report.category)
|
||||
|
||||
|
|
|
@ -42,8 +42,9 @@ import androidx.media3.common.util.UnstableApi
|
|||
import androidx.media3.exoplayer.ExoPlayer
|
||||
import androidx.media3.exoplayer.util.EventLogger
|
||||
import androidx.media3.ui.AspectRatioFrameLayout
|
||||
import androidx.media3.ui.PlayerView
|
||||
import com.bumptech.glide.Glide
|
||||
import com.bumptech.glide.request.target.CustomTarget
|
||||
import com.bumptech.glide.request.target.CustomViewTarget
|
||||
import com.bumptech.glide.request.transition.Transition
|
||||
import com.google.android.material.snackbar.Snackbar
|
||||
import com.keylesspalace.tusky.BuildConfig
|
||||
|
@ -321,22 +322,26 @@ class ViewVideoFragment : ViewMediaFragment() {
|
|||
// Audio-only files might have a preview image. If they do, set it as the artwork
|
||||
if (isAudio) {
|
||||
mediaAttachment.previewUrl?.let { url ->
|
||||
Glide.with(this).load(url).into(object : CustomTarget<Drawable>() {
|
||||
@SuppressLint("SyntheticAccessor")
|
||||
override fun onResourceReady(
|
||||
resource: Drawable,
|
||||
transition: Transition<in Drawable>?
|
||||
) {
|
||||
view ?: return
|
||||
binding.videoView.defaultArtwork = resource
|
||||
}
|
||||
Glide.with(this)
|
||||
.load(url)
|
||||
.into(
|
||||
object : CustomViewTarget<PlayerView, Drawable>(binding.videoView) {
|
||||
override fun onLoadFailed(errorDrawable: Drawable?) {
|
||||
// Don't do anything
|
||||
}
|
||||
|
||||
@SuppressLint("SyntheticAccessor")
|
||||
override fun onLoadCleared(placeholder: Drawable?) {
|
||||
view ?: return
|
||||
binding.videoView.defaultArtwork = null
|
||||
}
|
||||
})
|
||||
override fun onResourceCleared(placeholder: Drawable?) {
|
||||
view.defaultArtwork = null
|
||||
}
|
||||
|
||||
override fun onResourceReady(
|
||||
resource: Drawable,
|
||||
transition: Transition<in Drawable>?
|
||||
) {
|
||||
view.defaultArtwork = resource
|
||||
}
|
||||
}.clearOnDetach()
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -21,60 +21,97 @@ import android.graphics.Canvas
|
|||
import android.graphics.Paint
|
||||
import android.graphics.drawable.Animatable
|
||||
import android.graphics.drawable.Drawable
|
||||
import android.text.SpannableStringBuilder
|
||||
import android.text.style.ReplacementSpan
|
||||
import android.view.View
|
||||
import android.widget.TextView
|
||||
import androidx.core.text.toSpannable
|
||||
import com.bumptech.glide.Glide
|
||||
import com.bumptech.glide.request.target.CustomTarget
|
||||
import com.bumptech.glide.request.target.Target
|
||||
import com.bumptech.glide.request.transition.Transition
|
||||
import com.keylesspalace.tusky.R
|
||||
import com.keylesspalace.tusky.entity.Emoji
|
||||
import java.lang.ref.WeakReference
|
||||
import java.util.regex.Pattern
|
||||
|
||||
/**
|
||||
* replaces emoji shortcodes in a text with EmojiSpans
|
||||
* @receiver the text containing custom emojis
|
||||
* @param emojis a list of the custom emojis (nullable for backward compatibility with old mastodon instances)
|
||||
* @param emojis a list of the custom emojis
|
||||
* @param view a reference to the a view the emojis will be shown in (should be the TextView, but parents of the TextView are also acceptable)
|
||||
* @return the text with the shortcodes replaced by EmojiSpans
|
||||
*/
|
||||
fun CharSequence.emojify(emojis: List<Emoji>, view: View, animate: Boolean): CharSequence {
|
||||
if (emojis.isEmpty()) {
|
||||
return this
|
||||
return view.updateEmojiTargets {
|
||||
emojify(emojis, animate)
|
||||
}
|
||||
}
|
||||
|
||||
val builder = SpannableStringBuilder.valueOf(this)
|
||||
class EmojiTargetScope<T : View>(val view: T) {
|
||||
private val _targets = mutableListOf<Target<Drawable>>()
|
||||
val targets: List<Target<Drawable>>
|
||||
get() = _targets
|
||||
|
||||
emojis.forEach { (shortcode, url, staticUrl) ->
|
||||
val matcher = Pattern.compile(":$shortcode:", Pattern.LITERAL)
|
||||
.matcher(this)
|
||||
|
||||
while (matcher.find()) {
|
||||
val span = EmojiSpan(view)
|
||||
|
||||
builder.setSpan(span, matcher.start(), matcher.end(), 0)
|
||||
Glide.with(view)
|
||||
.asDrawable()
|
||||
.load(
|
||||
if (animate) {
|
||||
url
|
||||
} else {
|
||||
staticUrl
|
||||
}
|
||||
)
|
||||
.into(span.getTarget(animate))
|
||||
fun CharSequence.emojify(emojis: List<Emoji>, animate: Boolean): CharSequence {
|
||||
if (emojis.isEmpty()) {
|
||||
return this
|
||||
}
|
||||
|
||||
val spannable = toSpannable()
|
||||
val requestManager = Glide.with(view)
|
||||
|
||||
emojis.forEach { (shortcode, url, staticUrl) ->
|
||||
val pattern = ":$shortcode:"
|
||||
var start = indexOf(pattern)
|
||||
|
||||
while (start != -1) {
|
||||
val end = start + pattern.length
|
||||
val span = EmojiSpan(view)
|
||||
|
||||
spannable.setSpan(span, start, end, 0)
|
||||
val target = span.createGlideTarget(view, animate)
|
||||
requestManager
|
||||
.asDrawable()
|
||||
.load(
|
||||
if (animate) {
|
||||
url
|
||||
} else {
|
||||
staticUrl
|
||||
}
|
||||
)
|
||||
.into(target)
|
||||
_targets.add(target)
|
||||
|
||||
start = indexOf(pattern, end)
|
||||
}
|
||||
}
|
||||
|
||||
return spannable
|
||||
}
|
||||
return builder
|
||||
}
|
||||
|
||||
inline fun <T : View, R> T.updateEmojiTargets(body: EmojiTargetScope<T>.() -> R): R {
|
||||
clearEmojiTargets()
|
||||
val scope = EmojiTargetScope(this)
|
||||
val result = body(scope)
|
||||
setEmojiTargets(scope.targets)
|
||||
return result
|
||||
}
|
||||
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
fun View.clearEmojiTargets() {
|
||||
getTag(R.id.custom_emoji_targets_tag)?.let { tag ->
|
||||
val targets = tag as List<Target<Drawable>>
|
||||
val requestManager = Glide.with(this)
|
||||
targets.forEach { requestManager.clear(it) }
|
||||
setTag(R.id.custom_emoji_targets_tag, null)
|
||||
}
|
||||
}
|
||||
|
||||
fun View.setEmojiTargets(targets: List<Target<Drawable>>) {
|
||||
setTag(R.id.custom_emoji_targets_tag, targets.takeIf { it.isNotEmpty() })
|
||||
}
|
||||
|
||||
class EmojiSpan(view: View) : ReplacementSpan() {
|
||||
|
||||
private val viewWeakReference = WeakReference(view)
|
||||
|
||||
private val emojiSize: Int = if (view is TextView) {
|
||||
view.paint.textSize
|
||||
} else {
|
||||
|
@ -147,34 +184,52 @@ class EmojiSpan(view: View) : ReplacementSpan() {
|
|||
}
|
||||
}
|
||||
|
||||
fun getTarget(animate: Boolean): Target<Drawable> {
|
||||
fun createGlideTarget(view: View, animate: Boolean): Target<Drawable> {
|
||||
return object : CustomTarget<Drawable>(emojiSize, emojiSize) {
|
||||
override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
|
||||
viewWeakReference.get()?.let { view ->
|
||||
if (animate && resource is Animatable) {
|
||||
val callback = resource.callback
|
||||
|
||||
resource.callback = object : Drawable.Callback {
|
||||
override fun unscheduleDrawable(p0: Drawable, p1: Runnable) {
|
||||
callback?.unscheduleDrawable(p0, p1)
|
||||
}
|
||||
override fun scheduleDrawable(p0: Drawable, p1: Runnable, p2: Long) {
|
||||
callback?.scheduleDrawable(p0, p1, p2)
|
||||
}
|
||||
override fun invalidateDrawable(p0: Drawable) {
|
||||
callback?.invalidateDrawable(p0)
|
||||
view.invalidate()
|
||||
}
|
||||
}
|
||||
resource.start()
|
||||
}
|
||||
|
||||
imageDrawable = resource
|
||||
view.invalidate()
|
||||
}
|
||||
override fun onStart() {
|
||||
(imageDrawable as? Animatable)?.start()
|
||||
}
|
||||
|
||||
override fun onLoadCleared(placeholder: Drawable?) {}
|
||||
override fun onStop() {
|
||||
(imageDrawable as? Animatable)?.stop()
|
||||
}
|
||||
|
||||
override fun onLoadFailed(errorDrawable: Drawable?) {
|
||||
// Nothing to do
|
||||
}
|
||||
|
||||
override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
|
||||
if (animate && resource is Animatable) {
|
||||
resource.callback = object : Drawable.Callback {
|
||||
override fun invalidateDrawable(who: Drawable) {
|
||||
view.invalidate()
|
||||
}
|
||||
|
||||
override fun scheduleDrawable(who: Drawable, what: Runnable, `when`: Long) {
|
||||
view.postDelayed(what, `when`)
|
||||
}
|
||||
|
||||
override fun unscheduleDrawable(who: Drawable, what: Runnable) {
|
||||
view.removeCallbacks(what)
|
||||
}
|
||||
}
|
||||
resource.start()
|
||||
}
|
||||
|
||||
imageDrawable = resource
|
||||
view.invalidate()
|
||||
}
|
||||
|
||||
override fun onLoadCleared(placeholder: Drawable?) {
|
||||
imageDrawable?.let { currentDrawable ->
|
||||
if (currentDrawable is Animatable) {
|
||||
currentDrawable.stop()
|
||||
currentDrawable.callback = null
|
||||
}
|
||||
}
|
||||
imageDrawable = null
|
||||
view.invalidate()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1,4 +1,5 @@
|
|||
<?xml version="1.0" encoding="utf-8"?>
|
||||
<resources>
|
||||
<item name="pin" type="id" />
|
||||
<item name="custom_emoji_targets_tag" type="id" />
|
||||
</resources>
|
Loading…
Reference in New Issue