Ovilia commented on a change in pull request #12418:
URL: 
https://github.com/apache/incubator-echarts/pull/12418#discussion_r431009173



##########
File path: test/bar-polar-stack.html
##########
@@ -91,5 +104,205 @@
                 window.onresize = chart.resize;
             });
         </script>
+        <script>
+            require([
+                'echarts'
+            ], function (echarts) {
+
+                var chart = 
echarts.init(document.getElementById('polar-negative'), null, {});
+
+                var data = [];
+
+                chart.setOption({
+                    angleAxis: {
+                    type: 'category',
+                    data: ['S1', 'S2', 'S3']
+                },
+                radiusAxis: {
+                    min:-1,
+                    max: 3
+                },
+                polar: {

Review comment:
       Try adding `radius: [50, 200]` to all `polar` in the test cases to find 
potential bugs.

##########
File path: src/layout/barPolar.js
##########
@@ -95,6 +96,7 @@ function barLayoutPolar(seriesType, ecModel, api) {
             // stackResultDimension directly.
             // Only ordinal axis can be stacked.
             if (stacked) {
+

Review comment:
       You may remove this empty line.

##########
File path: src/chart/bar/BarView.js
##########
@@ -336,8 +336,14 @@ var clip = {
         return clipped;
     },
 
-    polar: function (coordSysClipArea) {
-        return false;
+    polar: function (coordSysClipArea, layout) {
+        var clipped = (layout.r - coordSysClipArea.r > 0 && layout.r0 - 
coordSysClipArea.r > 0)
+        var r = mathMin(layout.r, coordSysClipArea.r);
+        var r0 = mathMin(layout.r0, coordSysClipArea.r);
+
+        layout.r = r;
+        layout.r0 = r0;
+        return clipped;

Review comment:
       When adding `radius: [50, 200]` to the last test case, you may find the 
clipping algorithm is not correct.
   
   According to the clipping algorithm of cartesian2d, I think this may be 
changed into:
   
   ```js
   var signR = layout.r0 <= layout.r ? 1 : -1;
   // Make sure r is larger than r0
   if (signR < 0) {
       var r = layout.r;
       layout.r = layout.r0;
       layout.r0 = r;
   }
   
   var r = mathMin(layout.r, coordSysClipArea.r);
   var r0 = mathMax(layout.r0, coordSysClipArea.r0);
   
   layout.r = r;
   layout.r0 = r0;
   
   var clipped = r - r0 < 0;
   
   // Reverse back
   if (signR < 0) {
       var r = layout.r;
       layout.r = layout.r0;
       layout.r0 = r;
   }
   
   return clipped;
   ```




----------------------------------------------------------------
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

Reply via email to