Skip to content

Commit dfcd08c

Browse files
hannesa2hannesa2
authored
Merge pull request #811 from oliverClimbs/fix/viewport-shift-on-offset-recalc
Preserve the visible range when the content rect is resized (chart jumps sideways after a zoom)
2 parents 8a23ac2 + ae79148 commit dfcd08c

3 files changed

Lines changed: 263 additions & 0 deletions

File tree

‎chartLib/build.gradle.kts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ android {
1919
buildConfigField("String", "VERSION_NAME", "\"${getVersionText()}\"")
2020

2121
consumerProguardFiles.add(File("proguard-lib.pro"))
22+
23+
testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
2224
}
2325
compileOptions {
2426
sourceCompatibility = JavaVersion.VERSION_17
@@ -60,6 +62,13 @@ dependencies {
6062
testImplementation("junit:junit:4.13.2")
6163
testImplementation("org.mockito:mockito-core:5.23.0")
6264
testImplementation("org.mockito:mockito-inline:5.2.0")
65+
66+
// Instrumented tests run on a device so that android.graphics.Matrix / RectF do real
67+
// math - the local JVM unit tests stub them out (unitTests.isReturnDefaultValues), which
68+
// is fine for pure-data tests but cannot exercise the viewport transform.
69+
androidTestImplementation("junit:junit:4.13.2")
70+
androidTestImplementation("androidx.test.ext:junit:1.3.0")
71+
androidTestImplementation("androidx.test:runner:1.7.0")
6372
}
6473

6574
tasks.register<Jar>("androidSourcesJar") {

‎chartLib/src/androidTest/kotlin/info/appdev/charting/utils/ViewPortHandlerViewportPreservationTest.kt‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
package info.appdev.charting.utils
2+
3+
import android.graphics.Matrix
4+
import androidx.test.ext.junit.runners.AndroidJUnit4
5+
import org.junit.Assert.assertEquals
6+
import org.junit.Assert.assertTrue
7+
import org.junit.Test
8+
import org.junit.runner.RunWith
9+
import kotlin.math.abs
10+
11+
/**
12+
* Regression coverage for the viewport jumping sideways when the content rect is resized
13+
* (see [ViewPortHandler.restrainViewPort] / preserveViewPortOnResize).
14+
*
15+
* Scenario reproduced from a real chart: the user zooms into a long x-axis (e.g. a
16+
* session-length line chart) and pans to some point in the middle. Zooming the y-axis widens
17+
* the y-axis labels, so on the following layout the left offset grows and the content rect
18+
* shrinks. Because the pan is stored in the touch matrix as a *pixel* translation while the
19+
* value-to-pixel scale is derived from the content rect width, resizing the rect without
20+
* re-mapping the translation makes the same pixel offset denote a different x-value, and the
21+
* visible range slides.
22+
*
23+
* These are instrumented tests because they rely on the real [android.graphics.Matrix]; the
24+
* library's local JVM unit tests stub Android graphics out and cannot exercise the transform.
25+
*/
26+
@RunWith(AndroidJUnit4::class)
27+
class ViewPortHandlerViewportPreservationTest {
28+
29+
// ---- Chart geometry (pixels) ---------------------------------------------------------
30+
// Note: deliberately not named chartWidth/chartHeight - those would shadow the receiver's
31+
// properties inside ViewPortHandler.apply { ... }.
32+
private val chartWidthPx = 1000f
33+
private val chartHeightPx = 600f
34+
private val topOffset = 20f
35+
private val rightOffset = 30f
36+
private val bottomOffset = 40f
37+
38+
// Left axis label width before and after a y-zoom widens the labels. The extra 40px is
39+
// what shrinks the content rect and used to drag the chart sideways.
40+
private val leftOffsetBefore = 60f
41+
private val leftOffsetAfter = 100f
42+
43+
// Bottom (x-axis label) height. Growing it as well shrinks the content height, which
44+
// exercises the vertical half of the same rescaling.
45+
private val bottomOffsetAfter = 80f
46+
47+
// ---- Data range mapped onto the chart ------------------------------------------------
48+
// e.g. a one-hour session in seconds; deltaX is large, so a small rect change is a very
49+
// visible x shift.
50+
private val xChartMin = 0f
51+
private val deltaX = 3600f
52+
private val yChartMin = 0f
53+
private val deltaY = 100f
54+
55+
private val zoomX = 100f
56+
private val zoomY = 5f
57+
58+
// The x-value we pan to the left edge before the resize happens.
59+
private val targetLeftX = 1800f
60+
61+
@Test
62+
fun visibleXRange_survivesContentRectResize_whenYAxisLabelsGrow() {
63+
val handler = ViewPortHandler().apply {
64+
setChartDimens(chartWidthPx, chartHeightPx)
65+
setMinMaxScaleX(1f, 1000f)
66+
setMinMaxScaleY(1f, 1000f)
67+
}
68+
69+
// Narrow-label ("before y-zoom") layout, then zoom and pan to targetLeftX.
70+
handler.restrainViewPort(leftOffsetBefore, topOffset, rightOffset, bottomOffset)
71+
zoom(handler, zoomX, zoomY)
72+
panLeftEdgeTo(handler, targetLeftX)
73+
74+
val lowestVisibleBefore = lowestVisibleX(handler)
75+
// Sanity: the pan actually put targetLeftX at the left edge.
76+
assertEquals(targetLeftX.toDouble(), lowestVisibleBefore, 1.0)
77+
78+
// Capture the touch matrix as it stood before the resize so we can measure what the
79+
// pre-fix behaviour (rect resized, translation carried over unchanged) would produce.
80+
val touchBeforeResize = Matrix(handler.matrixTouch)
81+
82+
// Wide-label ("after y-zoom") layout: left offset grows, content rect shrinks. With
83+
// the fix, restrainViewPort rescales the pan so the visible range stays put.
84+
handler.restrainViewPort(leftOffsetAfter, topOffset, rightOffset, bottomOffset)
85+
val lowestVisibleAfter = lowestVisibleX(handler)
86+
87+
// What the unfixed library did: same shrunk rect, but the translation not re-mapped.
88+
val lowestVisibleWithoutFix = lowestVisibleXForResizeWithoutRescale(touchBeforeResize)
89+
90+
val fixedShift = abs(lowestVisibleAfter - lowestVisibleBefore)
91+
val unfixedShift = abs(lowestVisibleWithoutFix - lowestVisibleBefore)
92+
93+
// The fix holds the left edge to within a fraction of an x-unit: the tolerance of 1.0
94+
// (out of a 3600-unit range) only absorbs floating-point rounding in the matrix math.
95+
assertTrue(
96+
"Visible left edge shifted by $fixedShift x-units after the content rect resize " +
97+
"(before=$lowestVisibleBefore, after=$lowestVisibleAfter)",
98+
fixedShift < 1.0
99+
)
100+
101+
// Without the fix the same resize (a 40px left-offset growth on a 910px content rect)
102+
// slides the chart by tens of x-units - here ~83 out of 3600 - proving the bug is real
103+
// and that this test would fail on the unpatched code.
104+
assertTrue(
105+
"Expected a large pre-fix shift to demonstrate the bug, but it was only " +
106+
"$unfixedShift x-units (would-be lowestVisibleX=$lowestVisibleWithoutFix)",
107+
unfixedShift > 25.0
108+
)
109+
}
110+
111+
@Test
112+
fun restrainViewPort_rescalesPanProportionallyOnBothAxes() {
113+
val handler = ViewPortHandler().apply {
114+
setChartDimens(chartWidthPx, chartHeightPx)
115+
setMinMaxScaleX(1f, 1000f)
116+
setMinMaxScaleY(1f, 1000f)
117+
}
118+
119+
handler.restrainViewPort(leftOffsetBefore, topOffset, rightOffset, bottomOffset)
120+
zoom(handler, zoomX, zoomY)
121+
panLeftEdgeTo(handler, targetLeftX)
122+
// Also pan vertically so transY is non-zero and the y-axis rescaling is actually tested.
123+
panVerticallyBy(handler, 1000f)
124+
125+
val widthBefore = handler.contentWidth()
126+
val heightBefore = handler.contentHeight()
127+
val transXBefore = handler.transX
128+
val transYBefore = handler.transY
129+
130+
// Grow both the left (y-axis) and bottom (x-axis) labels, shrinking the rect on both axes.
131+
handler.restrainViewPort(leftOffsetAfter, topOffset, rightOffset, bottomOffsetAfter)
132+
133+
val widthAfter = handler.contentWidth()
134+
val heightAfter = handler.contentHeight()
135+
val transXAfter = handler.transX
136+
val transYAfter = handler.transY
137+
138+
// Guard against a vacuous test: the rect must actually have changed size on both axes.
139+
assertTrue(widthAfter < widthBefore && heightAfter < heightBefore)
140+
assertTrue(transXBefore != 0f && transYBefore != 0f)
141+
142+
// The visible left fraction is -transX / (contentWidth * scaleX); with scaleX fixed
143+
// across a pure resize it reduces to transX / contentWidth staying constant.
144+
assertEquals(
145+
transXBefore / widthBefore,
146+
transXAfter / widthAfter,
147+
1e-3f
148+
)
149+
150+
// The fix rescales the vertical pan by the same ratio, so transY / contentHeight is
151+
// preserved too.
152+
assertEquals(
153+
transYBefore / heightBefore,
154+
transYAfter / heightAfter,
155+
1e-3f
156+
)
157+
}
158+
159+
// ---- helpers -------------------------------------------------------------------------
160+
161+
/** Applies a zoom about the origin, matching a pinch that leaves the pan untouched. */
162+
private fun zoom(handler: ViewPortHandler, scaleX: Float, scaleY: Float) {
163+
val zoomed = handler.zoom(scaleX, scaleY)
164+
handler.refresh(zoomed, null, false)
165+
}
166+
167+
/** Pans the current viewport so that [valueX] sits exactly at the content left edge. */
168+
private fun panLeftEdgeTo(handler: ViewPortHandler, valueX: Float) {
169+
val transformer = preparedTransformer(handler)
170+
val pixel = transformer.getPixelForValues(valueX, yChartMin)
171+
val dx = (handler.contentLeft() - pixel.x).toFloat()
172+
173+
val panned = Matrix(handler.matrixTouch).apply { postTranslate(dx, 0f) }
174+
handler.refresh(panned, null, false)
175+
}
176+
177+
/** Pans the viewport vertically by [pixels] (positive = content shifts down). */
178+
private fun panVerticallyBy(handler: ViewPortHandler, pixels: Float) {
179+
val panned = Matrix(handler.matrixTouch).apply { postTranslate(0f, pixels) }
180+
handler.refresh(panned, null, false)
181+
}
182+
183+
/** The chart's lowestVisibleX: value at the bottom-left corner of the content rect. */
184+
private fun lowestVisibleX(handler: ViewPortHandler): Double {
185+
val transformer = preparedTransformer(handler)
186+
return transformer
187+
.getValuesByTouchPoint(handler.contentLeft(), handler.contentBottom())
188+
.x
189+
}
190+
191+
/**
192+
* Reproduces the pre-fix behaviour: shrink the content rect to the wide-label layout but
193+
* keep the pre-resize touch matrix unchanged (i.e. the translation is NOT re-mapped). Uses
194+
* a throwaway handler already sized to the narrow rect so no rescale is triggered.
195+
*/
196+
private fun lowestVisibleXForResizeWithoutRescale(touchBeforeResize: Matrix): Double {
197+
val handler = ViewPortHandler().apply {
198+
setChartDimens(chartWidthPx, chartHeightPx)
199+
setMinMaxScaleX(1f, 1000f)
200+
setMinMaxScaleY(1f, 1000f)
201+
// Size straight to the wide-label layout, then drop in the untouched matrix.
202+
restrainViewPort(leftOffsetAfter, topOffset, rightOffset, bottomOffset)
203+
refresh(Matrix(touchBeforeResize), null, false)
204+
}
205+
return lowestVisibleX(handler)
206+
}
207+
208+
private fun preparedTransformer(handler: ViewPortHandler): Transformer =
209+
Transformer(handler).apply {
210+
prepareMatrixOffset(false)
211+
prepareMatrixValuePx(xChartMin, deltaX, deltaY, yChartMin)
212+
}
213+
}

‎chartLib/src/main/kotlin/info/appdev/charting/utils/ViewPortHandler.kt‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,52 @@ open class ViewPortHandler {
105105
fun hasChartDimens(): Boolean = chartHeight > 0 && chartWidth > 0
106106

107107
fun restrainViewPort(offsetLeft: Float, offsetTop: Float, offsetRight: Float, offsetBottom: Float, logging: Boolean = false) {
108+
val previousWidth = contentRect.width()
109+
val previousHeight = contentRect.height()
110+
108111
contentRect[offsetLeft, offsetTop, chartWidth - offsetRight] = (chartHeight - offsetBottom)
112+
113+
preserveViewPortOnResize(previousWidth, previousHeight)
114+
109115
if (logging)
110116
Timber.i(contentRect.toString())
111117
}
112118

119+
/**
120+
* Keeps the visible data range fixed when the content rect is resized.
121+
*
122+
* The pan is held in [matrixTouch] as a *pixel* translation, while the value-to-pixel
123+
* scale is derived from the content rect. Resizing the rect therefore makes the same
124+
* pixel translation denote a different value, sliding the visible range.
125+
*
126+
* This bites hardest after a zoom gesture: BarLineChartTouchListener defers
127+
* chart.calculateOffsets() to ACTION_UP, so a y-zoom that widens the y-axis labels
128+
* resizes the content rect only once the gesture ends. The chart then jumps sideways by
129+
* roughly (x-range * change-in-width / content width) - on a chart whose x-axis spans a
130+
* long range, a few pixels of label growth is a very visible jump.
131+
*
132+
* Scaling the translation by the same ratio as the rect keeps the visible range where the
133+
* user left it. The visible extent itself is unaffected: it is (axis range / touch scale),
134+
* which does not depend on the content width.
135+
*/
136+
private fun preserveViewPortOnResize(previousWidth: Float, previousHeight: Float) {
137+
// Nothing to preserve before the first layout, or when the rect did not change size.
138+
if (previousWidth <= 0f || previousHeight <= 0f) return
139+
140+
val width = contentRect.width()
141+
val height = contentRect.height()
142+
143+
if (width == previousWidth && height == previousHeight) return
144+
if (width <= 0f || height <= 0f) return
145+
146+
matrixTouch.getValues(matrixBuffer)
147+
matrixBuffer[Matrix.MTRANS_X] *= width / previousWidth
148+
matrixBuffer[Matrix.MTRANS_Y] *= height / previousHeight
149+
matrixTouch.setValues(matrixBuffer)
150+
151+
limitTransAndScale(matrixTouch, contentRect)
152+
}
153+
113154
fun offsetLeft(): Float = contentRect.left
114155

115156
fun offsetRight(): Float = chartWidth - contentRect.right

0 commit comments

Comments
 (0)