fix(logScale): (1) Thoroughly resolve a long-standing issue of non-positive data on LogScale - exclude non-positive series data items when calculate dataExtent on LogScale. (2) Include Infinite into connectNulls handling on line series; the Infinite value may be generated by log(0) and previously the corresponding effect in unpredictable on line series (sometimes display as connected but sometimes not).

Author: 100pah

src/chart/line/LineSeries.ts

@@ -209,7 +209,7 @@ class LineSeriesModel extends SeriesModel<LineSeriesOption> {
         //           follow the label interval strategy.
         showAllSymbol: 'auto',
 
-        // Whether to connect break point.
+        // Whether to connect break point. (non-finite values)
         connectNulls: false,
 
         // Sampling for large data. Can be: 'average', 'max', 'min', 'sum', 'lttb'.

src/chart/line/LineView.ts

@@ -27,7 +27,7 @@ import * as graphic from '../../util/graphic';
 import * as modelUtil from '../../util/model';
 import { ECPolyline, ECPolygon } from './poly';
 import ChartView from '../../view/Chart';
-import { prepareDataCoordInfo, getStackedOnPoint } from './helper';
+import { prepareDataCoordInfo, getStackedOnPoint, isPointIllegal } from './helper';
 import { createGridClipPath, createPolarClipPath } from '../helper/createClipPathFromCoordSys';
 import LineSeriesModel, { LineSeriesOption } from './LineSeries';
 import type GlobalModel from '../../model/Global';
@@ -84,42 +84,33 @@ function isPointsSame(points1: ArrayLike<number>, points2: ArrayLike<number>) {
     return true;
 }
 
-function bboxFromPoints(points: ArrayLike<number>) {
-    let minX = Infinity;
-    let minY = Infinity;
-    let maxX = -Infinity;
-    let maxY = -Infinity;
+function xyExtentFromPoints(points: ArrayLike<number>) {
+    const xExtent = modelUtil.initExtentForUnion();
+    const yExtent = modelUtil.initExtentForUnion();
 
     for (let i = 0; i < points.length;) {
         const x = points[i++];
         const y = points[i++];
-        if (!isNaN(x)) {
-            minX = Math.min(x, minX);
-            maxX = Math.max(x, maxX);
-        }
-        if (!isNaN(y)) {
-            minY = Math.min(y, minY);
-            maxY = Math.max(y, maxY);
+        if (!isPointIllegal(x, y)) {
+            modelUtil.unionExtent(xExtent, x);
+            modelUtil.unionExtent(yExtent, y);
         }
     }
-    return [
-        [minX, minY],
-        [maxX, maxY]
-    ];
+    return [xExtent, yExtent];
 }
 
 function getBoundingDiff(points1: ArrayLike<number>, points2: ArrayLike<number>): number {
 
-    const [min1, max1] = bboxFromPoints(points1);
-    const [min2, max2] = bboxFromPoints(points2);
+    const [xExtent1, yExtent1] = xyExtentFromPoints(points1);
+    const [xExtent2, yExtent2] = xyExtentFromPoints(points2);
 
     // Get a max value from each corner of two boundings.
     return Math.max(
-        Math.abs(min1[0] - min2[0]),
-        Math.abs(min1[1] - min2[1]),
+        Math.abs(xExtent1[0] - xExtent2[0]),
+        Math.abs(yExtent1[0] - yExtent2[0]),
 
-        Math.abs(max1[0] - max2[0]),
-        Math.abs(max1[1] - max2[1])
+        Math.abs(xExtent1[1] - xExtent2[1]),
+        Math.abs(yExtent1[1] - yExtent2[1])
     );
 }
 
@@ -181,7 +172,7 @@ function turnPointsIntoStep(
              * should stay the same as the lines above. See #20021
              */
             const reference = basePoints || points;
-            if (!isNaN(reference[i]) && !isNaN(reference[i + 1])) {
+            if (!isPointIllegal(reference[i], reference[i + 1])) {
                 filteredPoints.push(points[i], points[i + 1]);
             }
         }
@@ -447,15 +438,10 @@ function canShowAllSymbolForCategory(
     return true;
 }
 
-
-function isPointNull(x: number, y: number) {
-    return isNaN(x) || isNaN(y);
-}
-
 function getLastIndexNotNull(points: ArrayLike<number>) {
     let len = points.length / 2;
     for (; len > 0; len--) {
-        if (!isPointNull(points[len * 2 - 2], points[len * 2 - 1])) {
+        if (!isPointIllegal(points[len * 2 - 2], points[len * 2 - 1])) {
             break;
         }
     }
@@ -477,7 +463,7 @@ function getIndexRange(points: ArrayLike<number>, xOrY: number, dim: 'x' | 'y')
     let nextIndex = -1;
     for (let i = 0; i < len; i++) {
         b = points[i * 2 + dimIdx];
-        if (isNaN(b) || isNaN(points[i * 2 + 1 - dimIdx])) {
+        if (isPointIllegal(b, points[i * 2 + 1 - dimIdx])) {
             continue;
         }
         if (i === 0) {
@@ -965,7 +951,7 @@ class LineView extends ChartView {
                 // Create a temporary symbol if it is not exists
                 const x = points[dataIndex * 2];
                 const y = points[dataIndex * 2 + 1];
-                if (isNaN(x) || isNaN(y)) {
+                if (isPointIllegal(x, y)) {
                     // Null data
                     return;
                 }

src/chart/line/helper.ts

@@ -132,3 +132,13 @@ export function getStackedOnPoint(
 
     return coordSys.dataToPoint(stackedData);
 }
+
+export function isPointIllegal(xOrY: number, yOrX: number) {
+    // NOTE:
+    //  - `NaN` point x/y may be generated by, e.g.,
+    //    original series data `NaN`, '-', `null`, `undefined`,
+    //    negative values in LogScale.
+    //  - `Infinite` point x/y may be generated by, e.g.,
+    //    original series data `Infinite`, `0` in LogScale.
+    return !isFinite(xOrY) || !isFinite(yOrX);
+}

src/chart/line/poly.ts

@@ -23,14 +23,11 @@ import Path, { PathProps } from 'zrender/src/graphic/Path';
 import PathProxy from 'zrender/src/core/PathProxy';
 import { cubicRootAt, cubicAt } from 'zrender/src/core/curve';
 import tokens from '../../visual/tokens';
+import { isPointIllegal } from './helper';
 
 const mathMin = Math.min;
 const mathMax = Math.max;
 
-function isPointNull(x: number, y: number) {
-    return isNaN(x) || isNaN(y);
-}
-
 /**
  * Draw smoothed line in non-monotone, in may cause undesired curve in extreme
  * situations. This should be used when points are non-monotone neither in x or
@@ -63,7 +60,7 @@ function drawSegment(
         if (idx >= allLen || idx < 0) {
             break;
         }
-        if (isPointNull(x, y)) {
+        if (isPointIllegal(x, y)) {
             if (connectNulls) {
                 idx += dir;
                 continue;
@@ -106,7 +103,7 @@ function drawSegment(
                 let tmpK = k + 1;
                 if (connectNulls) {
                     // Find next point not null
-                    while (isPointNull(nextX, nextY) && tmpK < segLen) {
+                    while (isPointIllegal(nextX, nextY) && tmpK < segLen) {
                         tmpK++;
                         nextIdx += dir;
                         nextX = points[nextIdx * 2];
@@ -120,7 +117,7 @@ function drawSegment(
                 let nextCpx0;
                 let nextCpy0;
                 // Is last point
-                if (tmpK >= segLen || isPointNull(nextX, nextY)) {
+                if (tmpK >= segLen || isPointIllegal(nextX, nextY)) {
                     cpx1 = x;
                     cpy1 = y;
                 }
@@ -256,12 +253,12 @@ export class ECPolyline extends Path<ECPolylineProps> {
         if (shape.connectNulls) {
             // Must remove first and last null values avoid draw error in polygon
             for (; len > 0; len--) {
-                if (!isPointNull(points[len * 2 - 2], points[len * 2 - 1])) {
+                if (!isPointIllegal(points[len * 2 - 2], points[len * 2 - 1])) {
                     break;
                 }
             }
             for (; i < len; i++) {
-                if (!isPointNull(points[i * 2], points[i * 2 + 1])) {
+                if (!isPointIllegal(points[i * 2], points[i * 2 + 1])) {
                     break;
                 }
             }
@@ -380,12 +377,12 @@ export class ECPolygon extends Path {
         if (shape.connectNulls) {
             // Must remove first and last null values avoid draw error in polygon
             for (; len > 0; len--) {
-                if (!isPointNull(points[len * 2 - 2], points[len * 2 - 1])) {
+                if (!isPointIllegal(points[len * 2 - 2], points[len * 2 - 1])) {
                     break;
                 }
             }
             for (; i < len; i++) {
-                if (!isPointNull(points[i * 2], points[i * 2 + 1])) {
+                if (!isPointIllegal(points[i * 2], points[i * 2 + 1])) {
                     break;
                 }
             }

src/coord/axisAlignTicks.ts

@@ -159,8 +159,8 @@ export function alignScaleTicks(
     const targetRawPowExtent = targetRawExtent;
     if (isTargetLogScale) {
         targetRawExtent = [
-            logScaleLogTick(targetRawExtent[0], targetLogScaleBase, false),
-            logScaleLogTick(targetRawExtent[1], targetLogScaleBase, false)
+            logScaleLogTick(targetRawExtent[0], targetLogScaleBase),
+            logScaleLogTick(targetRawExtent[1], targetLogScaleBase)
         ];
     }
     const targetExtent = intervalScaleEnsureValidExtent(targetRawExtent, targetMinMaxFixed);

src/coord/axisHelper.ts

@@ -46,8 +46,8 @@ import {
 import CartesianAxisModel from './cartesian/AxisModel';
 import SeriesData from '../data/SeriesData';
 import { getStackedDimension } from '../data/helper/dataStackHelper';
-import { Dictionary, DimensionName, NullUndefined, ScaleTick } from '../util/types';
-import { ensureScaleRawExtentInfo, ScaleRawExtentResult } from './scaleRawExtentInfo';
+import { Dictionary, DimensionName, ScaleTick } from '../util/types';
+import { clampForLogScale, ensureScaleRawExtentInfo, ScaleRawExtentResult } from './scaleRawExtentInfo';
 import { parseTimeAxisLabelFormatter } from '../util/time';
 import { getScaleBreakHelper } from '../scale/break';
 import { error } from '../util/log';
@@ -104,6 +104,11 @@ export function adoptScaleExtentOptionAndPrepare(
         }
     }
 
+    if (isLogScale(scale)) {
+        min = clampForLogScale(min);
+        max = clampForLogScale(max);
+    }
+
     rawExtentResult.min = min;
     rawExtentResult.max = max;
 
@@ -306,12 +311,6 @@ export function getDataDimensionsOnAxis(data: SeriesData, axisDim: string): Dime
     return zrUtil.keys(dataDimMap);
 }
 
-export function unionExtent(dataExtent: number[], val: number | NullUndefined): void {
-    // Considered that number could be NaN and should not write into the extent.
-    val < dataExtent[0] && (dataExtent[0] = val);
-    val > dataExtent[1] && (dataExtent[1] = val);
-}
-
 export function isNameLocationCenter(nameLocation: AxisBaseOptionCommon['nameLocation']) {
     return nameLocation === 'middle' || nameLocation === 'center';
 }

src/coord/cartesian/defaultAxisExtentFromData.ts

@@ -23,15 +23,15 @@ import SeriesModel from '../../model/Series';
 import {
     isCartesian2DDeclaredSeries, findAxisModels, isCartesian2DInjectedAsDataCoordSys
 } from './cartesianAxisHelper';
-import { getDataDimensionsOnAxis, unionExtent } from '../axisHelper';
+import { getDataDimensionsOnAxis } from '../axisHelper';
 import { AxisBaseModel } from '../AxisBaseModel';
 import type Axis from '../Axis';
 import GlobalModel from '../../model/Global';
 import { Dictionary } from '../../util/types';
 import {
     AXIS_EXTENT_INFO_BUILD_FROM_DATA_ZOOM, ensureScaleRawExtentInfo, ScaleRawExtentInfo, ScaleRawExtentResult
 } from '../scaleRawExtentInfo';
-import { initExtentForUnion } from '../../util/model';
+import { initExtentForUnion, unionExtent } from '../../util/model';
 
 /**
  * @obsolete

src/coord/scaleRawExtentInfo.ts

@@ -28,8 +28,8 @@ import { DimensionIndex, DimensionName, NullUndefined, ScaleDataValue } from '..
 import { isIntervalScale, isLogScale, isOrdinalScale, isTimeScale } from '../scale/helper';
 import type Axis from './Axis';
 import type SeriesModel from '../model/Series';
-import { makeInner, initExtentForUnion } from '../util/model';
-import { getDataDimensionsOnAxis, unionExtent } from './axisHelper';
+import { makeInner, initExtentForUnion, unionExtent } from '../util/model';
+import { getDataDimensionsOnAxis } from './axisHelper';
 import {
     getCoordForCoordSysUsageKindBox
 } from '../core/CoordinateSystem';
@@ -83,7 +83,7 @@ export interface ScaleRawExtentResult {
 export class ScaleRawExtentInfo {
 
     private _needCrossZero: ValueAxisBaseOption['scale'];
-    private _isOrdinal: boolean;
+    private _scale: Scale;
     private _axisDataLen: number;
     private _boundaryGapInner: number[];
 
@@ -118,26 +118,14 @@ export class ScaleRawExtentInfo {
         // Typically: data extent from all series on this axis.
         dataExtent: number[]
     ) {
-        this._prepareParams(scale, model, dataExtent);
-    }
+        this._scale = scale;
 
-    /**
-     * Parameters depending on outside (like model, user callback)
-     * are prepared and fixed here.
-     */
-    private _prepareParams(
-        scale: Scale,
-        model: AxisBaseModel,
-        // Usually: data extent from all series on this axis.
-        dataExtent: number[]
-    ) {
         if (dataExtent[1] < dataExtent[0]) {
             dataExtent = [NaN, NaN];
         }
         this._dataMin = dataExtent[0];
         this._dataMax = dataExtent[1];
 
-        const isOrdinal = this._isOrdinal = isOrdinalScale(scale);
         this._needCrossZero = isIntervalScale(scale) && model.getNeedCrossZero && model.getNeedCrossZero();
 
         if (isIntervalScale(scale) || isLogScale(scale) || isTimeScale(scale)) {
@@ -181,7 +169,7 @@ export class ScaleRawExtentInfo {
             this._modelMaxNum = parseAxisModelMinMax(scale, modelMaxRaw);
         }
 
-        if (isOrdinal) {
+        if (isOrdinalScale(scale)) {
             // FIXME: there is a flaw here: if there is no "block" data processor like `dataZoom`,
             // and progressive rendering is using, here the category result might just only contain
             // the processed chunk rather than the entire result.
@@ -227,7 +215,8 @@ export class ScaleRawExtentInfo {
         //      be the result that originalExtent enlarged by boundaryGap.
         // (3) If no data, it should be ensured that `scale.setBlank` is set.
 
-        const isOrdinal = this._isOrdinal;
+        const scale = this._scale;
+        const isOrdinal = isOrdinalScale(scale);
         let dataMin = this._dataMin;
         let dataMax = this._dataMax;
 
@@ -313,6 +302,11 @@ export class ScaleRawExtentInfo {
             maxFixed = maxDetermined = true;
         }
 
+        if (isLogScale(scale)) {
+            min = clampForLogScale(min);
+            max = clampForLogScale(max);
+        }
+
         // Ensure min/max be finite number or NaN here. (not to be null/undefined)
         // `NaN` means min/max axis is blank.
         return {
@@ -338,6 +332,9 @@ export class ScaleRawExtentInfo {
         if (__DEV__) {
             assert(this[attr] == null);
         }
+        if (isLogScale(this._scale)) {
+            val = clampForLogScale(val);
+        }
         this[attr] = val;
     }
 }
@@ -457,8 +454,9 @@ export function axisExtentInfoFinalBuild(
             // NOTE: This data may have been filtered by dataZoom on orthogonal axes.
             const data = seriesModel.getData();
             if (data) {
+                const filter = isLogScale(scale) ? {g: 0} : null;
                 each(getDataDimensionsOnAxis(data, axis.dim), function (dim) {
-                    const seriesExtent = data.getApproximateExtent(dim);
+                    const seriesExtent = data.getApproximateExtent(dim, filter);
                     unionExtent(extent, seriesExtent[0]);
                     unionExtent(extent, seriesExtent[1]);
                 });
@@ -503,3 +501,9 @@ function injectScaleRawExtentInfo(
     // @ts-ignore
     scaleRawExtentInfo.from = from;
 }
+
+export function clampForLogScale(val: number) {
+    // Avoid `NaN` for log scale.
+    // See also `DataStore#getDataExtent`.
+    return val < 0 ? 0 : val;
+}