[GitHub] [incubator-echarts] pissang commented on a change in pull request #13210: Feat: multiple value axis alignment
pissang commented on a change in pull request #13210: URL: https://github.com/apache/incubator-echarts/pull/13210#discussion_r492453250 ## File path: src/component/axis/AxisBuilder.ts ## @@ -372,7 +372,7 @@ const builders: Record<'axisLine' | 'axisTickLabel' | 'axisName', AxisElementsBu const textStyleModel = axisModel.getModel('nameTextStyle'); const gap = axisModel.get('nameGap') || 0; -const extent = axisModel.axis.getExtent(); +const extent = axisModel.axis.getGridExtent(); const gapSignal = extent[0] > extent[1] ? -1 : 1; Review comment: I thought the main reason two axes don't align is that they don't have the same split numbers. They should be aligned if the split numbers are same. ## File path: src/component/axis/AxisBuilder.ts ## @@ -372,7 +372,7 @@ const builders: Record<'axisLine' | 'axisTickLabel' | 'axisName', AxisElementsBu const textStyleModel = axisModel.getModel('nameTextStyle'); const gap = axisModel.get('nameGap') || 0; -const extent = axisModel.axis.getExtent(); +const extent = axisModel.axis.getGridExtent(); const gapSignal = extent[0] > extent[1] ? -1 : 1; Review comment: I thought the main reason two axes don't align is that they don't have the same split numbers. They should be aligned if the split numbers are same. I see, axis extent is different from grid extent only when `min`, `max` is fixed? ## File path: src/component/axis/AxisBuilder.ts ## @@ -372,7 +372,7 @@ const builders: Record<'axisLine' | 'axisTickLabel' | 'axisName', AxisElementsBu const textStyleModel = axisModel.getModel('nameTextStyle'); const gap = axisModel.get('nameGap') || 0; -const extent = axisModel.axis.getExtent(); +const extent = axisModel.axis.getGridExtent(); const gapSignal = extent[0] > extent[1] ? -1 : 1; Review comment: I thought the main reason two axes don't align is that they don't have the same split numbers. They should be aligned if the split numbers are same. I see, so axis extent is different from grid extent only when `min`, `max` is fixed? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@echarts.apache.org For additional commands, e-mail: commits-h...@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request #13210: Feat: multiple value axis alignment
pissang commented on a change in pull request #13210: URL: https://github.com/apache/incubator-echarts/pull/13210#discussion_r492453250 ## File path: src/component/axis/AxisBuilder.ts ## @@ -372,7 +372,7 @@ const builders: Record<'axisLine' | 'axisTickLabel' | 'axisName', AxisElementsBu const textStyleModel = axisModel.getModel('nameTextStyle'); const gap = axisModel.get('nameGap') || 0; -const extent = axisModel.axis.getExtent(); +const extent = axisModel.axis.getGridExtent(); const gapSignal = extent[0] > extent[1] ? -1 : 1; Review comment: I thought the main reason two axes don't align is that they don't have the same split numbers. They should be aligned if the split numbers are same. ## File path: src/component/axis/AxisBuilder.ts ## @@ -372,7 +372,7 @@ const builders: Record<'axisLine' | 'axisTickLabel' | 'axisName', AxisElementsBu const textStyleModel = axisModel.getModel('nameTextStyle'); const gap = axisModel.get('nameGap') || 0; -const extent = axisModel.axis.getExtent(); +const extent = axisModel.axis.getGridExtent(); const gapSignal = extent[0] > extent[1] ? -1 : 1; Review comment: I thought the main reason two axes don't align is that they don't have the same split numbers. They should be aligned if the split numbers are same. I see, axis extent is different from grid extent only when `min`, `max` is fixed? ## File path: src/component/axis/AxisBuilder.ts ## @@ -372,7 +372,7 @@ const builders: Record<'axisLine' | 'axisTickLabel' | 'axisName', AxisElementsBu const textStyleModel = axisModel.getModel('nameTextStyle'); const gap = axisModel.get('nameGap') || 0; -const extent = axisModel.axis.getExtent(); +const extent = axisModel.axis.getGridExtent(); const gapSignal = extent[0] > extent[1] ? -1 : 1; Review comment: I thought the main reason two axes don't align is that they don't have the same split numbers. They should be aligned if the split numbers are same. I see, so axis extent is different from grid extent only when `min`, `max` is fixed? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@echarts.apache.org For additional commands, e-mail: commits-h...@echarts.apache.org
[GitHub] [incubator-echarts] pissang commented on a change in pull request #13210: Feat: multiple value axis alignment
pissang commented on a change in pull request #13210: URL: https://github.com/apache/incubator-echarts/pull/13210#discussion_r489138320 ## File path: src/coord/Axis.ts ## @@ -106,6 +107,14 @@ class Axis { ); } +setGridExtent(start: number, end: number) { +this._gridExtent = [start, end]; +} + +getGridExtent(): [number, number] { +return this._gridExtent.slice() as [number, number]; +} + Review comment: As described above. The concept `grid` should not be leaked to the very general Axis module. ## File path: src/component/axis/AxisBuilder.ts ## @@ -372,7 +372,7 @@ const builders: Record<'axisLine' | 'axisTickLabel' | 'axisName', AxisElementsBu const textStyleModel = axisModel.getModel('nameTextStyle'); const gap = axisModel.get('nameGap') || 0; -const extent = axisModel.axis.getExtent(); +const extent = axisModel.axis.getGridExtent(); const gapSignal = extent[0] > extent[1] ? -1 : 1; Review comment: AxisBuilder will also be used on polar, parallel. Not only on grid. Also, I'm still not sure about the difference between the grid extent and axis extent. When will these two extents has different values? ## File path: src/coord/cartesian/Grid.ts ## @@ -85,18 +86,68 @@ class Grid implements CoordinateSystemMaster { return this._rect; } +calAxisNiceSplitNumber(axes: Axis2D[]) { +const splitNumber: number[] = []; +const niceAxisExtents: number[] = []; +each(axes, axis => { +niceScaleExtent(axis.scale, axis.model); +if (axis.type !== 'value') { +return; +} +const extent = axis.scale.getExtent(); +const interval = (axis.scale as IntervalScale).getInterval(); +splitNumber.push( +Math.floor((extent[1] - extent[0]) / interval) +); +const axisExtent = axis.getExtent(); +niceAxisExtents.push((axisExtent[1] - axisExtent[0]) * interval / extent[1]); +}); + +return splitNumber; +} + +resetAxisExtent(axes: Axis2D[], splitNumber: number[], maxSplitNumber: number): number[] { +const finalSplitNumber: number[] = []; +each(axes, function (axis, index) { +if (axis.type !== 'value') { +return; +} + +if (!(maxSplitNumber - splitNumber[index])) { +finalSplitNumber.push(maxSplitNumber); +return; +}; + +const extent = axis.scale.getExtent(); +const interval = (axis.scale as IntervalScale).getInterval(); +if (((extent[1] - extent[0]) % interval) === 0) { +extent[1] = extent[1] + (maxSplitNumber - splitNumber[index]) * interval; +niceScaleExtent(axis.scale, axis.model, extent, maxSplitNumber); +} +else { +(axis.scale as IntervalScale).niceTicks( +maxSplitNumber + 1, null, null, +((extent[1] - extent[0]) - ((extent[1] - extent[0]) % maxSplitNumber)) / maxSplitNumber +); Review comment: `extent[1] - extent[0]` should be rounded here to avoid rounding error. ```js numberUtil.round(extent[1] - extent[0]); ``` The result interval should also be rounded. ## File path: src/coord/cartesian/Grid.ts ## @@ -85,18 +86,68 @@ class Grid implements CoordinateSystemMaster { return this._rect; } +calAxisNiceSplitNumber(axes: Axis2D[]) { +const splitNumber: number[] = []; +const niceAxisExtents: number[] = []; +each(axes, axis => { +niceScaleExtent(axis.scale, axis.model); +if (axis.type !== 'value') { +return; +} +const extent = axis.scale.getExtent(); +const interval = (axis.scale as IntervalScale).getInterval(); +splitNumber.push( +Math.floor((extent[1] - extent[0]) / interval) +); +const axisExtent = axis.getExtent(); +niceAxisExtents.push((axisExtent[1] - axisExtent[0]) * interval / extent[1]); +}); + +return splitNumber; +} + +resetAxisExtent(axes: Axis2D[], splitNumber: number[], maxSplitNumber: number): number[] { +const finalSplitNumber: number[] = []; +each(axes, function (axis, index) { +if (axis.type !== 'value') { +return; +} + +if (!(maxSplitNumber - splitNumber[index])) { +finalSplitNumber.push(maxSplitNumber); +return; +}; + +const extent = axis.scale.getExtent(); +const interval = (axis.scale as IntervalScale).getInterval(); +if (((extent[1] - extent[0]) % interval) === 0) { Review comment: