From b10d1920def2ed9c8f446d013c0384d4b802d977 Mon Sep 17 00:00:00 2001 From: Oliver Date: Sat, 11 Jul 2026 18:54:45 -0700 Subject: [PATCH 1/2] Fix viewport jumping sideways when axis offsets are recalculated The pan is stored in the touch matrix as a pixel translation, while the value-to-pixel scale is derived from the content rect. Resizing the content rect therefore makes the same pixel translation denote a different value, sliding the visible range. This is most visible after a zoom gesture: BarLineChartTouchListener defers chart.calculateOffsets() to ACTION_UP, so a y-zoom that widens the y-axis labels resizes the content rect only once the gesture ends, and the chart jumps sideways. The same happens when a fling settles, via computeScroll(). Scale the translation by the same ratio as the content rect so the visible range survives the resize. The visible extent is unaffected: it is (axis range / touch scale), which does not depend on the content width. --- .../appdev/charting/utils/ViewPortHandler.kt | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/chartLib/src/main/kotlin/info/appdev/charting/utils/ViewPortHandler.kt b/chartLib/src/main/kotlin/info/appdev/charting/utils/ViewPortHandler.kt index bc6c2b60c..c6ec5006a 100644 --- a/chartLib/src/main/kotlin/info/appdev/charting/utils/ViewPortHandler.kt +++ b/chartLib/src/main/kotlin/info/appdev/charting/utils/ViewPortHandler.kt @@ -105,11 +105,52 @@ open class ViewPortHandler { fun hasChartDimens(): Boolean = chartHeight > 0 && chartWidth > 0 fun restrainViewPort(offsetLeft: Float, offsetTop: Float, offsetRight: Float, offsetBottom: Float, logging: Boolean = false) { + val previousWidth = contentRect.width() + val previousHeight = contentRect.height() + contentRect[offsetLeft, offsetTop, chartWidth - offsetRight] = (chartHeight - offsetBottom) + + preserveViewPortOnResize(previousWidth, previousHeight) + if (logging) Timber.i(contentRect.toString()) } + /** + * Keeps the visible data range fixed when the content rect is resized. + * + * The pan is held in [matrixTouch] as a *pixel* translation, while the value-to-pixel + * scale is derived from the content rect. Resizing the rect therefore makes the same + * pixel translation denote a different value, sliding the visible range. + * + * This bites hardest after a zoom gesture: BarLineChartTouchListener defers + * chart.calculateOffsets() to ACTION_UP, so a y-zoom that widens the y-axis labels + * resizes the content rect only once the gesture ends. The chart then jumps sideways by + * roughly (x-range * change-in-width / content width) - on a chart whose x-axis spans a + * long range, a few pixels of label growth is a very visible jump. + * + * Scaling the translation by the same ratio as the rect keeps the visible range where the + * user left it. The visible extent itself is unaffected: it is (axis range / touch scale), + * which does not depend on the content width. + */ + private fun preserveViewPortOnResize(previousWidth: Float, previousHeight: Float) { + // Nothing to preserve before the first layout, or when the rect did not change size. + if (previousWidth <= 0f || previousHeight <= 0f) return + + val width = contentRect.width() + val height = contentRect.height() + + if (width == previousWidth && height == previousHeight) return + if (width <= 0f || height <= 0f) return + + matrixTouch.getValues(matrixBuffer) + matrixBuffer[Matrix.MTRANS_X] *= width / previousWidth + matrixBuffer[Matrix.MTRANS_Y] *= height / previousHeight + matrixTouch.setValues(matrixBuffer) + + limitTransAndScale(matrixTouch, contentRect) + } + fun offsetLeft(): Float = contentRect.left fun offsetRight(): Float = chartWidth - contentRect.right From ae79148c48e87c34fb3ffcc3a8a48d6102d7a998 Mon Sep 17 00:00:00 2001 From: Oliver Date: Sat, 18 Jul 2026 11:57:13 -0700 Subject: [PATCH 2/2] Add instrumented tests for viewport preservation Adds two instrumented tests covering the viewport preservation fix in ViewPortHandler.restrainViewPort (PR #811). These must run on a device because the library's JVM unit tests stub android.graphics and cannot exercise the real Matrix math the fix depends on. The first test reproduces a real-world scenario: deeply zoomed/panned viewport, then content rect resize (from y-axis label growth). It verifies the visible x-range is preserved and reconstructs the pre-fix behavior to show it would shift ~83 x-units on a 3600-unit range. The second test asserts the pan ratios (transX/contentWidth, transY/contentHeight) are preserved across the resize. Also updates chartLib/build.gradle.kts to add androidTest dependencies. --- chartLib/build.gradle.kts | 9 + ...ViewPortHandlerViewportPreservationTest.kt | 213 ++++++++++++++++++ 2 files changed, 222 insertions(+) create mode 100644 chartLib/src/androidTest/kotlin/info/appdev/charting/utils/ViewPortHandlerViewportPreservationTest.kt diff --git a/chartLib/build.gradle.kts b/chartLib/build.gradle.kts index 348da2a28..6b337b261 100644 --- a/chartLib/build.gradle.kts +++ b/chartLib/build.gradle.kts @@ -19,6 +19,8 @@ android { buildConfigField("String", "VERSION_NAME", "\"${getVersionText()}\"") consumerProguardFiles.add(File("proguard-lib.pro")) + + testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" } compileOptions { sourceCompatibility = JavaVersion.VERSION_17 @@ -60,6 +62,13 @@ dependencies { testImplementation("junit:junit:4.13.2") testImplementation("org.mockito:mockito-core:5.23.0") testImplementation("org.mockito:mockito-inline:5.2.0") + + // Instrumented tests run on a device so that android.graphics.Matrix / RectF do real + // math - the local JVM unit tests stub them out (unitTests.isReturnDefaultValues), which + // is fine for pure-data tests but cannot exercise the viewport transform. + androidTestImplementation("junit:junit:4.13.2") + androidTestImplementation("androidx.test.ext:junit:1.3.0") + androidTestImplementation("androidx.test:runner:1.7.0") } tasks.register("androidSourcesJar") { diff --git a/chartLib/src/androidTest/kotlin/info/appdev/charting/utils/ViewPortHandlerViewportPreservationTest.kt b/chartLib/src/androidTest/kotlin/info/appdev/charting/utils/ViewPortHandlerViewportPreservationTest.kt new file mode 100644 index 000000000..6d2c29f26 --- /dev/null +++ b/chartLib/src/androidTest/kotlin/info/appdev/charting/utils/ViewPortHandlerViewportPreservationTest.kt @@ -0,0 +1,213 @@ +package info.appdev.charting.utils + +import android.graphics.Matrix +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.math.abs + +/** + * Regression coverage for the viewport jumping sideways when the content rect is resized + * (see [ViewPortHandler.restrainViewPort] / preserveViewPortOnResize). + * + * Scenario reproduced from a real chart: the user zooms into a long x-axis (e.g. a + * session-length line chart) and pans to some point in the middle. Zooming the y-axis widens + * the y-axis labels, so on the following layout the left offset grows and the content rect + * shrinks. Because the pan is stored in the touch matrix as a *pixel* translation while the + * value-to-pixel scale is derived from the content rect width, resizing the rect without + * re-mapping the translation makes the same pixel offset denote a different x-value, and the + * visible range slides. + * + * These are instrumented tests because they rely on the real [android.graphics.Matrix]; the + * library's local JVM unit tests stub Android graphics out and cannot exercise the transform. + */ +@RunWith(AndroidJUnit4::class) +class ViewPortHandlerViewportPreservationTest { + + // ---- Chart geometry (pixels) --------------------------------------------------------- + // Note: deliberately not named chartWidth/chartHeight - those would shadow the receiver's + // properties inside ViewPortHandler.apply { ... }. + private val chartWidthPx = 1000f + private val chartHeightPx = 600f + private val topOffset = 20f + private val rightOffset = 30f + private val bottomOffset = 40f + + // Left axis label width before and after a y-zoom widens the labels. The extra 40px is + // what shrinks the content rect and used to drag the chart sideways. + private val leftOffsetBefore = 60f + private val leftOffsetAfter = 100f + + // Bottom (x-axis label) height. Growing it as well shrinks the content height, which + // exercises the vertical half of the same rescaling. + private val bottomOffsetAfter = 80f + + // ---- Data range mapped onto the chart ------------------------------------------------ + // e.g. a one-hour session in seconds; deltaX is large, so a small rect change is a very + // visible x shift. + private val xChartMin = 0f + private val deltaX = 3600f + private val yChartMin = 0f + private val deltaY = 100f + + private val zoomX = 100f + private val zoomY = 5f + + // The x-value we pan to the left edge before the resize happens. + private val targetLeftX = 1800f + + @Test + fun visibleXRange_survivesContentRectResize_whenYAxisLabelsGrow() { + val handler = ViewPortHandler().apply { + setChartDimens(chartWidthPx, chartHeightPx) + setMinMaxScaleX(1f, 1000f) + setMinMaxScaleY(1f, 1000f) + } + + // Narrow-label ("before y-zoom") layout, then zoom and pan to targetLeftX. + handler.restrainViewPort(leftOffsetBefore, topOffset, rightOffset, bottomOffset) + zoom(handler, zoomX, zoomY) + panLeftEdgeTo(handler, targetLeftX) + + val lowestVisibleBefore = lowestVisibleX(handler) + // Sanity: the pan actually put targetLeftX at the left edge. + assertEquals(targetLeftX.toDouble(), lowestVisibleBefore, 1.0) + + // Capture the touch matrix as it stood before the resize so we can measure what the + // pre-fix behaviour (rect resized, translation carried over unchanged) would produce. + val touchBeforeResize = Matrix(handler.matrixTouch) + + // Wide-label ("after y-zoom") layout: left offset grows, content rect shrinks. With + // the fix, restrainViewPort rescales the pan so the visible range stays put. + handler.restrainViewPort(leftOffsetAfter, topOffset, rightOffset, bottomOffset) + val lowestVisibleAfter = lowestVisibleX(handler) + + // What the unfixed library did: same shrunk rect, but the translation not re-mapped. + val lowestVisibleWithoutFix = lowestVisibleXForResizeWithoutRescale(touchBeforeResize) + + val fixedShift = abs(lowestVisibleAfter - lowestVisibleBefore) + val unfixedShift = abs(lowestVisibleWithoutFix - lowestVisibleBefore) + + // The fix holds the left edge to within a fraction of an x-unit: the tolerance of 1.0 + // (out of a 3600-unit range) only absorbs floating-point rounding in the matrix math. + assertTrue( + "Visible left edge shifted by $fixedShift x-units after the content rect resize " + + "(before=$lowestVisibleBefore, after=$lowestVisibleAfter)", + fixedShift < 1.0 + ) + + // Without the fix the same resize (a 40px left-offset growth on a 910px content rect) + // slides the chart by tens of x-units - here ~83 out of 3600 - proving the bug is real + // and that this test would fail on the unpatched code. + assertTrue( + "Expected a large pre-fix shift to demonstrate the bug, but it was only " + + "$unfixedShift x-units (would-be lowestVisibleX=$lowestVisibleWithoutFix)", + unfixedShift > 25.0 + ) + } + + @Test + fun restrainViewPort_rescalesPanProportionallyOnBothAxes() { + val handler = ViewPortHandler().apply { + setChartDimens(chartWidthPx, chartHeightPx) + setMinMaxScaleX(1f, 1000f) + setMinMaxScaleY(1f, 1000f) + } + + handler.restrainViewPort(leftOffsetBefore, topOffset, rightOffset, bottomOffset) + zoom(handler, zoomX, zoomY) + panLeftEdgeTo(handler, targetLeftX) + // Also pan vertically so transY is non-zero and the y-axis rescaling is actually tested. + panVerticallyBy(handler, 1000f) + + val widthBefore = handler.contentWidth() + val heightBefore = handler.contentHeight() + val transXBefore = handler.transX + val transYBefore = handler.transY + + // Grow both the left (y-axis) and bottom (x-axis) labels, shrinking the rect on both axes. + handler.restrainViewPort(leftOffsetAfter, topOffset, rightOffset, bottomOffsetAfter) + + val widthAfter = handler.contentWidth() + val heightAfter = handler.contentHeight() + val transXAfter = handler.transX + val transYAfter = handler.transY + + // Guard against a vacuous test: the rect must actually have changed size on both axes. + assertTrue(widthAfter < widthBefore && heightAfter < heightBefore) + assertTrue(transXBefore != 0f && transYBefore != 0f) + + // The visible left fraction is -transX / (contentWidth * scaleX); with scaleX fixed + // across a pure resize it reduces to transX / contentWidth staying constant. + assertEquals( + transXBefore / widthBefore, + transXAfter / widthAfter, + 1e-3f + ) + + // The fix rescales the vertical pan by the same ratio, so transY / contentHeight is + // preserved too. + assertEquals( + transYBefore / heightBefore, + transYAfter / heightAfter, + 1e-3f + ) + } + + // ---- helpers ------------------------------------------------------------------------- + + /** Applies a zoom about the origin, matching a pinch that leaves the pan untouched. */ + private fun zoom(handler: ViewPortHandler, scaleX: Float, scaleY: Float) { + val zoomed = handler.zoom(scaleX, scaleY) + handler.refresh(zoomed, null, false) + } + + /** Pans the current viewport so that [valueX] sits exactly at the content left edge. */ + private fun panLeftEdgeTo(handler: ViewPortHandler, valueX: Float) { + val transformer = preparedTransformer(handler) + val pixel = transformer.getPixelForValues(valueX, yChartMin) + val dx = (handler.contentLeft() - pixel.x).toFloat() + + val panned = Matrix(handler.matrixTouch).apply { postTranslate(dx, 0f) } + handler.refresh(panned, null, false) + } + + /** Pans the viewport vertically by [pixels] (positive = content shifts down). */ + private fun panVerticallyBy(handler: ViewPortHandler, pixels: Float) { + val panned = Matrix(handler.matrixTouch).apply { postTranslate(0f, pixels) } + handler.refresh(panned, null, false) + } + + /** The chart's lowestVisibleX: value at the bottom-left corner of the content rect. */ + private fun lowestVisibleX(handler: ViewPortHandler): Double { + val transformer = preparedTransformer(handler) + return transformer + .getValuesByTouchPoint(handler.contentLeft(), handler.contentBottom()) + .x + } + + /** + * Reproduces the pre-fix behaviour: shrink the content rect to the wide-label layout but + * keep the pre-resize touch matrix unchanged (i.e. the translation is NOT re-mapped). Uses + * a throwaway handler already sized to the narrow rect so no rescale is triggered. + */ + private fun lowestVisibleXForResizeWithoutRescale(touchBeforeResize: Matrix): Double { + val handler = ViewPortHandler().apply { + setChartDimens(chartWidthPx, chartHeightPx) + setMinMaxScaleX(1f, 1000f) + setMinMaxScaleY(1f, 1000f) + // Size straight to the wide-label layout, then drop in the untouched matrix. + restrainViewPort(leftOffsetAfter, topOffset, rightOffset, bottomOffset) + refresh(Matrix(touchBeforeResize), null, false) + } + return lowestVisibleX(handler) + } + + private fun preparedTransformer(handler: ViewPortHandler): Transformer = + Transformer(handler).apply { + prepareMatrixOffset(false) + prepareMatrixValuePx(xChartMin, deltaX, deltaY, yChartMin) + } +}