diff --git a/chartLib/build.gradle.kts b/chartLib/build.gradle.kts index 348da2a2..6b337b26 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 00000000..6d2c29f2 --- /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) + } +} 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 bc6c2b60..c6ec5006 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