On Wed, 29 Mar 2023 17:31:42 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>>> > @andy-goryachev-oracle raises a good point about perhaps wanting to solve >>> > this in a similar way for the various places where we need to do some >>> > sort of iterative adjustment (such as here and in the eventual solution >>> > for JDK-8299753). It seems also worth looking at what @Maran23 proposes >>> > for AnchorPane to fix JDK-8295078 in PR #910 (for which the review still >>> > needs to be completed). >>> >>> Yes, at this point we should probably discuss what would be the best >>> solution to fix the snapping issues once and for all. We might even want to >>> look into other frameworks like Swing. Maybe there are people internally at >>> Oracle who can give valuable input here too? >> >> I think extracting the common code out of HBox and VBox would be good, as >> they're basically doing the same thing in a different axis. The code could >> then be unit tested directly (ie. given a set of min/max/pref sizes, a >> target size and a "snapper", calculate optimal sizes). Something like this: >> >> double[] distribute(double space, double[] minWidths, double[] >> maxWidths, Snapper snapper); >> >> The minWidths/maxWidths would be min + pref or pref + max depending on the >> situation, but this function wouldn't care. The `Snapper` here would be >> aware of the scale, and would be a dummy implementation that does nothing >> when snapping is disabled. >> >> This purposely does not handle spacing between children, as you can >> calculate that before hand and adjust `space` to the actual space available >> for children, simplifying the code. >> >> Disclaimer: I used to create my own layout manager (see: >> [smartlayout](https://github.com/hjohn/hs.ui.smartlayout/blob/master/src/main/java/hs/smartlayout/LayoutRequirements.java)) >> that took into account min/max and weight of each child, and even groups of >> children (a restriction over two or more children at once) -- weight would >> affect resizing, allowing you to make layouts where children took space >> according to some ratio (1:2:3 for example). This functionality I called a >> `SpaceDistributor` which was purposely given a neutral name as it could be >> used for both horizontal or vertical calculations. When including weight, >> the calculations could get so complex that I had multiple implementations >> and an exhaustive search implementation (which was used by unit tests to >> verify the optimized implementations). This was before scaling became an >> issue though, so this code did not take snapping values to pixels into >> account. > > Thank you very much @hjohn for your extensive review. Before addressing your > comments, I'd like to get some clarification (ideally also from > @kevinrushforth), because I am a bit confused by the discussion so far. > > This PR is a very narrow patch that fixes an obvious bug in `HBox` and > `VBox`. It's narrow in that it doesn't change the present algorithm, it > merely corrects some of its flaws. There is a little bit of refactoring, but > that's only done where it helps readers to better understand the algorithm. > > However, the discussion seems to center around the idea of large-scale > refactoring, even across multiple components, including open tickets like > [8299753](https://bugs.openjdk.org/browse/JDK-8299753). That's not something > I can do as part of this PR, and if large-scale refactoring is the way we > choose to go forward, I'd rather not spend more of my time on this particular > PR. There needs to be a realistic chance that this PR will be accepted for > integration basically as it is. > > If we want to expand the scope of this work, this PR should be closed and the > discussion should be continued on the mailing list. @mstr2 I noticed in your test that you said that when `prefWidth` is set, the container must use exactly that size, but this is not entirely true. It just so happens that `76.0` is a pixel boundary for all the render scales you tested. If you use a different width or use a different render scale (like `1.33333333`) then it won't be a pixel boundary and the width will not be exactly what you specified for preferred width. With pref width `76.0` and render scale `1.33333333` the final width will be `76.5` for example. ------------- PR Comment: https://git.openjdk.org/jfx/pull/445#issuecomment-1523144658
