Fix the pinch's jumpy hour gutter and its zoom-out floor (#56)

Both found reviewing the gesture on device.

The gutter is 24 stacked hour-tall boxes, each rounding its own height to
whole pixels, while the hour lines and event blocks are drawn at the
fractional height. A pinch lands on fractional heights constantly, so the
two sat on different grids — at 56.4px per hour the 23:00 label is ~9px
off the line it names — and the whole label column jumped as the pinch
drifted across each half pixel. A pinch now lands on whole pixels, so
every part of the timeline shares one grid.

Zooming out stopped at a fixed 24dp per hour, which is below the height
that fills the screen on a phone, so you could shrink the day until it
left dead space under midnight. The floor is now the height at which all
24 hours exactly fill the viewport: past that there is no more day to
uncover. It is applied when the height is resolved rather than only in
the gesture, so a height pinched in landscape still fills portrait.

app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt

@@ -42,9 +42,13 @@ sealed interface TimelineScale {
         /** The named steps the settings picker offers, coarse to roomy. */
         val presets: List<TimelineScale> = listOf(FitDay, Compact, Regular, Comfortable)
 
-        /** A pinched hour height, clamped into the legible zoom range. */
+        /**
+         * A pinched hour height. The floor a pinch actually stops at depends on
+         * the viewport and is applied when the height is resolved (see
+         * [hourHeight]); this only holds a stored value to something sane.
+         */
         fun custom(hourHeight: Dp): Custom =
-            Custom(hourHeight.coerceIn(MIN_PINCH_HOUR_HEIGHT, MAX_PINCH_HOUR_HEIGHT))
+            Custom(hourHeight.coerceIn(MIN_STORED_HOUR_HEIGHT, MAX_PINCH_HOUR_HEIGHT))
     }
 }
 
@@ -56,6 +60,11 @@ sealed interface TimelineScale {
  * being legible, and above [FIT_DAY_MAX] a short landscape day would stretch its
  * blocks absurdly. On a screen too short for the whole day the clamp wins and
  * the timeline still scrolls a little — honest, rather than unreadable.
+ *
+ * A pinched height is held to [fillHourHeight] here and not only in the gesture,
+ * because the viewport can change under a height that was already stored: pinch
+ * all the way out in landscape and the same value would leave portrait with dead
+ * space under midnight.
  */
 fun TimelineScale.hourHeight(viewportHeight: Dp): Dp = when (this) {
     TimelineScale.FitDay -> (viewportHeight / 24f).coerceIn(FIT_DAY_MIN, FIT_DAY_MAX)
@@ -63,6 +72,8 @@ fun TimelineScale.hourHeight(viewportHeight: Dp): Dp = when (this) {
     TimelineScale.Regular -> 56.dp
     TimelineScale.Comfortable -> 80.dp
     is TimelineScale.Custom -> this.hourHeight
+        .coerceAtMost(MAX_PINCH_HOUR_HEIGHT)
+        .coerceAtLeast(fillHourHeight(viewportHeight))
 }
 
 /**
@@ -91,10 +102,14 @@ val FIT_DAY_MIN = 24.dp
 val FIT_DAY_MAX = 96.dp
 
 /**
- * Floor for a pinch — the same legibility limit [TimelineScale.FitDay] stops at,
+ * The hour height at which all 24 hours exactly fill [viewportHeight] — how far
- * since the gutter has the same 24 labels to fit either way.
+ * out a pinch can zoom.
+ *
+ * Below it there is no more day left to uncover, so the only thing shrinking
+ * further buys is empty space under midnight. Zooming out means "show me more of
+ * the day", and once the whole day is on screen that request is answered.
  */
-val MIN_PINCH_HOUR_HEIGHT = FIT_DAY_MIN
+fun fillHourHeight(viewportHeight: Dp): Dp = viewportHeight / 24f
 
 /**
  * Ceiling for a pinch, deliberately far above [TimelineScale.Comfortable]: a
@@ -103,6 +118,13 @@ val MIN_PINCH_HOUR_HEIGHT = FIT_DAY_MIN
  */
 val MAX_PINCH_HOUR_HEIGHT = 240.dp
 
+/**
+ * Floor for a *stored* pinched height, which only has to stay sane enough to be
+ * re-clamped against whatever viewport it is later shown in. The floor a pinch
+ * stops at is [fillHourHeight].
+ */
+private val MIN_STORED_HOUR_HEIGHT = 1.dp
+
 @get:StringRes
 val TimelineScale.labelRes: Int
     get() = when (this) {

app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt

@@ -19,6 +19,7 @@ import androidx.compose.ui.input.pointer.PointerEventPass
 import androidx.compose.ui.input.pointer.pointerInput
 import androidx.compose.ui.unit.Dp
 import kotlin.math.abs
+import kotlin.math.roundToInt
 
 /**
  * The scale the timelines draw at right now: the stored preference, or whatever
@@ -104,16 +105,19 @@ fun rememberTimelineZoom(
  *
  * [hourHeight] is the *resolved* height, so a pinch that starts from
  * `FitDay` picks up where the viewport left it rather than jumping.
+ * [viewportHeight] sets how far out it can go — see [fillHourHeight].
  */
 @Composable
 fun rememberTimelinePinchZoom(
     scrollState: ScrollState,
+    viewportHeight: Dp,
     hourHeight: Dp,
     zoom: TimelineZoom,
 ): Modifier {
-    // The gesture loop outlives any single composition, so it reads the height
+    // The gesture loop outlives any single composition, so it reads these
-    // through a state handle rather than capturing the value it started with.
+    // through state handles rather than capturing what they were when it started.
     val currentHourHeight = rememberUpdatedState(hourHeight)
+    val currentViewport = rememberUpdatedState(viewportHeight)
     return Modifier.pointerInput(scrollState, zoom) {
         awaitEachGesture {
             awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Initial)
@@ -136,8 +140,11 @@ fun rememberTimelinePinchZoom(
                     }
                     if (claimed) {
                         val old = currentHourHeight.value.toPx()
-                        val new = (old * step)
+                        val new = pinchedHourHeightPx(
-                            .coerceIn(MIN_PINCH_HOUR_HEIGHT.toPx(), MAX_PINCH_HOUR_HEIGHT.toPx())
+                            target = old * step,
+                            fillPx = fillHourHeight(currentViewport.value).toPx(),
+                            maxPx = MAX_PINCH_HOUR_HEIGHT.toPx(),
+                        )
                         if (new != old) {
                             val centroidY = event.calculateCentroid(useCurrent = true).y
                             pending += anchoredScroll(scrollState.value, centroidY, old, new) -
@@ -157,6 +164,23 @@ fun rememberTimelinePinchZoom(
     }
 }
 
+/**
+ * Where a pinch aiming at [target] px per hour actually lands: clamped between
+ * "the whole day fills the screen" ([fillPx]) and [maxPx], then rounded to a
+ * whole pixel.
+ *
+ * The rounding is not cosmetic. The hour gutter is 24 stacked boxes one hour
+ * tall, and each rounds its own height to whole pixels, while the hour lines and
+ * event blocks are drawn at the fractional height — so a height of 56.4px lays
+ * the labels out at 56px and leaves the 23:00 label ~9px above the line it
+ * names, jumping the whole column as a pinch drifts across each half pixel.
+ * Pinning the hour to whole pixels keeps every part of the timeline on one grid.
+ */
+internal fun pinchedHourHeightPx(target: Float, fillPx: Float, maxPx: Float): Float =
+    // Filling the viewport wins over the ceiling: on a screen tall enough for
+    // the two to disagree, dead space is the worse of the two failures.
+    target.coerceAtMost(maxPx).coerceAtLeast(fillPx).roundToInt().toFloat()
+
 /**
  * The scroll offset that keeps the moment under [centroidY] under it after the
  * hour height changes from [oldHourPx] to [newHourPx].

app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt

@@ -499,7 +499,7 @@ private fun Timeline(
         // The pinch sits on the Row, above both scroll viewports: it has to
         // outrank the vertical scroll, and it does that by watching the initial
         // pass, which only reaches it if it is their ancestor.
-        val pinch = rememberTimelinePinchZoom(scrollState, hourHeight, zoom)
+        val pinch = rememberTimelinePinchZoom(scrollState, maxHeight, hourHeight, zoom)
         // Gutter and day column are two scroll viewports that SHARE one scroll
         // state, so they stay perfectly aligned. The day-column viewport is a
         // static, rounded-clipped window — the content scrolls inside it, so the

app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt

@@ -634,7 +634,7 @@ private fun Timeline(
         // The pinch sits on the Row, above both scroll viewports: it has to
         // outrank the vertical scroll, and it does that by watching the initial
         // pass, which only reaches it if it is their ancestor.
-        val pinch = rememberTimelinePinchZoom(scrollState, hourHeight, zoom)
+        val pinch = rememberTimelinePinchZoom(scrollState, maxHeight, hourHeight, zoom)
         // Gutter and day columns are two scroll viewports that SHARE one scroll
         // state, so they stay perfectly aligned. The day-column viewport is a
         // static, rounded-clipped window — the content scrolls inside it, so the

app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineScaleTest.kt

@@ -73,27 +73,34 @@ class TimelineScaleTest {
     }
 
     @Test
-    fun `a pinched height is used as given, whatever the viewport`() {
+    fun `a pinched height is used as given`() {
-        val scale = TimelineScale.custom(63.dp)
+        assertThat(TimelineScale.custom(63.dp).hourHeight(phoneViewport)).isEqualTo(63.dp)
-        assertThat(scale.hourHeight(phoneViewport)).isEqualTo(63.dp)
-        assertThat(scale.hourHeight(120.dp)).isEqualTo(63.dp)
     }
 
     @Test
-    fun `a pinch cannot zoom past the legible range`() {
+    fun `a pinched height never leaves the timeline short of the screen`() {
-        // The gesture clamps as it goes, but the floor and ceiling belong to the
+        // Zooming out means "show me more of the day"; once the whole day is on
-        // model — nothing that constructs a scale may land outside them (#56).
+        // screen that is answered, and going further would only open dead space
-        assertThat(TimelineScale.custom(1.dp).hourHeight).isEqualTo(MIN_PINCH_HOUR_HEIGHT)
+        // under midnight.
-        assertThat(TimelineScale.custom(10_000.dp).hourHeight).isEqualTo(MAX_PINCH_HOUR_HEIGHT)
+        val tooSmall = TimelineScale.custom(4.dp)
+        assertThat(tooSmall.hourHeight(phoneViewport) * 24).isAtLeast(phoneViewport)
     }
 
     @Test
-    fun `a pinch can go beyond every preset in both directions`() {
+    fun `a height pinched on one viewport still fills a taller one`() {
+        // Pinching all the way out in landscape stores a small height; rotating
+        // back to portrait must not leave the day floating in the top half.
+        val pinchedInLandscape = TimelineScale.custom(fillHourHeight(320.dp))
+        assertThat(pinchedInLandscape.hourHeight(phoneViewport) * 24).isAtLeast(phoneViewport)
+    }
+
+    @Test
+    fun `a pinch can zoom in past every preset`() {
         // Otherwise the gesture would be strictly less capable than the picker
         // it is meant to refine.
         val presetHeights = TimelineScale.presets.map { it.hourHeight(phoneViewport) }
-        assertThat(MIN_PINCH_HOUR_HEIGHT).isAtMost(presetHeights.min())
         assertThat(MAX_PINCH_HOUR_HEIGHT).isGreaterThan(presetHeights.max())
+        assertThat(TimelineScale.custom(200.dp).hourHeight(phoneViewport)).isEqualTo(200.dp)
     }
 
     @Test

app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt

@@ -45,6 +45,35 @@ class TimelineZoomTest {
         assertThat(anchoredScroll(0, 0f, 56f, 84f)).isWithin(0.001f).of(0f)
     }
 
+    @Test
+    fun `a pinch lands on whole pixels`() {
+        // The gutter is 24 stacked hour-tall boxes, each rounding its own height,
+        // while the lines and blocks are drawn at the fractional one — a
+        // fractional hour puts the two on different grids and jumps the labels
+        // about as the pinch drifts across each half pixel.
+        for (target in listOf(56.4f, 56.6f, 83.5f, 120.01f)) {
+            val landed = pinchedHourHeightPx(target, fillPx = 28f, maxPx = 240f)
+            assertThat(landed).isEqualTo(landed.toInt().toFloat())
+        }
+    }
+
+    @Test
+    fun `a pinch stops where the day fills the screen`() {
+        assertThat(pinchedHourHeightPx(target = 5f, fillPx = 28f, maxPx = 240f)).isEqualTo(28f)
+    }
+
+    @Test
+    fun `filling the screen outranks the ceiling`() {
+        // On a viewport tall enough for the two to disagree, dead space under
+        // midnight is the worse of the two failures.
+        assertThat(pinchedHourHeightPx(target = 10f, fillPx = 300f, maxPx = 240f)).isEqualTo(300f)
+    }
+
+    @Test
+    fun `a pinch stops at the ceiling`() {
+        assertThat(pinchedHourHeightPx(target = 9_000f, fillPx = 28f, maxPx = 240f)).isEqualTo(240f)
+    }
+
     @Test
     fun `a settled pinch is what gets persisted`() {
         var persisted: TimelineScale? = null