diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt index a336a1b..69527f9 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt @@ -19,7 +19,6 @@ 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.ceil import kotlin.math.floor import kotlin.math.roundToInt @@ -197,18 +196,26 @@ fun rememberTimelinePinchZoom( * 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. * - * The bounds themselves are pulled onto that grid too, each in the direction - * that keeps its own promise — up for the fill floor, so no dead space opens - * under midnight, down for the ceiling. A fractional bound would be a height the - * pinch can be held against but never actually land on, and the difference feeds - * the focal anchor a scroll correction on every frame the fingers sit still. + * The ceiling is pulled onto that grid too, downwards, so it stays a height the + * pinch can actually land on. + * + * [fillPx] is deliberately *not* rounded (#290). It is the one height the whole + * day exactly fills the viewport at, and it is the same value + * [TimelineScale.FitDay] resolves to — rounding it up by the fraction of a pixel + * that 24 hours don't divide the viewport into leaves the timeline a pixel per + * hour taller than its own viewport, so a pinched-all-the-way-out day still + * scrolls a hair and bounces off Android's overscroll stretch, while the + * identical FitDay preset sits still. Being the clamp result rather than a bound + * the gesture is merely held against, it is a height the pinch does land on: the + * next frame reads it back unchanged and the focal anchor is handed nothing to + * correct. */ 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.roundToInt().toFloat() .coerceAtMost(floor(maxPx)) - .coerceAtLeast(ceil(fillPx)) + .coerceAtLeast(fillPx) /** * The scroll offset that keeps the moment under [centroidY] under it after the diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt index b804a37..346dc8c 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt @@ -1,5 +1,6 @@ package de.jeanlucmakiola.calendula.ui.common +import androidx.compose.ui.unit.Density import androidx.compose.ui.unit.dp import com.google.common.truth.Truth.assertThat import org.junit.jupiter.api.Test @@ -75,17 +76,16 @@ class TimelineZoomTest { } @Test - fun `a pinch held against a fractional bound stays put`() { - // A bound that is not a whole pixel is a height the pinch can be pushed - // against but never land on, so every frame of a held gesture would look - // like a scale change and hand the focal anchor a scroll correction. + fun `a pinch held against either bound stays put`() { val fillPx = 62.083f val maxPx = 616.5f val floor = pinchedHourHeightPx(target = 1f, fillPx, maxPx) val ceiling = pinchedHourHeightPx(target = 9_000f, fillPx, maxPx) - assertThat(floor).isEqualTo(63f) + // The ceiling is pulled onto the pixel grid so it stays landable; the + // fill floor is landable as it is, being the clamp result itself. + assertThat(floor).isEqualTo(fillPx) assertThat(ceiling).isEqualTo(616f) // Landing there and being pushed further must not move them again. assertThat(pinchedHourHeightPx(floor * 0.9f, fillPx, maxPx)).isEqualTo(floor) @@ -100,6 +100,35 @@ class TimelineZoomTest { assertThat(floor * 24).isAtLeast(viewport) } + @Test + fun `pinching all the way out leaves nothing to scroll`() { + // #290: rounding the fill floor up to a whole pixel made the day one + // pixel per hour taller than the viewport it was supposed to fill, so a + // fully zoomed-out timeline still scrolled a hair and bounced off + // Android's overscroll stretch -- while FitDay, at the same zoom, sat + // still. 1490 is deliberately not divisible by 24. + val viewport = 1490f + val floor = pinchedHourHeightPx(target = 1f, fillPx = viewport / 24f, maxPx = 616f) + assertThat(floor * 24).isWithin(0.01f).of(viewport) + } + + @Test + fun `the pinch floor is the height FitDay resolves to`() { + // The inconsistency the issue is about: the two ways to reach "the whole + // day on one screen" have to arrive at the same height. + val density = Density(2.5f) + val viewport = 596.dp + with(density) { + val fitDay = TimelineScale.FitDay.hourHeight(viewport).toPx() + val pinched = pinchedHourHeightPx( + target = 1f, + fillPx = fillHourHeight(viewport).toPx(), + maxPx = MAX_PINCH_HOUR_HEIGHT.toPx(), + ) + assertThat(pinched).isWithin(0.01f).of(fitDay) + } + } + @Test fun `a settled pinch is what gets persisted`() { var persisted: TimelineScale? = null