RFR: 8314754: Minor ticks are not getting updated both the axes in LineChart

[Andy Goryachev]

On Tue, 25 Jun 2024 15:14:14 GMT, Markus Mack <mmack at openjdk.org> wrote:

> This PR ensures the `tickMarksUpdated()` method (overriden with minor tick mark layout code in subclasses) is called during `Chart` `Axis` layout when needed.
> Previously, it was only called when tick mark length or range was changed, which is not happening during axis animations, causing minor ticks to become outdated when animations are enabled.
> 
> I've added a test that fails with the previous code, specifically checking the tick mark spacing.
> Alternatively the test could be implemented by accessing ValueAxis.minorTickMarkValues, which would need to be made at least package-private however (with a shim). This implementation checks the tick mark `Path` shape instead, which is already publicly accessible (but could be considered an implementation detail).

modules/javafx.controls/src/main/java/javafx/scene/chart/Axis.java line 753:

> 751: 
> 752:             // call tick marks updated to inform subclasses that we have updated tick marks
> 753:             tickMarksUpdated();

is this the only place where this call needs to be added?

for example, in BarChart the x axis is not updated after "Add series with duplicate category" - is this a separate issue?


![Screenshot 2024-06-25 at 11 58 52](https://github.com/openjdk/jfx/assets/107069028/672c0804-fd00-4bc1-9c01-bd89f41056dc)

modules/javafx.controls/src/test/java/test/javafx/scene/chart/XYNumberLineChartsTest.java line 156:

> 154:         NumberAxis yAxis = (NumberAxis)chart.getYAxis();
> 155: 
> 156:         List<Double> majorTickYValues = yAxis.getChildrenUnmodifiable().stream()

does this code captures the ticks before the first frame of animation or after the first frame?

should we also check after the animation ran its course?  (I assume animation does work with the StubToolkit)

modules/javafx.controls/src/test/java/test/javafx/scene/chart/XYNumberLineChartsTest.java line 168:

> 166:                 .filter(path -> path instanceof MoveTo)
> 167:                 .map(moveTo -> ((MoveTo) moveTo).getY())
> 168:                 .toList();

very clever - I think this is the right approach to obtain the necessary information via public APIs rather than shims.

just need to be aware that only the nodes present in the scene graph can be found this way (I think, please correct me if I am wrong)

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1489#discussion_r1653496875
PR Review Comment: https://git.openjdk.org/jfx/pull/1489#discussion_r1653364109
PR Review Comment: https://git.openjdk.org/jfx/pull/1489#discussion_r1653500022