Preserve the visible range when the content rect is resized (chart jumps sideways after a zoom)#811
Open
oliverClimbs wants to merge 1 commit into
Conversation
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.
Collaborator
Yes, it's always good to have tests. Please add an androidTest in advance to see the improvement with this. (If possible) Thank you ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The chart jumps sideways the moment a zoom gesture ends.
BarLineChartTouchListenerdeferschart.calculateOffsets()toACTION_UP:Zooming the y-axis changes the width of the y-axis labels (e.g.
1000becomes1012.5), so this deferred call resizes the content rect only once the gesture finishes.The pan, however, lives in
matrixTouchas a pixel translation, while the value-to-pixel scale is derived from the content rect width.restrainViewPort()resizes the rect without re-mapping that translation, so the same pixel offset suddenly denotes a different x-value and the visible range slides:The displacement is a fraction of the entire x-range, independent of zoom depth, and it grows with
transX— so the further you are zoomed in and panned, the more violent it gets.Measured on device
A chart whose x-axis spans a ~30-minute recording, zoomed to 112×, panned right:
ACTION_UPscaleXandtransXare unchanged, but a 3px content-rect change moved the viewport by 81,912 ms — about 82 seconds of data. In another capture, a 2px change at 112× zoom displaced the chart by 143 pixels.An
X_ZOOMthat happened not to change the label widths produced no offset change and no jump — which isolates the cause precisely.The same applies to the fling path:
computeScroll()also callscalculateOffsets()when the deceleration settles.Fix
Rescale the touch matrix's translation by the same ratio as the content rect whenever the rect is resized, then re-clamp. This 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 — so preserving the left edge preserves the whole window.It is a no-op before the first layout (
previousWidth == 0), and a no-op when unzoomed (transX == 0).Verification
Verified on a physical device (moto g stylus 5G, Android 14) against tag
3.1.0.31, built as an AAR and consumed by a real app whose chart spans a long time range:I did not add a regression test:
chartLib's JVM tests run withunitTests.isReturnDefaultValues = true, which stubsandroid.graphics.MatrixandRectFto return zeros, so the matrix arithmetic this touches cannot be exercised there, andchartLibhas no instrumented test source set. Happy to add coverage if you'd like to point me at the approach you'd prefer (e.g. Robolectric, or anandroidTestsource set for the library).