Thank you for detecting and fixing this issue guys. I have spent many days / weeks chasing issues related to this issue. It often came up with my custom controls and I was always assuming that it is me making a mistake. I never even considered that this might be a bug in the layout code.
Dirk > Am 23.02.2025 um 23:10 schrieb John Hendrikx <jhendr...@openjdk.org>: > > Fixes the case where `VBox` will ignore the (horizontal) bias of a child when > its fill width property is set to `false`. This manifests itself with labels > that have their wrap text property set to `true`, and the container is not > wide enough to hold the text on a single line (in other words, the label is > potentially far wider, and fill width should have no effect). No reflow > would occur before this fix. > > The solution is to ensure the `computeChildAreaMin/Pref/MaxHeight` functions > are provided with a `fillWidth` parameter, to be used in the case a > horizontally biased control is encountered (several of the parameters to > these compute functions are only used for those special cases and ignored > otherwise). > > With this additional information, the compute functions can decide between > the preferred width of a control or the available width of the container. In > the old implementation this decision was made *before* these functions were > called, and the available width was reset to -1 to indicate the case that the > preferred width should be used. However, there are three cases, and so > setting a single parameter to -1 can't effectively communicate that: > > Assuming a control that is horizontally biased, and is resizable there are > three cases when calculating a **height**: > > 1. There is no available width: bias is ignored (as there is no value to use > as a dependent value) and the height is then simply calculated ignoring > available width (which is -1) and fill width settings (same as unbiased > controls). > 2. There is an available width and fill width is false; the bias logic must > first find a reasonable width before it can call any height function; with > fill width false, this reasonable width will be the preferred width of the > control, unless it exceeds its maximum width or the available width, in which > case the smallest of these three values is used. The final width calculated > is then used to determine the height. > 3. There is an available width and fill width is true; this is the same as > case 2, except the available width is used as a dependent value (unless its > greater than the child's maximum width, in which case the smaller is used). > The final width calculated is then used to determine the height. > > All in all, this PR removes several inconsistencies between horizontal and > vertical computations. The end result is that the horizontal and vertical > bias calculations now more closely mirror each other. > > **Note**: some tests have had their values adjusted. This is because the > asymmetries between the compute width/height functions have been removed. > Most of these tests use the `MockBiased` test control that simulates an area > of pixels that always covers the same size (ie. it is 10x1000 or 100x100 or > 1000x10 = 10000). One should be aware though that when correctly querying > its minimum size, it will say something like 100x100 -- and that's correct, > as that's what `MockBiased` will tell the layout system, even though in a > single dimension it can return a minimum size of 1 when the other dimension > provided is set to say 10000. > > Due to a bug in how content bias was handled before in the height functions > (content bias was taken into account when `-1` was passed which is incorrect) > some tests saw much smaller minimum sizes. `HBoxTest` is an example of this, > but if you compare it to the equivalent code in `VBoxTest`, you can see that > something is clearly off, as the tests don't agree even though they mirror > each other. The original developer tried to rationalize these differences, > instead of investigating where they came from. > > It may be worth to add tests for a different type of biased control. > `MockBiased` is an example of a control that requires roughly the same area > when resized (like a reflowing Label, or a FlowPane). Another example is a > control that tries to maintain a fixed aspect ratio, like a scaling Image. > The latter type is not interested in maintaining an area of roughly the same > size, but instead it is interested in maintaining a fixed factor between its > width and height. > > **Note 2**: Many layout containers will call min/pref/max width/height > methods with a dependent size provided (instead of -1) even when the control > has no content bias. Control implementations therefore need to be careful > not to use this value when they have no bias or have the opposite bias. > `MockBiased` handles this correctly. > > **BorderPaneTest** > > BorderPaneTest has had some tests adjusted as its height computations were > affected by the bug in the compute height functions where a biased > calculation was partially executed when it shouldn't have. The height values > make much more sense now instead of being some weird fractional number when a > biased control is involved. I've adjusted the tests accordingly, and tried > adding some rationalization of how the values are reached. Note that > `BorderPane` (apparently) always adjusts whatever size it is given during > layout to be greater than its minimum size; this is the first time I've seen > containers do that, instead of going with whatever they've been given... > > ------------- > > Commit messages: > - Make computeChildMin/Pref/MaxAreaHeight accept a fillWidth parameter > > Changes: https://git.openjdk.org/jfx/pull/1723/files > Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1723&range=00 > Issue: https://bugs.openjdk.org/browse/JDK-8350149 > Stats: 989 lines in 12 files changed: 815 ins; 49 del; 125 mod > Patch: https://git.openjdk.org/jfx/pull/1723.diff > Fetch: git fetch https://git.openjdk.org/jfx.git pull/1723/head:pull/1723 > > PR: https://git.openjdk.org/jfx/pull/1723
