fix: Prevent memory leak in CompositeWithOpaqueBackground (#309)

Quoting @connyduck in https://github.com/tuskyapp/Tusky/pull/4150:

"""
The transformation ends up in Glide's memory cache and leaks whole
Activities through the view -> context reference.

This fixes the problem by removing the background detection logic (so
the view reference is no longer needed) and setting the background
directly instead. Looks exactly as before.
"""

Co-authored-by: Konrad Pozniak <opensource@connyduck.at>
This commit is contained in:
Nik Clayton 2023-12-09 18:36:49 +01:00 committed by GitHub
parent 6ee41177cd
commit d4eed2fbf8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 7 additions and 42 deletions

View File

@ -350,7 +350,7 @@ abstract class StatusBaseViewHolder protected constructor(itemView: View) :
avatar, avatar,
avatarRadius, avatarRadius,
statusDisplayOptions.animateAvatars, statusDisplayOptions.animateAvatars,
listOf(CompositeWithOpaqueBackground(avatar)), listOf(CompositeWithOpaqueBackground(MaterialColors.getColor(avatar, android.R.attr.colorBackground))),
) )
} }

View File

@ -24,12 +24,7 @@ import android.graphics.ColorMatrix
import android.graphics.ColorMatrixColorFilter import android.graphics.ColorMatrixColorFilter
import android.graphics.Paint import android.graphics.Paint
import android.graphics.Shader import android.graphics.Shader
import android.graphics.drawable.ColorDrawable import androidx.annotation.ColorInt
import android.graphics.drawable.Drawable
import android.util.TypedValue
import android.view.View
import androidx.annotation.AttrRes
import androidx.core.content.ContextCompat
import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool
import com.bumptech.glide.load.resource.bitmap.BitmapTransformation import com.bumptech.glide.load.resource.bitmap.BitmapTransformation
import com.bumptech.glide.util.Util import com.bumptech.glide.util.Util
@ -55,18 +50,18 @@ import java.security.MessageDigest
* So the partially transparent areas on the original image are composited over the original * So the partially transparent areas on the original image are composited over the original
* background, the fully transparent areas on the original image are left transparent. * background, the fully transparent areas on the original image are left transparent.
*/ */
class CompositeWithOpaqueBackground(val view: View) : BitmapTransformation() { class CompositeWithOpaqueBackground(@ColorInt val backgroundColor: Int) : BitmapTransformation() {
override fun equals(other: Any?): Boolean { override fun equals(other: Any?): Boolean {
if (other is CompositeWithOpaqueBackground) { if (other is CompositeWithOpaqueBackground) {
return other.view == view return other.backgroundColor == backgroundColor
} }
return false return false
} }
override fun hashCode() = Util.hashCode(ID.hashCode(), view.hashCode()) override fun hashCode() = Util.hashCode(ID.hashCode(), backgroundColor.hashCode())
override fun updateDiskCacheKey(messageDigest: MessageDigest) { override fun updateDiskCacheKey(messageDigest: MessageDigest) {
messageDigest.update(ID_BYTES) messageDigest.update(ID_BYTES)
messageDigest.update(ByteBuffer.allocate(4).putInt(view.hashCode()).array()) messageDigest.update(ByteBuffer.allocate(4).putInt(backgroundColor.hashCode()).array())
} }
override fun transform( override fun transform(
@ -78,20 +73,9 @@ class CompositeWithOpaqueBackground(val view: View) : BitmapTransformation() {
// If the input bitmap has no alpha channel then there's nothing to do // If the input bitmap has no alpha channel then there's nothing to do
if (!toTransform.hasAlpha()) return toTransform if (!toTransform.hasAlpha()) return toTransform
// Get the background drawable for this view, falling back to the given attribute
val backgroundDrawable = view.getFirstNonNullBackgroundOrAttr(android.R.attr.colorBackground)
backgroundDrawable ?: return toTransform
// Convert the background to a bitmap. // Convert the background to a bitmap.
val backgroundBitmap = pool.get(outWidth, outHeight, Bitmap.Config.ARGB_8888) val backgroundBitmap = pool.get(outWidth, outHeight, Bitmap.Config.ARGB_8888)
when (backgroundDrawable) { backgroundBitmap.eraseColor(backgroundColor)
is ColorDrawable -> backgroundBitmap.eraseColor(backgroundDrawable.color)
else -> {
val backgroundCanvas = Canvas(backgroundBitmap)
backgroundDrawable.setBounds(0, 0, outWidth, outHeight)
backgroundDrawable.draw(backgroundCanvas)
}
}
// Convert the alphaBitmap (where the alpha channel has 8bpp) to a mask of 1bpp // Convert the alphaBitmap (where the alpha channel has 8bpp) to a mask of 1bpp
// TODO: toTransform.extractAlpha(paint, ...) could be used here, but I can't find any // TODO: toTransform.extractAlpha(paint, ...) could be used here, but I can't find any
@ -141,24 +125,5 @@ class CompositeWithOpaqueBackground(val view: View) : BitmapTransformation() {
) )
isAntiAlias = false isAntiAlias = false
} }
/**
* @param attr attribute reference for the default drawable if no background is set on
* this view or any of its ancestors.
* @return The first non-null background drawable from this view, or its ancestors,
* falling back to the attribute resource given by `attr` if none of the views have a
* background.
*/
fun View.getFirstNonNullBackgroundOrAttr(@AttrRes attr: Int): Drawable? =
background ?: (parent as? View)?.getFirstNonNullBackgroundOrAttr(attr) ?: run {
val v = TypedValue()
context.theme.resolveAttribute(attr, v, true)
// TODO: On API 29 can use v.isColorType here
if (v.type >= TypedValue.TYPE_FIRST_COLOR_INT && v.type <= TypedValue.TYPE_LAST_COLOR_INT) {
ColorDrawable(v.data)
} else {
ContextCompat.getDrawable(context, v.resourceId)
}
}
} }
} }