Coil: PhotoView + Coil + crossfade = no pinch-to-zoom

[JeremiahStephenson]

Describe the bug
When using Chris Banes' PhotoView library with Coil + crossfade the pinch-to-zoom is broken. This appears to be due to CrossfadeDrawable updating the loaded drawable's bounds. Unless I'm mistaken I only see two options to get around this. Either disable crossfade for PhotoView or create my own transition.

Expected behavior
Pinch-to-zoom continues to function with crossfade enabled and without creating custom transition.

To Reproduce
Load an image into a PhotoView using Coil with crossfade enabled.

Version
This is with version 0.9.1 of Coil

[colinrtwhite]

Thanks for the report. @Jawnnypoo any ideas (since you're maintaining PhotoView)?

The behaviour of CrossfadeDrawable changed slightly in 0.9.0 to better support crossfading between drawables with no intrinsic dimensions (like a ColorDrawable) and drawables with dimensions.

CrossfadeDrawable will have a width/height of -1 if either of its children have that dimension as -1. Code.

The child drawable's bounds will be centered in the middle of the CrossfadeDrawable's bounds. They're scaled at draw-time if it's not a BitmapDrawable (since BitmapDrawables do their own scaling). Code.

[Jawnnypoo]

Hey @JeremiahStephenson , I added Coil to the PhotoView sample on master, and I don't see any issues currently. Can you potentially make a PR on that repo that shows the issue, or post code changes in here needed to make this break? https://github.com/chrisbanes/PhotoView/blob/master/sample/src/main/java/com/github/chrisbanes/photoview/sample/CoilSampleActivity.kt

[JeremiahStephenson]

@Jawnnypoo The sample works because the multiplier value in CrossfadeDrawable.updateBounds is always 1. In my scenario that's coming out as 1.2 and that leads to the issue. I'll try and duplicate it in your sample so you can see what I mean.

[JeremiahStephenson]

Turns out that my drawable is being loaded at a shorter width than it should be and that is what leads to multiplier being higher than 1. I'm not entirely sure what's causing it yet. I suspect it has something to do with my loading drawable. I'll investigate more tomorrow.

[JeremiahStephenson]

@Jawnnypoo @colinrtwhite
It is an issue with having a custom loading drawable with the crossfade animation. The loading drawable sets the target bounds on the PhotoView which is what leads to the multiplier value being greater than 1 for the end image in the Crossfade drawable.
I was able to break the sample by using this below as the loading drawable.
You also have to make sure it's loading from network and not from memory otherwise it skips the loading drawable and the crossfade.

class LoadingDrawable(context: Context) : ColorDrawable(ContextCompat.getColor(context, R.color.blue)), Drawable.Callback {

    private val animatedDrawable = CircularProgressDrawable(context)

    init {
        animatedDrawable.setStyle(CircularProgressDrawable.LARGE)
        animatedDrawable.setColorSchemeColors(ContextCompat.getColor(context, R.color.green))
        animatedDrawable.callback = this
        animatedDrawable.start()
    }

    override fun onBoundsChange(bounds: Rect) {
        super.onBoundsChange(bounds)

        val width = min(bounds.width(), animatedDrawable.intrinsicWidth)
        val height = min(bounds.height(), animatedDrawable.intrinsicHeight)
        val left = (bounds.width() - width) / 2
        val top = (bounds.height() - height) / 2
        animatedDrawable.setBounds(left, top, left + width, top + height)
    }

    override fun draw(canvas: Canvas) {
        super.draw(canvas)
        animatedDrawable.draw(canvas)
    }

    override fun unscheduleDrawable(who: Drawable, what: Runnable) {
        unscheduleSelf(what)
    }

    override fun invalidateDrawable(who: Drawable) {
        invalidateSelf()
    }

    override fun scheduleDrawable(who: Drawable, what: Runnable, `when`: Long) {
        scheduleSelf(what, `when`)
    }
}

[JeremiahStephenson]

If I create a custom transition that uses the CrossfadeDrawable from 0.8.0 then everything works fine.

[colinrtwhite]

The behaviour change in 0.9.0 was likely due to https://github.com/coil-kt/coil/commit/6c2adb2a0f8fffed2475831cd66debbe616338ce. Looking into if it's possible to keep the desired behaviour for both crossfading into PhotoView and also crossfading with drawables with no intrinsic size.

[mikepenz]

Also had to disable crossfading for now as it seems to break in various situations as noted in this issue, and the linked one.

Especially when using in the PhotoView, and in cases where we would adjust the bounds based on the image which get loaded.

[colinrtwhite]

Confirmed that this is due CrossfadeDrawable returning -1 for its intrinsic dimensions if either its start or end drawable also returns -1. Unfortunately, I'm not sure that changing back to the old behaviour (where both drawables must have intrinsic dimensions of -1 for CrossfadeDrawable to return -1), as it causes the drawable with -1 intrinsic size to not fill an ImageView:

OLD (left) - NEW (right)

Personally, I'd expect ColorDrawable to fill the view if used as a placeholder, as it fills the view if set manually using ImageView.setImageDrawable.

[colinrtwhite]

As of Coil 1.1 you'll be able to set transition(CrossfadeTransition(preferExactIntrinsicSize = true)) when loading an image into a PhotoView/CircleImageView/any other view that requires a drawable with a non-'-1' intrinsic size. This should fix the pinch-to-zoom issue.