changeset 10052:e3061a3b2ddd jdk-9+138

8166055: PieChart: overlapped labels sometimes Reviewed-by: jgiles
author vadim
date Mon, 26 Sep 2016 01:09:51 +0300
parents 46fed21d2824
children a6654cc0b154 140a56440fa4
files modules/javafx.controls/src/main/java/javafx/scene/chart/PieChart.java modules/javafx.controls/src/test/java/test/javafx/scene/chart/PieChartTest.java
diffstat 2 files changed, 43 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/modules/javafx.controls/src/main/java/javafx/scene/chart/PieChart.java	Thu Sep 22 14:04:38 2016 -0700
+++ b/modules/javafx.controls/src/main/java/javafx/scene/chart/PieChart.java	Mon Sep 26 01:09:51 2016 +0300
@@ -637,18 +637,15 @@
                 index++;
             }
 
-             // Check for collision and resolve by hiding the label of the smaller pie slice
-            resolveCollision(fullPie);
-
             // update/draw pie slices
             double sAngle = getStartAngle();
             for (Data item = begin; item != null; item = item.next) {
-             Node node = item.getNode();
+                Node node = item.getNode();
                 Arc arc = null;
-                 if (node != null) {
+                if (node != null) {
                     if (node instanceof Region) {
                         Region arcRegion = (Region)node;
-                        if( arcRegion.getShape() == null) {
+                        if (arcRegion.getShape() == null) {
                             arc = new Arc();
                             arcRegion.setShape(arc);
                         } else {
@@ -674,6 +671,9 @@
             }
             // finally draw the text and line
             if (fullPie != null) {
+                // Check for collision and resolve by hiding the label of the smaller pie slice
+                resolveCollision(fullPie);
+
                 for (LabelLayoutInfo info : fullPie) {
                     if (info.text.isVisible()) drawLabelLinePath(info);
                 }
@@ -685,23 +685,21 @@
     // compare the size of the slices, and hide the label of the smaller slice.
     private void resolveCollision(ArrayList<LabelLayoutInfo> list) {
         int boxH = (begin != null) ? (int)begin.textNode.getLayoutBounds().getHeight() : 0;
-        int i; int j;
-        for (i = 0, j = 1; list != null && j < list.size(); j++ ) {
-            LabelLayoutInfo box1 = list.get(i);
-            LabelLayoutInfo box2 = list.get(j);
-            if ((box1.text.isVisible() && box2.text.isVisible()) &&
-                    (fuzzyGT(box2.textY, box1.textY) ? fuzzyLT((box2.textY - boxH - box1.textY), 2) :
-                     fuzzyLT((box1.textY - boxH - box2.textY), 2)) &&
-                    (fuzzyGT(box1.textX, box2.textX) ? fuzzyLT((box1.textX - box2.textX), box2.text.prefWidth(-1)) :
-                        fuzzyLT((box2.textX - box1.textX), box1.text.prefWidth(-1)))) {
-                if (fuzzyLT(box1.size, box2.size)) {
-                    box1.text.setVisible(false);
-                    i = j;
-                } else {
-                    box2.text.setVisible(false);
+        for (int i = 0; i < list.size(); i++ ) {
+            for (int j = i+1; j < list.size(); j++ ) {
+                LabelLayoutInfo box1 = list.get(i);
+                LabelLayoutInfo box2 = list.get(j);
+                if ((box1.text.isVisible() && box2.text.isVisible()) &&
+                        (fuzzyGT(box2.textY, box1.textY) ? fuzzyLT((box2.textY - boxH - box1.textY), 2) :
+                                fuzzyLT((box1.textY - boxH - box2.textY), 2)) &&
+                        (fuzzyGT(box1.textX, box2.textX) ? fuzzyLT((box1.textX - box2.textX), box2.text.prefWidth(-1)) :
+                                fuzzyLT((box2.textX - box1.textX), box1.text.prefWidth(-1)))) {
+                    if (fuzzyLT(box1.size, box2.size)) {
+                        box1.text.setVisible(false);
+                    } else {
+                        box2.text.setVisible(false);
+                    }
                 }
-            } else {
-                i = j;
             }
         }
     }
--- a/modules/javafx.controls/src/test/java/test/javafx/scene/chart/PieChartTest.java	Thu Sep 22 14:04:38 2016 -0700
+++ b/modules/javafx.controls/src/test/java/test/javafx/scene/chart/PieChartTest.java	Mon Sep 26 01:09:51 2016 +0300
@@ -26,13 +26,18 @@
 package test.javafx.scene.chart;
 
 import com.sun.javafx.charts.Legend;
+
+import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
+
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.scene.Node;
 import javafx.scene.chart.Chart;
 import javafx.scene.chart.ChartShim;
 import javafx.scene.chart.PieChart;
+import javafx.scene.text.Text;
 import org.junit.Test;
 import static org.junit.Assert.*;
 
@@ -160,4 +165,22 @@
         pc.setLegendVisible(true);
         assertEquals(4, ((Legend)ChartShim.getLegend(pc)).getItems().size());
     }
+
+    @Test
+    public void testLabelsCollision_8166055() {
+        data.addAll(
+                new PieChart.Data("AAAAA", 2),
+                new PieChart.Data("BBBBB", 1),
+                new PieChart.Data("CCCCC", 1000),
+                new PieChart.Data("BBBBB", 1),
+                new PieChart.Data("BBBBB", 1)
+        );
+        startApp();
+        List<Text> labels = ChartShim.getChartChildren(pc).stream()
+                .filter(n -> n.getStyleClass().contains("chart-pie-label"))
+                .map(n -> (Text) n)
+                .collect(Collectors.toList());
+        assertTrue(labels.stream().filter(n -> n.getText().equals("BBBBB")).noneMatch(n -> n.isVisible()));
+        assertTrue(labels.stream().filter(n -> !n.getText().equals("BBBBB")).allMatch(n -> n.isVisible()));
+    }
 }