RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v6]

[Kevin Rushforth]

On Mon, 26 Apr 2021 13:14:20 GMT, Jonathan Vusich <github.com+31666175+JonathanVusich at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/chart/CategoryAxis.java line 365:
>> 
>>> 363:             double requiredLengthToDisplay = calculateRequiredSize(side.isVertical(), tickLabelRotation);
>>> 364:             if (requiredLengthToDisplay > length) {
>>> 365:                 if (tickLabelRotation != 90) {
>> 
>> One difference between the new algorithm and the old is that the new doesn't take into account which side you are on. If the only two values are 0 or 90, then it wouldn't matter. But what happens when someone sets a 45 degree rotation (or 30)?
>
> Well, it's complicated. The algorithm that calculates the required space for the tick labels does not take into account the tick label rotation. I did not look into adding this at this time because that opens up a lot of other potential pathways that seem broader in scope. With these changes, if the application detects that there is not enough space to display the text at the current rotation (which is assumed to be either 90 or 0 degrees), we rotate it by 90 degrees and remeasure. If that second remeasuring shows that the tick labels can be fully displayed at that rotation, the tick label rotation is overwritten to either 0 or 90 degrees. Theoretically if a user defines a set tick label rotation, it can be overwritten under these circumstances. However, I think this is reasonable given that when `autoRanging` is disabled a number of other properties are also overwritten in a similar manner.

Yeah, it is complicated, and I agree doesn't work entirely correctly today, but this change breaks setting the axis tick label rotation completely, even in case where will fit without forcing the rotation to 90.

To reproduce this problem, run the Ensemble8 app with your fix, select the Bar Chart sample, and attempt to change the X axis `tickLabelRotation`, either using the slider or entering a number in the text field. You will be unable to change the value since the code you added will overwrite it.

If you run it without your fix, you can change the X axis `tickLabelRotation`. It won't take effect right away (that's a bug), but if you uncheck and then recheck `tickLabelsVisible` it will re-layout the chart and the axis labels will be drawn rotated.

With auto-ranging selected, if you run with your fix, you can change the axis, but it won't actually do anything, even in the case where the label fits.

Without your fix, you can rotate the labels with the slider just fine.

So while I agree that improving the situation when a rotation is set could be done as a follow-up bug, we need a solution that won't break existing use cases that are working.

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

PR: https://git.openjdk.java.net/jfx/pull/342