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/11 02:35:53 UTC

[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

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