fix(scale): make time getValueForPixel the inverse of getPixelForValue - #12289
Open
contactjawad wants to merge 1 commit into
Open
fix(scale): make time getValueForPixel the inverse of getPixelForValue#12289contactjawad wants to merge 1 commit into
contactjawad wants to merge 1 commit into
Conversation
getPixelForValue maps a value using offsets.start, so the pixel->value inverse must subtract offsets.start, not offsets.end. On an offset axis with asymmetric margins (offsets.start !== offsets.end) the conversion was wrong by a constant (start - end) * (max - min), so hover/tooltip lookups mapped to the wrong value. Apply the same correction to the timeseries scale.
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.
What
TimeScale.getValueForPixel(andTimeSeriesScale.getValueForPixel) is meant to be the exact inverse ofgetPixelForValue, but it subtracts the wrong margin offset. On a time axis withoffset: trueand irregularly spaced ticks — whereoffsets.start !== offsets.end— converting a pixel back to a value is wrong by a constant, so hover/tooltip/nearest-element interaction maps to the wrong data.Why
getPixelForValuemaps a value to a decimal usingoffsets.start:so the inverse must be
pos = decimal / factor - offsets.start. Instead the code usedoffsets.end:When the two margins differ, the result is off by
(start - end) * (max - min).How
Subtract
offsets.startin bothsrc/scales/scale.time.jsandsrc/scales/scale.timeseries.js, makinggetValueForPixelthe exact inverse ofgetPixelForValue(which usesoffsets.start).Test
Added a spec in
test/specs/scale.time.tests.jsthat builds a time axis withoffset: trueand irregularly spaced labels sooffsets.start !== offsets.end(the test asserts this premise), then verifies the round tripgetValueForPixel(getPixelForValue(v))returnsv. Before the fix the round trip is off by ~21 hours for the chosen data; after the fix it matches.