fix infinite loop in numAxisSplits for big-magnitude flat ranges - #1142
Open
cpruijsen wants to merge 1 commit into
Open
fix infinite loop in numAxisSplits for big-magnitude flat ranges#1142cpruijsen wants to merge 1 commit into
cpruijsen wants to merge 1 commit into
Conversation
numIntDigits() wrapped to int32 for |x| >= 2^31, so findIncr's significant-digits guard under-counted the magnitude and could pick a sub-ULP foundIncr (e.g. 1e-8 at 1e14) that can never advance val in the numAxisSplits loop, hanging the browser. - numIntDigits: use log10(abs(x)) outside the int32 fast range - numAxisSplits: require val to strictly increase each iteration
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
1 participant
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.
numAxisSplitsinsrc/opts.jscan hang the page. Its loop advances withval = roundDec(val + foundIncr, numDec), and whennumDecis too small for the increment, therounded result comes back equal to the previous
val. The conditionval <= scaleMaxstays trueforever and the tab locks up rather than rendering a wrong axis.
Two changes, in the order the failure happens.
numIntDigitsinsrc/utils.jscomputed the digit count with the branchless int32 absolute value(x ^ (x >> 31)) - (x >> 31). Both shifts coerce to a signed 32 bit integer, so any magnitude at orabove 2^31 wraps and the digit count comes back wrong, which is what produces the
numDecthatcannot represent the increment. It now takes
abs(x)and only truncates with| 0while the valueis inside int32 range, using the float directly above it.
The loop then carries a
prevValguard and stops when a step fails to advance. That is a backstoprather than the fix: with the digit count corrected the loop terminates on its own, but a stalled
step should end the loop rather than the session, since the remaining causes of a non advancing step
are floating point ones that are hard to enumerate.
Fixes #827