8198830: BarChart: auto-range of CategoryAxis not working on dynamically setting data

Reviewed-by: angorya

Author: Markus Mack

modules/javafx.controls/src/main/java/javafx/scene/chart/BarChart.java

@@ -384,7 +384,7 @@ public BarChart(@NamedArg("xAxis") Axis<X> xAxis, @NamedArg("yAxis") Axis<Y> yAx
     @Override protected void layoutPlotChildren() {
         double catSpace = categoryAxis.getCategorySpacing();
         // calculate bar spacing
-        final double availableBarSpace = catSpace - (getCategoryGap() + getBarGap());
+        final double availableBarSpace = catSpace - getCategoryGap() + getBarGap();
         double barWidth = (availableBarSpace / getSeriesSize()) - getBarGap();
         final double barOffset = -((catSpace - getCategoryGap()) / 2);
         final double zeroPos = (valueAxis.getLowerBound() > 0) ?
@@ -423,9 +423,9 @@ public BarChart(@NamedArg("xAxis") Axis<X> xAxis, @NamedArg("yAxis") Axis<Y> yAx
                         bar.resizeRelocate( bottom, categoryPos + barOffset + (barWidth + getBarGap()) * index,
                                             top-bottom, barWidth);
                     }
-
-                    index++;
                 }
+
+                index++;
             }
             catIndex++;
         }

modules/javafx.controls/src/main/java/javafx/scene/chart/CategoryAxis.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -68,8 +68,17 @@ public final class CategoryAxis extends Axis<String> {
     // -------------- PRIVATE FIELDS -------------------------------------------
     private List<String> allDataCategories = new ArrayList<>();
     private boolean changeIsLocal = false;
-    /** This is the gap between one category and the next along this axis */
-    private final DoubleProperty firstCategoryPos = new SimpleDoubleProperty(this, "firstCategoryPos", 0);
+
+    /** This is the position of the first category along this axis */
+    private final DoubleProperty firstCategoryPos =
+        new SimpleDoubleProperty(this, "firstCategoryPos", 0) {
+            @Override
+            protected void invalidated() {
+                requestAxisLayout();
+                measureInvalid = true;
+            }
+        };
+
     private Object currentAnimationID;
     private final ChartLayoutAnimator animator = new ChartLayoutAnimator(this);
     private ListChangeListener<String> itemsListener = c -> {
@@ -239,7 +248,15 @@ public final ObservableList<String> getCategories() {
     }
 
     /** This is the gap between one category and the next along this axis */
-    private final ReadOnlyDoubleWrapper categorySpacing = new ReadOnlyDoubleWrapper(this, "categorySpacing", 1);
+    private final ReadOnlyDoubleWrapper categorySpacing =
+        new ReadOnlyDoubleWrapper(this, "categorySpacing", 1) {
+            @Override
+            protected void invalidated() {
+                requestAxisLayout();
+                measureInvalid = true;
+            }
+        };
+
     public final double getCategorySpacing() {
         return categorySpacing.get();
     }

modules/javafx.controls/src/test/java/test/javafx/scene/chart/BarChartTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -29,6 +29,10 @@
 import java.util.List;
 
 import javafx.collections.FXCollections;
+import javafx.scene.Scene;
+import javafx.scene.shape.MoveTo;
+import javafx.scene.shape.Path;
+import javafx.stage.Stage;
 import org.junit.Test;
 import static org.junit.Assert.assertEquals;
 import javafx.collections.*;
@@ -271,4 +275,94 @@ public void testAddingMultipleSeriesWithDuplicateCategories() {
         assertEquals("5", categories.get(3));
         assertEquals("4", categories.get(4));
     }
+
+    @Test
+    public void testTickMarksMatchBarPositionsAfterAnimation() {
+        startApp();
+        CategoryAxis xAxis = new CategoryAxis();
+        NumberAxis yAxis = new NumberAxis();
+        BarChart<String, Number> chart = new BarChart<>(xAxis, yAxis);
+        Series<String, Number> series = new Series<>();
+        chart.getData().add(series);
+        chart.setAnimated(true);
+        getTestScene().setRoot(chart);
+
+        // add some categories, starting axis animation
+        series.getData().add(new XYChart.Data<>("1", 1));
+        series.getData().add(new XYChart.Data<>("2", 2));
+        series.getData().add(new XYChart.Data<>("3", 3));
+        pulse();
+        // forward time until after animation is finished
+        toolkit.setAnimationTime(1000);
+
+        List<Node> bars = series.getData().stream().map(XYChart.Data::getNode).toList();
+
+        List<Double> barCenterXValues = series.getData().stream()
+                .map(XYChart.Data::getNode)
+                .map(bar -> bar.getLayoutX() + bar.getLayoutBounds().getCenterX())
+                .toList();
+
+        List<Double> tickXValues = xAxis.getChildrenUnmodifiable().stream()
+                .filter(obj -> obj instanceof Path && obj.getStyleClass().contains("axis-tick-mark"))
+                .flatMap(obj -> ((Path) obj).getElements().stream())
+                .filter(path -> path instanceof MoveTo)
+                .map(moveTo -> ((MoveTo) moveTo).getX())
+                .toList();
+
+        double delta = 0.001;
+        assertEquals(barCenterXValues.size(), tickXValues.size());
+        for (int i = 0; i < barCenterXValues.size(); i++) {
+            assertEquals(barCenterXValues.get(i), tickXValues.get(i), delta);
+        }
+    }
+
+    @Test
+    public void testBarPositionsWithMultipleIncompleteSeries() {
+        startApp();
+        CategoryAxis xAxis = new CategoryAxis();
+        NumberAxis yAxis = new NumberAxis();
+        BarChart<String, Number> chart = new BarChart<>(xAxis, yAxis);
+        chart.setAnimated(false);
+        chart.setBarGap(0.0);
+        chart.setCategoryGap(0.0);
+        getTestScene().setRoot(chart);
+
+        XYChart.Series<String, Number> series1 = new XYChart.Series<>();
+        series1.setName("S1");
+        chart.getData().setAll(List.of(series1));
+        series1.getData().add(new XYChart.Data<>("1", 1));
+        series1.getData().add(new XYChart.Data<>("2", 2));
+
+        XYChart.Series<String, Number> series2 = new XYChart.Series<>();
+        series2.setName("S2");
+        series2.getData().add(new XYChart.Data<>("2", 3)); // duplicate category with series1
+        series2.getData().add(new XYChart.Data<>("3", 4)); // new category
+        chart.getData().add(series2);
+
+        pulse();
+
+        // check bar layout
+        List<Node> s1bars = series1.getData().stream().map(XYChart.Data::getNode).toList();
+        List<Node> s2bars = series2.getData().stream().map(XYChart.Data::getNode).toList();
+
+        double x0 = s1bars.getFirst().getLayoutX();
+        double barWidth = s1bars.getFirst().getBoundsInLocal().getWidth();
+
+        // normalize bar positions with respect to the first bar position and width
+        List<Double> normalized1 = s1bars.stream()
+                .map(node -> (node.getLayoutX() - x0) / barWidth)
+                .toList();
+
+        List<Double> normalized2 = s2bars.stream()
+                .map(node -> (node.getLayoutX() - x0) / barWidth)
+                .toList();
+
+        // expect even integers for series1 and odd integers for series2
+        double delta = 0.001;
+        assertEquals(0, normalized1.get(0), delta);
+        assertEquals(2, normalized1.get(1), delta);
+        assertEquals(3, normalized2.get(0), delta);
+        assertEquals(5, normalized2.get(1), delta);
+    }
+
 }