Fix image viewer & transitions (#1448)

This commit is contained in:
Ivan Kupalov 2019-08-17 20:05:24 +02:00 committed by Konrad Pozniak
parent d13a341a35
commit a3fa0647b6
2 changed files with 69 additions and 19 deletions

View File

@ -17,6 +17,7 @@ package com.keylesspalace.tusky.fragment
import android.animation.Animator import android.animation.Animator
import android.animation.AnimatorListenerAdapter import android.animation.AnimatorListenerAdapter
import android.annotation.SuppressLint
import android.content.Context import android.content.Context
import android.graphics.drawable.Drawable import android.graphics.drawable.Drawable
import android.os.Bundle import android.os.Bundle
@ -51,6 +52,11 @@ class ViewImageFragment : ViewMediaFragment() {
private lateinit var photoActionsListener: PhotoActionsListener private lateinit var photoActionsListener: PhotoActionsListener
private lateinit var toolbar: View private lateinit var toolbar: View
private var transition = BehaviorSubject.create<Unit>() private var transition = BehaviorSubject.create<Unit>()
private var shouldStartTransition = false
// Volatile: Image requests happen on background thread and we want to see updates to it
// immediately on another thread. Atomic is an overkill for such thing.
@Volatile
private var startedTransition = false
override lateinit var descriptionView: TextView override lateinit var descriptionView: TextView
override fun onAttach(context: Context) { override fun onAttach(context: Context) {
@ -78,11 +84,13 @@ class ViewImageFragment : ViewMediaFragment() {
} }
} }
startedTransition = false
loadImageFromNetwork(url, previewUrl, photoView) loadImageFromNetwork(url, previewUrl, photoView)
} }
override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View { override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
toolbar = activity!!.toolbar toolbar = activity!!.toolbar
this.transition = BehaviorSubject.create()
return inflater.inflate(R.layout.fragment_view_image, container, false) return inflater.inflate(R.layout.fragment_view_image, container, false)
} }
@ -91,6 +99,7 @@ class ViewImageFragment : ViewMediaFragment() {
val arguments = this.arguments!! val arguments = this.arguments!!
val attachment = arguments.getParcelable<Attachment>(ARG_ATTACHMENT) val attachment = arguments.getParcelable<Attachment>(ARG_ATTACHMENT)
this.shouldStartTransition = arguments.getBoolean(ARG_START_POSTPONED_TRANSITION)
val url: String? val url: String?
var description: String? = null var description: String? = null
@ -129,6 +138,7 @@ class ViewImageFragment : ViewMediaFragment() {
override fun onDestroyView() { override fun onDestroyView() {
Glide.with(this).clear(photoView) Glide.with(this).clear(photoView)
transition.onComplete()
super.onDestroyView() super.onDestroyView()
} }
@ -160,6 +170,23 @@ class ViewImageFragment : ViewMediaFragment() {
} }
/** /**
* We start transition as soon as we think reasonable but we must take care about couple of
* things>
* - Do not change image in the middle of transition. It messes up the view.
* - Do not transition for the views which don't require it. Starting transition from
* multiple fragments does weird things
* - Do not wait to transition until the image loads from network
*
* Preview, cached image, network image, x - failed, o - succeeded
* P C N - start transition after...
* x x x - the cache fails
* x x o - the cache fails
* x o o - the cache succeeds
* o x o - the preview succeeds. Do not start on cache.
* o o o - the preview succeeds. Do not start on cache.
*
* So start transition after the first success or after anything with the cache
*
* @param isCacheRequest - is this listener for request image from cache or from the network * @param isCacheRequest - is this listener for request image from cache or from the network
*/ */
private inner class ImageRequestListener( private inner class ImageRequestListener(
@ -168,32 +195,46 @@ class ViewImageFragment : ViewMediaFragment() {
override fun onLoadFailed(e: GlideException?, model: Any, target: Target<Drawable>, override fun onLoadFailed(e: GlideException?, model: Any, target: Target<Drawable>,
isFirstResource: Boolean): Boolean { isFirstResource: Boolean): Boolean {
// If cache for full image failed, complete transition // If cache for full image failed complete transition
if (isCacheRequest && !isThumnailRequest) photoActionsListener.onBringUp() if (isCacheRequest && !isThumnailRequest && shouldStartTransition
&& !startedTransition) {
photoActionsListener.onBringUp()
}
// Hide progress bar only on fail request from internet // Hide progress bar only on fail request from internet
if (!isCacheRequest) progressBar?.hide() if (!isCacheRequest) progressBar?.hide()
return false // We don't want to overwrite preview with null when main image fails to load
return !isCacheRequest
} }
@SuppressLint("CheckResult")
override fun onResourceReady(resource: Drawable, model: Any, target: Target<Drawable>, override fun onResourceReady(resource: Drawable, model: Any, target: Target<Drawable>,
dataSource: DataSource, isFirstResource: Boolean): Boolean { dataSource: DataSource, isFirstResource: Boolean): Boolean {
progressBar?.hide() // Always hide the progress bar on success progressBar?.hide() // Always hide the progress bar on success
if (isThumnailRequest) {
photoView.post {
target.onResourceReady(resource, null)
photoActionsListener.onBringUp()
}
} else {
transition
.take(1)
.subscribe {
target.onResourceReady(resource, null)
photoActionsListener.onBringUp()
}
}
return true
}
if (!startedTransition || !shouldStartTransition) {
// Set this right away so that we don't have to concurrent post() requests
startedTransition = true
// post() because load() replaces image with null. Sometimes after we set
// the thumbnail.
photoView.post {
target.onResourceReady(resource, null)
if (shouldStartTransition) photoActionsListener.onBringUp()
}
} else {
// This wait for transition. If there's no transition then we should hit
// another branch. take() will unsubscribe after we have it to not leak menmory
transition
.take(1)
.subscribe {
target.onResourceReady(resource, null)
// It's needed. Don't ask why, I don't know, setImageDrawable() should
// do it by itself but somehow it doesn't work automatically.
// Just do it. If you don't, image will jump around when touched.
attacher.update()
}
}
return true
}
} }
override fun onTransitionEnd() { override fun onTransitionEnd() {

View File

@ -16,6 +16,7 @@ class ImagePagerAdapter(
) : FragmentStatePagerAdapter(fragmentManager), SharedElementTransitionListener { ) : FragmentStatePagerAdapter(fragmentManager), SharedElementTransitionListener {
private var primaryItem: ViewMediaFragment? = null private var primaryItem: ViewMediaFragment? = null
private var didTransition = false
override fun setPrimaryItem(container: ViewGroup, position: Int, item: Any) { override fun setPrimaryItem(container: ViewGroup, position: Int, item: Any) {
super.setPrimaryItem(container, position, item) super.setPrimaryItem(container, position, item)
@ -24,7 +25,14 @@ class ImagePagerAdapter(
override fun getItem(position: Int): Fragment { override fun getItem(position: Int): Fragment {
return if (position >= 0 && position < attachments.size) { return if (position >= 0 && position < attachments.size) {
ViewMediaFragment.newInstance(attachments[position], position == initialPosition) // Fragment should not wait for or start transition if it already happened but we
// instantiate the same fragment again, e.g. open the first photo, scroll to the
// forth photo and then back to the first. The first fragment will trz to start the
// transition and wait until it's over and it will never take place.
ViewMediaFragment.newInstance(
attachment = attachments[position],
shouldStartPostponedTransition = !didTransition && position == initialPosition
)
} else { } else {
throw IllegalStateException() throw IllegalStateException()
} }
@ -39,6 +47,7 @@ class ImagePagerAdapter(
} }
override fun onTransitionEnd() { override fun onTransitionEnd() {
this.didTransition = true
primaryItem?.onTransitionEnd() primaryItem?.onTransitionEnd()
} }
} }