Skip to content

Commit

Permalink
Merge pull request #15015 from apache/fix/stack-miss-data
Browse files Browse the repository at this point in the history
Fix: (1) number getPrecisionSafe incorrect on scientific notation like 3.45e-1 (2) stack sum eliminate floating arithmetic problem. (3) Should no dataZoom filtering when `toolbox.feature.dataZoom` not declared.
  • Loading branch information
100pah authored May 31, 2021
2 parents c695011 + 40b599a commit 0d4758b
Show file tree
Hide file tree
Showing 11 changed files with 694 additions and 34 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"test:visual": "node test/runTest/server.js",
"test": "npx jest --config test/ut/jest.config.js",
"test:single": "npx jest --config test/ut/jest.config.js --coverage=false -t",
"test:single:debug": "npx --node-arg=--inspect-brk jest --runInBand --config test/ut/jest.config.js --coverage=false -t",
"test:dts": "node build/testDts.js",
"mktest": "node test/build/mktest.js",
"mktest:help": "node test/build/mktest.js -h",
Expand Down
5 changes: 3 additions & 2 deletions src/component/toolbox/feature/DataZoom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,11 @@ function updateZoomBtnStatus(

registerInternalOptionCreator('dataZoom', function (ecModel: GlobalModel): ComponentOption[] {
const toolboxModel = ecModel.getComponent('toolbox', 0) as ToolboxModel;
if (!toolboxModel) {
const featureDataZoomPath = ['feature', 'dataZoom'] as const;
if (!toolboxModel || toolboxModel.get(featureDataZoomPath) == null) {
return;
}
const dzFeatureModel = toolboxModel.getModel(['feature', 'dataZoom'] as any) as ToolboxDataZoomFeatureModel;
const dzFeatureModel = toolboxModel.getModel(featureDataZoomPath as any) as ToolboxDataZoomFeatureModel;
const dzOptions = [] as ComponentOption[];

const finder = makeAxisFinder(dzFeatureModel);
Expand Down
6 changes: 5 additions & 1 deletion src/processor/dataStack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import GlobalModel from '../model/Global';
import SeriesModel from '../model/Series';
import { SeriesOption, SeriesStackOptionMixin, DimensionName } from '../util/types';
import List from '../data/List';
import { addSafe } from '../util/number';

interface StackInfo {
stackedDimension: DimensionName
Expand Down Expand Up @@ -125,7 +126,10 @@ function calculateStack(stackInfoList: StackInfo[]) {
if ((sum >= 0 && val > 0) // Positive stack
|| (sum <= 0 && val < 0) // Negative stack
) {
sum += val;
// The sum should be as less as possible to be effected
// by floating arithmetic problem. A wrong result probably
// filtered incorrectly by axis min/max.
sum = addSafe(sum, val);
stackedOver = val;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/scale/Interval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class IntervalScale<SETTING extends Dictionary<unknown> = Dictionary<unknown>> e
let precision = opt && opt.precision;

if (precision == null) {
precision = numberUtil.getPrecisionSafe(data.value) || 0;
precision = numberUtil.getPrecision(data.value) || 0;
}
else if (precision === 'auto') {
// Should be more precise then tick.
Expand Down
3 changes: 1 addition & 2 deletions src/scale/Log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const scaleProto = Scale.prototype;
// FIXME:TS refactor: not good to call it directly with `this`?
const intervalScaleProto = IntervalScale.prototype;

const getPrecisionSafe = numberUtil.getPrecisionSafe;
const roundingErrorFix = numberUtil.round;

const mathFloor = Math.floor;
Expand Down Expand Up @@ -201,7 +200,7 @@ proto.getLabel = intervalScaleProto.getLabel;


function fixRoundingError(val: number, originalVal: number): number {
return roundingErrorFix(val, getPrecisionSafe(originalVal));
return roundingErrorFix(val, numberUtil.getPrecision(originalVal));
}


Expand Down
2 changes: 1 addition & 1 deletion src/scale/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export function intervalScaleNiceTicks(
*/
export function getIntervalPrecision(interval: number): number {
// Tow more digital for tick.
return numberUtil.getPrecisionSafe(interval) + 2;
return numberUtil.getPrecision(interval) + 2;
}

function clamp(
Expand Down
10 changes: 5 additions & 5 deletions src/util/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { Dictionary } from 'zrender/src/core/types';
import SeriesModel from '../model/Series';
import CartesianAxisModel from '../coord/cartesian/AxisModel';
import GridModel from '../coord/cartesian/GridModel';
import { isNumeric, getRandomIdBase, getPrecisionSafe, round } from './number';
import { isNumeric, getRandomIdBase, getPrecision, round } from './number';
import { interpolateNumber } from 'zrender/src/animation/Animator';
import { warn } from './log';

Expand Down Expand Up @@ -1053,8 +1053,8 @@ export function interpolateRawValues(
return round(
value,
isAutoPrecision ? Math.max(
getPrecisionSafe(sourceValue as number || 0),
getPrecisionSafe(targetValue as number)
getPrecision(sourceValue as number || 0),
getPrecision(targetValue as number)
)
: precision as number
);
Expand All @@ -1081,8 +1081,8 @@ export function interpolateRawValues(
interpolated[i] = round(
value,
isAutoPrecision ? Math.max(
getPrecisionSafe(leftVal),
getPrecisionSafe(rightVal)
getPrecision(leftVal),
getPrecision(rightVal)
)
: precision as number
);
Expand Down
59 changes: 42 additions & 17 deletions src/util/number.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
import * as zrUtil from 'zrender/src/core/util';

const RADIAN_EPSILON = 1e-4;
// Although chrome already enlarge this number to 100 for `toFixed`, but
// we sill follow the spec for compatibility.
const ROUND_SUPPORTED_PRECISION_MAX = 20;

function _trim(str: string): string {
return str.replace(/^\s+|\s+$/g, '');
Expand Down Expand Up @@ -138,7 +141,8 @@ export function round(x: number | string, precision?: number, returnStr?: boolea
precision = 10;
}
// Avoid range error
precision = Math.min(Math.max(0, precision), 20);
precision = Math.min(Math.max(0, precision), ROUND_SUPPORTED_PRECISION_MAX);
// PENDING: 1.005.toFixed(2) is '1.00' rather than '1.01'
x = (+x).toFixed(precision);
return (returnStr ? x : +x);
}
Expand All @@ -155,42 +159,49 @@ export function asc<T extends number[]>(arr: T): T {
}

/**
* Get precision
* Get precision.
*/
export function getPrecision(val: string | number): number {
val = +val;
if (isNaN(val)) {
return 0;
}

// It is much faster than methods converting number to string as follows
// let tmp = val.toString();
// return tmp.length - 1 - tmp.indexOf('.');
// especially when precision is low
let e = 1;
let count = 0;
while (Math.round(val * e) / e !== val) {
e *= 10;
count++;
// Notice:
// (1) If the loop count is over about 20, it is slower than `getPrecisionSafe`.
// (see https://jsbench.me/2vkpcekkvw/1)
// (2) If the val is less than for example 1e-15, the result may be incorrect.
// (see test/ut/spec/util/number.test.ts `getPrecision_equal_random`)
if (val > 1e-14) {
let e = 1;
for (let i = 0; i < 15; i++, e *= 10) {
if (Math.round(val * e) / e === val) {
return i;
}
}
}
return count;

return getPrecisionSafe(val);
}

/**
* Get precision with slow but safe method
*/
export function getPrecisionSafe(val: string | number): number {
const str = val.toString();
// toLowerCase for: '3.4E-12'
const str = val.toString().toLowerCase();

// Consider scientific notation: '3.4e-12' '3.4e+12'
const eIndex = str.indexOf('e');
if (eIndex > 0) {
const precision = +str.slice(eIndex + 1);
return precision < 0 ? -precision : 0;
}
else {
const dotIndex = str.indexOf('.');
return dotIndex < 0 ? 0 : str.length - 1 - dotIndex;
}
const exp = eIndex > 0 ? +str.slice(eIndex + 1) : 0;
const significandPartLen = eIndex > 0 ? eIndex : str.length;
const dotIndex = str.indexOf('.');
const decimalPartLen = dotIndex < 0 ? 0 : significandPartLen - 1 - dotIndex;
return Math.max(0, decimalPartLen - exp);
}

/**
Expand Down Expand Up @@ -268,6 +279,20 @@ export function getPercentWithPrecision(valueList: number[], idx: number, precis
return seats[idx] / digits;
}

/**
* Solve the floating point adding problem like 0.1 + 0.2 === 0.30000000000000004
* See <http://0.30000000000000004.com/>
*/
export function addSafe(val0: number, val1: number): number {
const maxPrecision = Math.max(getPrecision(val0), getPrecision(val1));
// const multiplier = Math.pow(10, maxPrecision);
// return (Math.round(val0 * multiplier) + Math.round(val1 * multiplier)) / multiplier;
const sum = val0 + val1;
// // PENDING: support more?
return maxPrecision > ROUND_SUPPORTED_PRECISION_MAX
? sum : round(sum, maxPrecision);
}

// Number.MAX_SAFE_INTEGER, ie do not support.
export const MAX_SAFE_INTEGER = 9007199254740991;

Expand Down
1 change: 1 addition & 0 deletions test/axis-filter-extent.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

158 changes: 158 additions & 0 deletions test/axis-filter-extent2.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 0d4758b

Please sign in to comment.