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 2020/06/10 15:21:25 UTC

[GitHub] [incubator-echarts] yanheSu opened a new pull request #12793: fix(pictorialBar): fix linewidth NaN, when data is zero

yanheSu opened a new pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793


   <!-- 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?
   
   <!-- USE ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
   fix zero data in pictorialbar can't render correct, when the borderWidth be setted.
   
   
   ### Fixed issues
   
   <!--
   - #xxxx: ...
   -->
   - #12789
   
   ## Details
   
   ### Before: What was the problem?
   
   <!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
   I'm not exactly sure what 'pxSign' really means, but it looks like a direction.
   when data is zero, then boundingLength is zero, `pxSign` will be zero, and then symbolScale will be zero too, and if `borderWidth` setted and not zero,  the `valueLineWidth` will be NaN.
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   <img width="675" alt="图片" src="https://user-images.githubusercontent.com/47936701/84285156-b1be9c00-ab6f-11ea-826a-a3a75fb51207.png">
   <img width="988" alt="图片" src="https://user-images.githubusercontent.com/47936701/84285436-10841580-ab70-11ea-9745-65367eda9eae.png">
   
   
   ### After: How is it fixed in this PR?
   
   <!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
   `pxSign` will be part of the denominator somewhere else, so it shouldn't be 0
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   <img width="762" alt="图片" src="https://user-images.githubusercontent.com/47936701/84285508-21cd2200-ab70-11ea-9b4c-9653b876fd7c.png">
   
   
   ## Usage
   
   ### Are there any API changes?
   
   - [ ] The API has been changed.
   
   <!-- LIST THE API CHANGES HERE -->
   
   
   
   ### Related test cases or examples to use the new APIs
   
   NA.
   
   
   
   ## 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.

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] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438562589



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       @yanheSu Now I think you are right, maybe the sign of pixel shouldn't be `0` but negative or positive. And as you said, if `symbolSize` is specified, the chart also should draw it even though its data is `0`. But for this example:
   https://echarts.apache.org/examples/zh/editor.html?c=doc-example/pictorialBar-position
   If the data of symbol is `0`, should the people be drawn? Unlike the example in #12789, it seems to be strange here.
   
   **Before**
   ![image](https://user-images.githubusercontent.com/26999792/84351477-0a347e80-abee-11ea-91d8-751cfe6de5a2.png)
   
   **Now**
   ![image](https://user-images.githubusercontent.com/26999792/84351445-fa1c9f00-abed-11ea-944f-b1533c277fae.png)
   




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438562589



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       @yanheSu Now I think you are right, maybe the sign of pixel shouldn't be `0` but negative or positive. And as you said, if `symbolSize` is specified, the chart also should draw it even though its data is `0`.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438506022



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       I think no change to be done here. It is just used to indicate the sign of pixel.
   
   https://github.com/apache/incubator-echarts/blob/3ab134179298a780659be9009bce2c0f7ec6bec9/src/chart/bar/PictorialBarView.js#L296
   
   The reason of the bug is beacuse `pathForLineWidth.getLineScale()` may be zero, as we all know, divisor can not be `0`, which is meaningless. So there should be a condition to judge whether `lineScale` is `0` or not before
   ```js
   valueLineWidth /= pathForLineWidth.getLineScale();
   valueLineWidth *= symbolScale[opt.valueDim.index];
   ```
   if `0`, don't perform the following operations and set `valueLineWidth` to `0` directly.
   Maybe changes should be like below:
   ```js
   var lineScale = pathForLineWidth.getLineScale();
   if (lineScale) {
       valueLineWidth /= lineScale;
       valueLineWidth *= symbolScale[opt.valueDim.index];
   }
   else {
       valueLineWidth = 0;
   }
   ```




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #12793: fix(pictorialBar): fix linewidth NaN, when data is zero

Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#issuecomment-642080015


   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/incubator-echarts/wiki/How-to-make-a-pull-request).


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438562589



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       @yanheSu Now I think you are right, maybe the sign of pixel shouldn't be `0` but negative or positive. And as you said, if `symbolSize` is specified, the chart also should draw it even though its data is `0`.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438506022



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       I think no change to be done here. It is just used to indicate the sign of pixel.
   
   https://github.com/apache/incubator-echarts/blob/3ab134179298a780659be9009bce2c0f7ec6bec9/src/chart/bar/PictorialBarView.js#L296-L297
   
   The reason of the bug is beacuse `pathForLineWidth.getLineScale()` may be zero, as we all know, divisor can not be `0`, which is meaningless. So there should be a condition to judge whether `lineScale` is `0` or not before the code lines below.
   ```js
   valueLineWidth /= pathForLineWidth.getLineScale();
   valueLineWidth *= symbolScale[opt.valueDim.index];
   ```
   if `0`, don't perform the following operations and set `valueLineWidth` to `0` directly.
   Maybe changes should be like below:
   ```js
   var lineScale = pathForLineWidth.getLineScale();
   if (lineScale) {
       valueLineWidth /= lineScale;
       valueLineWidth *= symbolScale[opt.valueDim.index];
   }
   else {
       valueLineWidth = 0;
   }
   ```




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438562589



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       @yanheSu Now I think you are right, maybe the sign of pixel shouldn't be `0` but negative or positive. And as you said, if `symbolSize` is specified, the chart also should draw it even though its data is `0`. But for this example:
   https://echarts.apache.org/examples/zh/editor.html?c=doc-example/pictorialBar-position
   If the data of symbol is `0`, should the people be drawn? Unlike the example in #12789, it seems to be strange here.
   
   **Before**
   ![image](https://user-images.githubusercontent.com/26999792/84351477-0a347e80-abee-11ea-91d8-751cfe6de5a2.png)
   We can know the data of the first bar is `0` through the image.
   
   **Now**
   ![image](https://user-images.githubusercontent.com/26999792/84351445-fa1c9f00-abed-11ea-944f-b1533c277fae.png)
   Because of different `symbolPosition`, we can not be very sure what the first bar indicates `0` or `20` or any other 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.

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] [incubator-echarts] 307590317 commented on pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
307590317 commented on pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#issuecomment-742409683


   When will this bug be fixed?Now is December 10th. It's been six months


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] linghaoSu commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
linghaoSu commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438545047



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       I originally planned to modify it like this, but if it is modified like this, the graph representing this data will disappear. When symbolSize is specified, should such a display be made?
   And as a pixel sign, why can the sign be 0?
   Based on the above doubts, I made the current modification. Under this modification, when the symbolSize is specified, even if the data is 0, the data will be displayed at the corresponding data position.
   However, I also think that a conditional selection statement should also be added here to prevent the situation where the dividend is 0.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] 307590317 edited a comment on pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
307590317 edited a comment on pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#issuecomment-742409683


   这个bug什么时候会被修复?现在是12月10号,距离6月10号已经过去6个月了
   When will this bug be fixed?Now is December 10th. It's been six months


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438562589



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       @yanheSu Now I think you are right, maybe the sign of pixel shouldn't be `0` but negative or positive. And as you said, if `symbolSize` is specified, the chart also should draw it even though its data is `0`.
   But for this example:
   https://gallery.echartsjs.com/editor.html?c=xJEKNxRWsu&v=1
   If the data of both bar and people symbol is `0`, should the people be drawn? Unlike the example in #12789, it seems to be strange here.
   
   **Before**
   ![image](https://user-images.githubusercontent.com/26999792/84352639-48cb3880-abf0-11ea-80c2-cce9b57ebecc.png)
   
   **Now**
   ![image](https://user-images.githubusercontent.com/26999792/84352343-bcb91100-abef-11ea-9d95-22e93f727b97.png)
   




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] yanheSu commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
yanheSu commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438546327



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       I originally planned to modify it like this, but if it is modified like this, the graph representing this data will disappear. When symbolSize is specified, should such a display be made?
   And as a pixel sign, why can the sign be 0?
   Based on the above doubts, I made the current modification. Under this modification, when the symbolSize is specified, even if the data is 0, the data will be displayed at the corresponding data position.
   However, I also think that a conditional selection statement should also be added here to prevent the situation where the dividend is 0.
   
   Sorry, I just used the wrong account, so delete the previous comment and re-send it.🌚




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org


[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438562589



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       @yanheSu Now I think you are right, maybe the sign of pixel shouldn't be `0` but negative or positive. And as you said, if `symbolSize` is specified, the chart also should draw it even though its data is `0`. But for this example:
   https://echarts.apache.org/examples/zh/editor.html?c=doc-example/pictorialBar-position
   If the data of symbol is `0`, should the people be drawn? Unlike the example in #12789, it seems to be strange here.
   
   **Before**
   ![image](https://user-images.githubusercontent.com/26999792/84351477-0a347e80-abee-11ea-91d8-751cfe6de5a2.png)
   We can know the data of first bar is `0` through the image.
   
   **Now**
   ![image](https://user-images.githubusercontent.com/26999792/84351445-fa1c9f00-abed-11ea-944f-b1533c277fae.png)
   Because of different `symbolPosition`, we can not be very sure what the first bar indicates `0` or `20` or any other 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.

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] [incubator-echarts] yanheSu commented on a change in pull request #12793: fix(pictorialBar): Fixed a problem with rendering incorrectly when data is 0 and `borderWidth` is set to a non-0 value

Posted by GitBox <gi...@apache.org>.
yanheSu commented on a change in pull request #12793:
URL: https://github.com/apache/incubator-echarts/pull/12793#discussion_r438725421



##########
File path: src/chart/bar/PictorialBarView.js
##########
@@ -227,7 +227,9 @@ function prepareBarLength(itemModel, symbolRepeat, layout, opt, output) {
         output.repeatCutLength = layout[valueDim.wh];
     }
 
-    output.pxSign = boundingLength > 0 ? 1 : boundingLength < 0 ? -1 : 0;
+    // if 'pxSign' means sign of pixel,  it can't be zero, or symboScale will be zero
+    // and when borderwidth be setted, the actual linewidth will be NaN
+    output.pxSign = boundingLength > 0 ? 1 : -1;

Review comment:
       @plainheart 
   From my personal point of view, I don’t think this is very strange. When `symboSize` is specified and `symbolRepeat` is not turned on, it is not a problem to draw a symbol of size `symbolSize` at the position of `symbolPosition`.
   0 value is just a special point, all `symbolPosition` positions coincide at one point.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org