Retain the same final zoom level across viewport changes

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"),

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),
+      )
+    }
   }
 }
 

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,30 +17,24 @@ 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.
 ) {
 
   fun adjustForNewViewportSize(
     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
     // space so it will be be transformed to the viewport space for the scope of this calculation.
     val newUserOffset = oldContentOffsetAtViewportCenter.withZoom(newZoom.finalZoom()) { anchorInViewportSpace ->
       anchorInViewportSpace - inputs.viewportSize.center
     }
-
     val proposedContentOffset = ContentOffset.forFinalOffset(
       baseOffset = inputs.baseOffset,
       finalOffset = newUserOffset,

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,
     )
   }
 }