From 7e7160b9f08d0f17fcfc711627f01a24af53be34 Mon Sep 17 00:00:00 2001 From: Saket Narayan Date: Wed, 13 Nov 2024 11:30:13 -0500 Subject: [PATCH] Retain the same final zoom level across viewport changes --- .../telephoto/zoomable/ZoomableImageTest.kt | 23 ++++++++++++++++++- ...ll,Portrait]_[after_another_rotation].png} | 0 ...change[Fill,Portrait]_[after_rotation].png | 3 +++ ...ange[Fill,Portrait]_[before_rotation].png} | 0 ...entation_change[Fill]_[after_rotation].png | 3 --- ...it,Landscape]_[after_another_rotation].png | 3 +++ ...change[Fit,Landscape]_[after_rotation].png | 3 +++ ...hange[Fit,Landscape]_[before_rotation].png | 3 +++ ...it,Portrait]_[after_another_rotation].png} | 0 ..._change[Fit,Portrait]_[after_rotation].png | 3 +++ ...hange[Fit,Portrait]_[before_rotation].png} | 0 ...ientation_change[Fit]_[after_rotation].png | 3 --- .../telephoto/zoomable/RealZoomableState.kt | 9 +++++++- .../zoomable/internal/GestureStateAdjuster.kt | 16 ++++--------- .../telephoto/zoomable/internal/savedState.kt | 14 +++++------ 15 files changed, 57 insertions(+), 26 deletions(-) rename zoomable-image/core/src/androidTest/screenshots/{non_empty_transformations_are_retained_across_orientation_change[Fill]_[after_another_rotation].png => non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[after_another_rotation].png} (100%) create mode 100644 zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[after_rotation].png rename zoomable-image/core/src/androidTest/screenshots/{non_empty_transformations_are_retained_across_orientation_change[Fill]_[before_rotation].png => non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[before_rotation].png} (100%) delete mode 100644 zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[after_rotation].png create mode 100644 zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_another_rotation].png create mode 100644 zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_rotation].png create mode 100644 zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[before_rotation].png rename zoomable-image/core/src/androidTest/screenshots/{non_empty_transformations_are_retained_across_orientation_change[Fit]_[after_another_rotation].png => non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[after_another_rotation].png} (100%) create mode 100644 zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[after_rotation].png rename zoomable-image/core/src/androidTest/screenshots/{non_empty_transformations_are_retained_across_orientation_change[Fit]_[before_rotation].png => non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[before_rotation].png} (100%) delete mode 100644 zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[after_rotation].png diff --git a/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt b/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt index 401bc77c..d27bb568 100644 --- a/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt +++ b/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt @@ -114,6 +114,7 @@ import me.saket.telephoto.zoomable.ZoomableImageTest.ScrollDirection import me.saket.telephoto.zoomable.ZoomableImageTest.ScrollDirection.LeftToRight import me.saket.telephoto.zoomable.ZoomableImageTest.ScrollDirection.RightToLeft import org.junit.After +import org.junit.AssumptionViolatedException import org.junit.Before import org.junit.Rule import org.junit.Test @@ -1496,7 +1497,15 @@ class ZoomableImageTest { // todo: should probably move these tests to ZoomableTest. @Test fun non_empty_transformations_are_retained_across_orientation_change( @TestParameter contentScaleParam: ContentScaleParamWithDifferentProportions, + @TestParameter imageOrientationParam: ImageOrientationParam, ) { + if ( + imageOrientationParam == ImageOrientationParam.Landscape + && contentScaleParam != ContentScaleParamWithDifferentProportions.Fit + ) { + throw AssumptionViolatedException("not needed") + } + lateinit var imageState: ZoomableImageState val recreationTester = ActivityRecreationTester(rule) @@ -1509,7 +1518,7 @@ class ZoomableImageTest { .fillMaxSize() .border(1.dp, Color.Yellow) .testTag("image"), - image = ZoomableImageSource.asset("cat_1920.jpg", subSample = true), + image = ZoomableImageSource.asset(imageOrientationParam.assetName, subSample = true), state = imageState, contentDescription = null, contentScale = contentScaleParam.value, @@ -1520,8 +1529,12 @@ class ZoomableImageTest { (rule.onNodeWithTag("image")).run { performTouchInput { doubleClick(center - Offset(0f, 360f)) } } + rule.waitForIdle() rule.waitUntil { imageState.isImageDisplayedInFullQuality } + + val zoomFractionBeforeRotation = imageState.zoomableState.zoomFraction rule.runOnIdle { + assertThat(zoomFractionBeforeRotation).isEqualTo(1f) dropshots.assertSnapshot(rule.activity, testName.methodName + "_[before_rotation]") } @@ -1540,6 +1553,7 @@ class ZoomableImageTest { rule.waitUntil { imageState.isImageDisplayedInFullQuality } rule.runOnIdle { + assertThat(imageState.zoomableState.zoomFraction).isEqualTo(zoomFractionBeforeRotation) dropshots.assertSnapshot(rule.activity, testName.methodName + "_[after_another_rotation]") } } @@ -1577,6 +1591,7 @@ class ZoomableImageTest { rule.waitUntil { imageState.isImageDisplayedInFullQuality } rule.runOnIdle { + assertThat(imageState.zoomableState.zoomFraction).isEqualTo(0f) dropshots.assertSnapshot(rule.activity, testName.methodName + "_[after_rotation]") } } @@ -1725,6 +1740,12 @@ class ZoomableImageTest { Fill(ContentScale.FillBounds), // Scaling is disproportionate } + @Suppress("unused") + enum class ImageOrientationParam(val assetName: String) { + Portrait("cat_1920.jpg"), + Landscape("fox_1500.jpg") + } + @Suppress("unused") enum class ImageAssetParam(val assetName: String) { SmallerThanLayoutSize("fox_250.jpg"), diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[after_another_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[after_another_rotation].png similarity index 100% rename from zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[after_another_rotation].png rename to zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[after_another_rotation].png diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[after_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[after_rotation].png new file mode 100644 index 00000000..e07c85ba --- /dev/null +++ b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[after_rotation].png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5d0a17d9c3c5fa9dcd0d54c72918b4fe72f5a0b9f17da12155550f59641311f4 +size 2595726 diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[before_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[before_rotation].png similarity index 100% rename from zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[before_rotation].png rename to zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill,Portrait]_[before_rotation].png diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[after_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[after_rotation].png deleted file mode 100644 index fef2b09e..00000000 --- a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fill]_[after_rotation].png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:ea33c9e25619a9756ae24ca64f2bb28f06b9eada1897de4be0dd83c93ee81da4 -size 2432882 diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_another_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_another_rotation].png new file mode 100644 index 00000000..e9b6dffd --- /dev/null +++ b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_another_rotation].png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:426bc9e540ed60ae02cdc77dda69a3201a840952af8bd9de01ec25599f6a1369 +size 2037769 diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_rotation].png new file mode 100644 index 00000000..c77bc454 --- /dev/null +++ b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[after_rotation].png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4e85e55b20e727e5ddce8bfc845b23a2437abe8099cba541eef6e799a396fb17 +size 1904153 diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[before_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[before_rotation].png new file mode 100644 index 00000000..e9b6dffd --- /dev/null +++ b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Landscape]_[before_rotation].png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:426bc9e540ed60ae02cdc77dda69a3201a840952af8bd9de01ec25599f6a1369 +size 2037769 diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[after_another_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[after_another_rotation].png similarity index 100% rename from zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[after_another_rotation].png rename to zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[after_another_rotation].png diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[after_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[after_rotation].png new file mode 100644 index 00000000..3763dda0 --- /dev/null +++ b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[after_rotation].png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:58f0f1dbd6ef357514b34828356bfbcaf79e9f7fa76236c48c78ffaa4c6d9da7 +size 2790138 diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[before_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[before_rotation].png similarity index 100% rename from zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[before_rotation].png rename to zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit,Portrait]_[before_rotation].png diff --git a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[after_rotation].png b/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[after_rotation].png deleted file mode 100644 index 4b18709d..00000000 --- a/zoomable-image/core/src/androidTest/screenshots/non_empty_transformations_are_retained_across_orientation_change[Fit]_[after_rotation].png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:bf21ecf42e5a9a3cc8b9304b0f4519772405509899eb1a95bec94fc492273a1b -size 2744060 diff --git a/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/RealZoomableState.kt b/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/RealZoomableState.kt index 22a71f0f..5f58e4ff 100644 --- a/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/RealZoomableState.kt +++ b/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/RealZoomableState.kt @@ -131,7 +131,7 @@ internal class RealZoomableState internal constructor( GestureStateCalculator { inputs -> savedState?.asGestureState( inputs = inputs, - coerceWithinBounds = { contentOffset, contentZoom -> + coerceOffsetWithinBounds = { contentOffset, contentZoom -> contentOffset.coerceWithinContentBounds(contentZoom, inputs) } ) @@ -726,6 +726,13 @@ internal data class ContentZoomFactor( userZoom = UserZoomFactor(finalZoom / baseZoom.value.maxScale), ) } + + fun forFinalZoom(baseZoom: BaseZoomFactor, finalZoom: ScaleFactor): ContentZoomFactor { + return ContentZoomFactor( + baseZoom = baseZoom, + userZoom = UserZoomFactor(finalZoom.maxScale / baseZoom.value.maxScale), + ) + } } } diff --git a/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/GestureStateAdjuster.kt b/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/GestureStateAdjuster.kt index 55800f8d..a227048a 100644 --- a/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/GestureStateAdjuster.kt +++ b/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/GestureStateAdjuster.kt @@ -4,11 +4,11 @@ import androidx.compose.ui.geometry.Offset import androidx.compose.ui.geometry.Rect import androidx.compose.ui.geometry.Size import androidx.compose.ui.geometry.center +import androidx.compose.ui.layout.ScaleFactor import me.saket.telephoto.zoomable.ContentOffset import me.saket.telephoto.zoomable.ContentZoomFactor import me.saket.telephoto.zoomable.GestureState import me.saket.telephoto.zoomable.GestureStateInputs -import me.saket.telephoto.zoomable.UserZoomFactor import me.saket.telephoto.zoomable.ZoomableContentTransformation import me.saket.telephoto.zoomable.ZoomableState @@ -17,7 +17,7 @@ import me.saket.telephoto.zoomable.ZoomableState * Adjusts zoom and pan values to maintain the content's centroid position in the new viewport. */ internal class GestureStateAdjuster( - private val oldUserZoom: UserZoomFactor, + private val oldFinalZoom: ScaleFactor, private val oldContentOffsetAtViewportCenter: Offset, // Present in the content's coordinate space. ) { @@ -25,14 +25,9 @@ internal class GestureStateAdjuster( inputs: GestureStateInputs, coerceWithinBounds: (ContentOffset, ContentZoomFactor) -> ContentOffset, ): GestureState { - // A new base zoom is generated based on the viewport size. Only the user's zoom - // value should be retained, not the final zoom. This prevents content from appearing - // visually different across screen sizes - for example, a final zoom of 2f would - // look smaller on a 2400p screen compared to a 1080p screen if the final zoom were kept. - val newZoom = ContentZoomFactor( - baseZoom = inputs.baseZoom, - userZoom = oldUserZoom, - ) + // Retain the same zoom level. This will change the user zoom level, but that's okay. + // Switching from a smaller to a larger screen should display more content, not the same. + val newZoom = ContentZoomFactor.forFinalZoom(inputs.baseZoom, finalZoom = oldFinalZoom) // Find the offset needed to move the old anchor (i.e., the content offset at the viewport // center) back to the viewport's center. The anchor is present in the content's coordinate @@ -40,7 +35,6 @@ internal class GestureStateAdjuster( val newUserOffset = oldContentOffsetAtViewportCenter.withZoom(newZoom.finalZoom()) { anchorInViewportSpace -> anchorInViewportSpace - inputs.viewportSize.center } - val proposedContentOffset = ContentOffset.forFinalOffset( baseOffset = inputs.baseOffset, finalOffset = newUserOffset, diff --git a/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/savedState.kt b/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/savedState.kt index 1ff72be0..7967c844 100644 --- a/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/savedState.kt +++ b/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/internal/savedState.kt @@ -20,7 +20,7 @@ internal data class ZoomableSavedState private constructor( private val userOffset: Long, private val userZoom: Float, private val centroid: Long, - private val stateRestorerInfo: StateRestorerInfo?, + private val stateAdjusterInfo: StateRestorerInfo?, ) : AndroidParcelable { @AndroidParcelize @@ -38,7 +38,7 @@ internal data class ZoomableSavedState private constructor( userOffset = gestureState.userOffset.value.packToLong(), userZoom = gestureState.userZoom.value, centroid = gestureState.lastCentroid.packToLong(), - stateRestorerInfo = gestureStateInputs.viewportSize + stateAdjusterInfo = gestureStateInputs.viewportSize .takeIf { it.isSpecifiedAndNonEmpty } ?.let { viewportSize -> StateRestorerInfo( @@ -59,13 +59,13 @@ internal data class ZoomableSavedState private constructor( fun asGestureState( inputs: GestureStateInputs, - coerceWithinBounds: (ContentOffset, ContentZoomFactor) -> ContentOffset, + coerceOffsetWithinBounds: (ContentOffset, ContentZoomFactor) -> ContentOffset, ): GestureState { val restoredUserOffset = userOffset.unpackAsOffset() val wasGestureStateEmpty = restoredUserOffset == Offset.Zero && userZoom == 1f if ( wasGestureStateEmpty - || (stateRestorerInfo == null || stateRestorerInfo.viewportSize.unpackAsSize() == inputs.viewportSize) + || (stateAdjusterInfo == null || stateAdjusterInfo.viewportSize.unpackAsSize() == inputs.viewportSize) ) { return GestureState( userOffset = UserOffset(restoredUserOffset), @@ -79,12 +79,12 @@ internal data class ZoomableSavedState private constructor( // Treat the content offset at the viewport's center as the anchor and adjust the gesture state // to maintain the anchor's position in the new viewport. val stateAdjuster = GestureStateAdjuster( - oldUserZoom = UserZoomFactor(userZoom), - oldContentOffsetAtViewportCenter = stateRestorerInfo.contentOffsetAtViewportCenter.unpackAsOffset(), + oldFinalZoom = stateAdjusterInfo.finalZoomFactor.unpackAsScaleFactor(), + oldContentOffsetAtViewportCenter = stateAdjusterInfo.contentOffsetAtViewportCenter.unpackAsOffset(), ) return stateAdjuster.adjustForNewViewportSize( inputs = inputs, - coerceWithinBounds = coerceWithinBounds, + coerceWithinBounds = coerceOffsetWithinBounds, ) } }