On Tue, 25 Jun 2024 15:14:14 GMT, Markus Mack <mm...@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?  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
