Fix image zoom / pan / scroll / swipe (#3894)

Migrate to touchimageview from photoview, and adjust the touch logic to correctly handle single finger drag, two finger pinch/stretch, flings, taps, and swipes.

As before, the features are:

- Single tap, show/hide controls and media description
- Double tap, zoom in/out
- Single finger drag up/down, scale/translate image, dismiss if scrolled too far
- Single finger drag left/right
  - When not zoomed, swipe to next image if multiple images present
- When zoomed, scroll to edge of image, then to next image if multiple images present
- Two finger pinch/zoom, zoom in/out on the image

Behaviour differences to previous code

1. Bug fix: The image can't get "stuck" when zoomed, and impossible to scroll
2. Bug fix: Pinching is not mis-interpreted as a fling, closing the image
3. Bug fix: The zoom state of images is not lost or misinterpreted when the user swipes through multiple images
4. Bug fix: Double-tap zooms all the way, instead of stopping
5. Tapping outside the image does not dismiss it, controls and description show/hide

Fixes https://github.com/tuskyapp/Tusky/issues/3562, https://github.com/tuskyapp/Tusky/issues/2297

app/build.gradle

@@ -158,7 +158,7 @@ dependencies {
 
     implementation libs.sparkbutton
 
-    implementation libs.photoview
+    implementation libs.touchimageview
 
     implementation libs.bundles.material.drawer
     implementation libs.material.typeface
@@ -186,3 +186,20 @@ dependencies {
     androidTestImplementation libs.androidx.room.testing
     androidTestImplementation libs.androidx.test.junit
 }
+
+// Work around warnings of:
+// WARNING: Illegal reflective access by org.jetbrains.kotlin.kapt3.util.ModuleManipulationUtilsKt (file:/C:/Users/Andi/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-annotation-processing-gradle/1.8.22/28dab7e0ee9ce62c03bf97de3543c911dc653700/kotlin-annotation-processing-gradle-1.8.22.jar) to constructor com.sun.tools.javac.util.Context()
+// See https://youtrack.jetbrains.com/issue/KT-30589/Kapt-An-illegal-reflective-access-operation-has-occurred
+tasks.withType(org.jetbrains.kotlin.gradle.internal.KaptWithoutKotlincTask) {
+    kaptProcessJvmArgs.addAll([
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+        "--add-opens", "jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED"])
+}

app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt

@@ -51,7 +51,7 @@ class CaptionDialog : DialogFragment() {
 
         input = binding.imageDescriptionText
         val imageView = binding.imageDescriptionView
-        imageView.maximumScale = 6f
+        imageView.maxZoom = 6f
 
         input.hint = resources.getQuantityString(
             R.plurals.hint_describe_for_visually_impaired,

app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt

@@ -19,25 +19,31 @@ import android.animation.Animator
 import android.animation.AnimatorListenerAdapter
 import android.annotation.SuppressLint
 import android.content.Context
+import android.graphics.PointF
 import android.graphics.drawable.Drawable
 import android.os.Bundle
+import android.view.GestureDetector
 import android.view.LayoutInflater
 import android.view.MotionEvent
 import android.view.View
 import android.view.ViewGroup
 import android.widget.ImageView
 import androidx.core.os.BundleCompat
+import androidx.core.view.GestureDetectorCompat
 import com.bumptech.glide.Glide
 import com.bumptech.glide.load.DataSource
 import com.bumptech.glide.load.engine.GlideException
 import com.bumptech.glide.request.RequestListener
 import com.bumptech.glide.request.target.Target
-import com.github.chrisbanes.photoview.PhotoViewAttacher
+import com.keylesspalace.tusky.R
 import com.keylesspalace.tusky.ViewMediaActivity
 import com.keylesspalace.tusky.databinding.FragmentViewImageBinding
 import com.keylesspalace.tusky.entity.Attachment
 import com.keylesspalace.tusky.util.hide
+import com.keylesspalace.tusky.util.viewBinding
 import com.keylesspalace.tusky.util.visible
+import com.ortiz.touchview.OnTouchCoordinatesListener
+import com.ortiz.touchview.TouchImageView
 import io.reactivex.rxjava3.subjects.BehaviorSubject
 import kotlin.math.abs
 
@@ -48,10 +54,8 @@ class ViewImageFragment : ViewMediaFragment() {
         fun onPhotoTap()
     }
 
-    private var _binding: FragmentViewImageBinding? = null
+    private val binding by viewBinding(FragmentViewImageBinding::bind)
-    private val binding get() = _binding!!
 
-    private lateinit var attacher: PhotoViewAttacher
     private lateinit var photoActionsListener: PhotoActionsListener
     private lateinit var toolbar: View
     private var transition = BehaviorSubject.create<Unit>()
@@ -84,8 +88,7 @@ class ViewImageFragment : ViewMediaFragment() {
     override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
         toolbar = (requireActivity() as ViewMediaActivity).toolbar
         this.transition = BehaviorSubject.create()
-        _binding = FragmentViewImageBinding.inflate(inflater, container, false)
+        return inflater.inflate(R.layout.fragment_view_image, container, false)
-        return binding.root
     }
 
     @SuppressLint("ClickableViewAccessibility")
@@ -108,95 +111,139 @@ class ViewImageFragment : ViewMediaFragment() {
             }
         }
 
-        attacher = PhotoViewAttacher(binding.photoView).apply {
+        val singleTapDetector = GestureDetectorCompat(
-            // This prevents conflicts with ViewPager
+            requireContext(),
-            setAllowParentInterceptOnEdge(true)
+            object : GestureDetector.SimpleOnGestureListener() {
+                override fun onDown(e: MotionEvent) = true
+                override fun onSingleTapConfirmed(e: MotionEvent): Boolean {
+                    photoActionsListener.onPhotoTap()
+                    return false
+                }
+            }
+        )
 
-            // Clicking outside the photo closes the viewer.
+        binding.photoView.setOnTouchCoordinatesListener(object : OnTouchCoordinatesListener {
-            setOnOutsidePhotoTapListener { photoActionsListener.onDismiss() }
+            /** Y coordinate of the last single-finger drag */
-            setOnClickListener { onMediaTap() }
+            var lastDragY: Float? = null
 
-            /* A vertical swipe motion also closes the viewer. This is especially useful when the photo
+            override fun onTouchCoordinate(view: View, event: MotionEvent, bitmapPoint: PointF) {
-             * mostly fills the screen so clicking outside is difficult. */
+                singleTapDetector.onTouchEvent(event)
-            setOnSingleFlingListener { _, _, velocityX, velocityY ->
+
-                var result = false
+                // Two fingers have gone down after a single finger drag. Finish the drag
-                if (abs(velocityY) > abs(velocityX)) {
+                if (event.pointerCount == 2 && lastDragY != null) {
+                    onGestureEnd(view)
+                    lastDragY = null
+                }
+
+                // Stop the parent view from handling touches if either (a) the user has 2+
+                // fingers on the screen, or (b) the image has been zoomed in, and can be scrolled
+                // horizontally in both directions.
+                //
+                // This stops things like ViewPager2 from trying to intercept a left/right swipe
+                // and ensures that the image does not appear to "stick" to the screen as different
+                // views fight over who should be handling the swipe.
+                //
+                // If the view can be scrolled in one direction it's OK to let the parent intercept,
+                // which allows the user to swipe between images even if one or more of them have
+                // been zoomed in.
+                if (event.pointerCount >= 2 || view.canScrollHorizontally(1) && view.canScrollHorizontally(-1)) {
+                    when (event.action) {
+                        MotionEvent.ACTION_DOWN, MotionEvent.ACTION_MOVE -> {
+                            view.parent.requestDisallowInterceptTouchEvent(true)
+                        }
+
+                        MotionEvent.ACTION_UP -> {
+                            view.parent.requestDisallowInterceptTouchEvent(false)
+                        }
+                    }
+                    return
+                }
+
+                // The user is dragging the image around
+                if (event.pointerCount == 1) {
+                    // If the image is zoomed then the swipe-to-dismiss functionality is disabled
+                    if ((view as TouchImageView).isZoomed) return
+
+                    // The user's finger just went down, start recording where they are dragging from
+                    if (event.action == MotionEvent.ACTION_DOWN) {
+                        lastDragY = event.rawY
+                        return
+                    }
+
+                    // The user is dragging the un-zoomed image to possibly fling it up or down
+                    // to dismiss.
+                    if (event.action == MotionEvent.ACTION_MOVE) {
+                        // lastDragY may be null; e.g., the user was performing a two-finger drag,
+                        // and has lifted one finger. In this case do nothing
+                        lastDragY ?: return
+
+                        // Compute the Y offset of the drag, and scale/translate the photoview
+                        // accordingly.
+                        val diff = event.rawY - lastDragY!!
+                        if (view.translationY != 0f || abs(diff) > 40) {
+                            // Drag has definitely started, stop the parent from interfering
+                            view.parent.requestDisallowInterceptTouchEvent(true)
+                            view.translationY += diff
+                            val scale = (-abs(view.translationY) / 720 + 1).coerceAtLeast(0.5f)
+                            view.scaleY = scale
+                            view.scaleX = scale
+                            lastDragY = event.rawY
+                        }
+                        return
+                    }
+
+                    // The user has finished dragging. Allow the parent to handle touch events if
+                    // appropriate, and end the gesture.
+                    if (event.action == MotionEvent.ACTION_UP || event.action == MotionEvent.ACTION_CANCEL) {
+                        view.parent.requestDisallowInterceptTouchEvent(false)
+                        if (lastDragY != null) onGestureEnd(view)
+                        lastDragY = null
+                        return
+                    }
+                }
+            }
+
+            /**
+             * Handle the end of the user's gesture.
+             *
+             * If the user was previously dragging, and the image has been dragged a sufficient
+             * distance then we are done. Otherwise, animate the image back to its starting position.
+             */
+            private fun onGestureEnd(view: View) {
+                if (abs(view.translationY) > 180) {
                     photoActionsListener.onDismiss()
-                    result = true
+                } else {
+                    view.animate().translationY(0f).scaleX(1f).start()
                 }
-                result
             }
-        }
+        })
-
-        var lastY = 0f
-
-        binding.photoView.setOnTouchListener { v, event ->
-            // This part is for scaling/translating on vertical move.
-            // We use raw coordinates to get the correct ones during scaling
-
-            if (event.action == MotionEvent.ACTION_DOWN) {
-                lastY = event.rawY
-            } else if (event.pointerCount == 1 &&
-                attacher.scale == 1f &&
-                event.action == MotionEvent.ACTION_MOVE
-            ) {
-                val diff = event.rawY - lastY
-                // This code is to prevent transformations during page scrolling
-                // If we are already translating or we reached the threshold, then transform.
-                if (binding.photoView.translationY != 0f || abs(diff) > 40) {
-                    binding.photoView.translationY += (diff)
-                    val scale = (-abs(binding.photoView.translationY) / 720 + 1).coerceAtLeast(0.5f)
-                    binding.photoView.scaleY = scale
-                    binding.photoView.scaleX = scale
-                    lastY = event.rawY
-                    return@setOnTouchListener true
-                }
-            } else if (event.action == MotionEvent.ACTION_UP || event.action == MotionEvent.ACTION_CANCEL) {
-                onGestureEnd()
-            }
-            attacher.onTouch(v, event)
-        }
 
         finalizeViewSetup(url, attachment?.previewUrl, description)
     }
 
-    private fun onGestureEnd() {
-        if (_binding == null) {
-            return
-        }
-        if (abs(binding.photoView.translationY) > 180) {
-            photoActionsListener.onDismiss()
-        } else {
-            binding.photoView.animate().translationY(0f).scaleX(1f).scaleY(1f).start()
-        }
-    }
-
-    private fun onMediaTap() {
-        photoActionsListener.onPhotoTap()
-    }
-
     override fun onToolbarVisibilityChange(visible: Boolean) {
-        if (_binding == null || !userVisibleHint) {
+        if (!userVisibleHint) return
-            return
+
-        }
         isDescriptionVisible = showingDescription && visible
         val alpha = if (isDescriptionVisible) 1.0f else 0.0f
         binding.captionSheet.animate().alpha(alpha)
             .setListener(object : AnimatorListenerAdapter() {
                 override fun onAnimationEnd(animation: Animator) {
-                    if (_binding != null) {
+                    view ?: return
-                        binding.captionSheet.visible(isDescriptionVisible)
+                    binding.captionSheet.visible(isDescriptionVisible)
-                    }
                     animation.removeListener(this)
                 }
             })
             .start()
     }
 
-    override fun onDestroyView() {
+    override fun onStop() {
+        super.onStop()
         Glide.with(this).clear(binding.photoView)
+    }
+
+    override fun onDestroyView() {
         transition.onComplete()
-        _binding = null
         super.onDestroyView()
     }
 
@@ -270,7 +317,7 @@ class ViewImageFragment : ViewMediaFragment() {
                 photoActionsListener.onBringUp()
             }
             // Hide progress bar only on fail request from internet
-            if (!isCacheRequest && _binding != null) binding.progressBar.hide()
+            if (!isCacheRequest) binding.progressBar.hide()
             // We don't want to overwrite preview with null when main image fails to load
             return !isCacheRequest
         }
@@ -283,9 +330,7 @@ class ViewImageFragment : ViewMediaFragment() {
             dataSource: DataSource,
             isFirstResource: Boolean
         ): Boolean {
-            if (_binding != null) {
+            binding.progressBar.hide() // Always hide the progress bar on success
-                binding.progressBar.hide() // Always hide the progress bar on success
-            }
 
             if (!startedTransition || !shouldStartTransition) {
                 // Set this right away so that we don't have to concurrent post() requests
@@ -303,10 +348,6 @@ class ViewImageFragment : ViewMediaFragment() {
                     .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

app/src/main/res/layout/dialog_image_description.xml

@@ -6,7 +6,7 @@
     android:orientation="vertical"
     android:paddingBottom="0dp">
 
-    <com.github.chrisbanes.photoview.PhotoView
+    <com.ortiz.touchview.TouchImageView
         android:id="@+id/imageDescriptionView"
         android:layout_width="match_parent"
         android:layout_height="0dp"

app/src/main/res/layout/fragment_view_image.xml

@@ -7,7 +7,7 @@
     android:clickable="true"
     android:focusable="true">
 
-    <com.github.chrisbanes.photoview.PhotoView
+    <com.ortiz.touchview.TouchImageView
         android:id="@+id/photoView"
         android:layout_width="match_parent"
         android:layout_height="match_parent" />
@@ -76,4 +76,4 @@
             </androidx.core.widget.NestedScrollView>
         </LinearLayout>
     </androidx.coordinatorlayout.widget.CoordinatorLayout>
-</androidx.constraintlayout.widget.ConstraintLayout>
+</androidx.constraintlayout.widget.ConstraintLayout>

gradle/libs.versions.toml

@@ -47,8 +47,8 @@ robolectric = "4.10.3"
 rxandroid3 = "3.0.2"
 rxjava3 = "3.1.6"
 rxkotlin3 = "3.0.1"
-photoview = "2.3.0"
 sparkbutton = "4.1.0"
+touchimageview = "3.4"
 truth = "1.1.5"
 turbine = "1.0.0"
 unified-push = "2.1.1"
@@ -127,7 +127,6 @@ mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "
 networkresult-calladapter = { module = "at.connyduck:networkresult-calladapter", version.ref = "networkresult-calladapter" }
 okhttp-core = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp" }
 okhttp-logging-interceptor = { module = "com.squareup.okhttp3:logging-interceptor", version.ref = "okhttp" }
-photoview = { module = "com.github.chrisbanes:PhotoView", version.ref = "photoview" }
 retrofit-adapter-rxjava3 = { module = "com.squareup.retrofit2:adapter-rxjava3", version.ref = "retrofit" }
 retrofit-converter-gson = { module = "com.squareup.retrofit2:converter-gson", version.ref = "retrofit" }
 retrofit-core = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" }
@@ -136,6 +135,7 @@ rxjava3-android = { module = "io.reactivex.rxjava3:rxandroid", version.ref = "rx
 rxjava3-core = { module = "io.reactivex.rxjava3:rxjava", version.ref = "rxjava3" }
 rxjava3-kotlin = { module = "io.reactivex.rxjava3:rxkotlin", version.ref = "rxkotlin3" }
 sparkbutton = { module = "com.github.connyduck:sparkbutton", version.ref = "sparkbutton" }
+touchimageview = { module = "com.github.MikeOrtiz:TouchImageView", version.ref = "touchimageview" }
 truth = { module = "com.google.truth:truth", version.ref = "truth" }
 turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine" }
 unified-push = { module = "com.github.UnifiedPush:android-connector", version.ref = "unified-push" }