You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2021/07/27 05:18:42 UTC
[GitHub] [echarts] plainheart opened a new pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
plainheart opened a new pull request #15416:
URL: https://github.com/apache/echarts/pull/15416
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [x] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
Resolves #15407. When `visualMap` is enabled, the line color is unexpected.
### Fixed issues
- #15407
## Details
### Before: What was the problem?
<!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
<img src="https://user-images.githubusercontent.com/26999792/127099222-6dd716cd-7115-4970-b292-ff0c8b5a7ba8.png" width="400">
### After: How is it fixed in this PR?
<!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
Revert the change brought by #14602 in `LineView.ts` and normalize the infinite value
https://github.com/apache/echarts/blob/cf7c4efc5c33961e1e932c450e2c16073feaa8fc/src/chart/line/LineView.ts#L246
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
<img src="https://user-images.githubusercontent.com/26999792/127099265-d893377b-c50b-4bda-9c2d-ada2601b1b83.png" width="400">
## Misc
<!-- ADD RELATED ISSUE ID WHEN APPLICABLE -->
- [ ] The API has been changed (apache/echarts-doc#xxx).
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
Please refer to `test/linear-gradient.html`.
## Others
### Merging options
- [ ] Please squash the commits into a single one when merge.
### Other information
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677239468
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
Take the first log axis case, it should be because `dataToCoord` will probably return an infinite value.
After invoking https://github.com/apache/echarts/blob/4db2f1e7c1eb50cf9305370aa85381e2840623ef/src/coord/Axis.ts#L125
the data may become `Infinity`.
We expect `clamp` is `true` but before this, `clamp` was calculated as `false`.
So it may be better to check again and clamp after the first coordinate conversion
or modify the `dataToCoord` function (I'm afraid we can't)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#issuecomment-887363045
Congratulations! Your PR has been merged. Thanks for your contribution! 👍
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677147217
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
We knew Infinite coord is calculated from an infinite value. So can we simplify the code to only clamp the infinite value.
```ts
const value = stop.value;
const coord = axis.toGlobalCoord(axis.dataToCoord(
value, !(isFinite(value) || isNaN(value))
));
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677282656
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
Understand, thanks for the explaination
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677225221
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
Take the first log axis case, it should be because `dataToCoord` will probably return an infinite value.
After invoking https://github.com/apache/echarts/blob/4db2f1e7c1eb50cf9305370aa85381e2840623ef/src/coord/Axis.ts#L125
the data may become `Infinity` and we expect `clamp` is `true` but before this, the `clamp` was calculated as `false`.
So it may be better to check again and clamp after the first coordinate conversion.
Now the logic should be like this,
```ts
// consider NaN to be 0
const value = stop.value || 0;
let coord = axis.toGlobalCoord(axis.dataToCoord(value, !isFinite(value)));
// if coord is still infinite
isNaN(coord) || isFinite(coord) || (coord = axis.toGlobalCoord(axis.dataToCoord(coord, true)));
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677225221
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
Take the first log axis case, it should be because `dataToCoord` will probably return an infinite value.
After invoking https://github.com/apache/echarts/blob/4db2f1e7c1eb50cf9305370aa85381e2840623ef/src/coord/Axis.ts#L125
the data may become `Infinity` and we expect `clamp` is `true` but before this, the `clamp` was calculated as `false`.
So it may be better to check again and clamp after the first coordinate conversion.
Now the logic should be like this,
```ts
const value = stop.value || 0;
let coord = axis.toGlobalCoord(axis.dataToCoord(value, !isFinite(value)));
// if coord is still infinite
isNaN(coord) || isFinite(coord) || (coord = axis.toGlobalCoord(axis.dataToCoord(coord, true)));
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677205783
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
This is simple and can solve the visual map bug but it can't work for the first case that uses the log axis unless we remove the `isNaN` condition. But the third (current) case won't work.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677205783
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
This is simple and can solve the visual map bug but it can't work for the first case that uses the log axis.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#issuecomment-887217132
Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/echarts/wiki/How-to-make-a-pull-request).
The pull request is marked to be `PR: author is committer` because you are a committer of this project.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] pissang merged pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #15416:
URL: https://github.com/apache/echarts/pull/15416
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a change in pull request #15416: fix(line): normalize the infinite value when calculating the visual gradient.
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #15416:
URL: https://github.com/apache/echarts/pull/15416#discussion_r677225221
##########
File path: src/chart/line/LineView.ts
##########
@@ -233,17 +238,17 @@ function getVisualGradient(
// LinearGradient to render `outerColors`.
const axis = coordSys.getAxis(coordDim);
+ const axisScaleExtent = axis.scale.getExtent();
- interface ColorStop {
- offset: number
- coord?: number
- color: ColorString
- }
// dataToCoord mapping may not be linear, but must be monotonic.
const colorStops: ColorStop[] = zrUtil.map(visualMeta.stops, function (stop) {
+ let coord = axis.toGlobalCoord(axis.dataToCoord(stop.value));
+ // normalize the infinite value
+ isNaN(coord) || isFinite(coord)
Review comment:
Take the first log axis case, it should be caused by `dataToCoord` will probably return an infinite value.
After invoking https://github.com/apache/echarts/blob/4db2f1e7c1eb50cf9305370aa85381e2840623ef/src/coord/Axis.ts#L125
the data may become `Infinity` and we expect `clamp` is `true` but before this, the `clamp` was calculated as `false`.
So it may be better to check again and clamp after the first coordinate conversion.
Now the logic should be like this,
```ts
const value = stop.value || 0;
let coord = axis.toGlobalCoord(axis.dataToCoord(value, !isFinite(value)));
// if coord is still infinite
isNaN(coord) || isFinite(coord) || (coord = axis.toGlobalCoord(axis.dataToCoord(coord, true)));
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org