From 79ee2dc32ccf3a03a1649ab0423e529c9f61e2aa Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Mon, 31 Jul 2023 12:44:01 +0200 Subject: [PATCH] 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 | 19 +- .../compose/dialog/CaptionDialog.kt | 2 +- .../tusky/fragment/ViewImageFragment.kt | 201 +++++++++++------- .../res/layout/dialog_image_description.xml | 2 +- .../main/res/layout/fragment_view_image.xml | 4 +- gradle/libs.versions.toml | 4 +- 6 files changed, 145 insertions(+), 87 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index b7f60b3a8..1e67b551b 100644 --- a/app/build.gradle +++ b/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"]) +} diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt index a1dc032cd..c066b3d9f 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt +++ b/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, diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt index 24c8feb03..bbbfdf219 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt +++ b/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 get() = _binding!! + private val binding by viewBinding(FragmentViewImageBinding::bind) - private lateinit var attacher: PhotoViewAttacher private lateinit var photoActionsListener: PhotoActionsListener private lateinit var toolbar: View private var transition = BehaviorSubject.create() @@ -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 binding.root + return inflater.inflate(R.layout.fragment_view_image, container, false) } @SuppressLint("ClickableViewAccessibility") @@ -108,95 +111,139 @@ class ViewImageFragment : ViewMediaFragment() { } } - attacher = PhotoViewAttacher(binding.photoView).apply { - // This prevents conflicts with ViewPager - setAllowParentInterceptOnEdge(true) + val singleTapDetector = GestureDetectorCompat( + requireContext(), + 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. - setOnOutsidePhotoTapListener { photoActionsListener.onDismiss() } - setOnClickListener { onMediaTap() } + binding.photoView.setOnTouchCoordinatesListener(object : OnTouchCoordinatesListener { + /** Y coordinate of the last single-finger drag */ + var lastDragY: Float? = null - /* A vertical swipe motion also closes the viewer. This is especially useful when the photo - * mostly fills the screen so clicking outside is difficult. */ - setOnSingleFlingListener { _, _, velocityX, velocityY -> - var result = false - if (abs(velocityY) > abs(velocityX)) { + override fun onTouchCoordinate(view: View, event: MotionEvent, bitmapPoint: PointF) { + singleTapDetector.onTouchEvent(event) + + // Two fingers have gone down after a single finger drag. Finish the drag + 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) { - return - } + if (!userVisibleHint) 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) { - binding.captionSheet.visible(isDescriptionVisible) - } + view ?: return + 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 diff --git a/app/src/main/res/layout/dialog_image_description.xml b/app/src/main/res/layout/dialog_image_description.xml index 4d749a8f5..d20d8795f 100644 --- a/app/src/main/res/layout/dialog_image_description.xml +++ b/app/src/main/res/layout/dialog_image_description.xml @@ -6,7 +6,7 @@ android:orientation="vertical" android:paddingBottom="0dp"> - - @@ -76,4 +76,4 @@ - \ No newline at end of file + diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 1d855edfa..1ef4a5ceb 100644 --- a/gradle/libs.versions.toml +++ b/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" }