RFR: 8299753: Tree/TableView: Column Resizing With Fractional Scale [v7]
Kevin Rushforth
kcr at openjdk.org
Wed Jan 29 21:45:59 UTC 2025
On Mon, 27 Jan 2025 20:22:30 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> Modified the resize algorithm to work well with fractional scale, thanks for deeper understanding of the problem thanks to @hjohn and @mstr2 .
>>
>> Removed earlier manual tester in favor of the monkey tester.
>>
>> It is important to note that even though the constraints are given by the user in unsnapped coordinates, they are converted to snapped values, since the snapped values correspond to the actual pixels on the display. This means the tests that validate honoring constraints should, in all the cases where (scale != 1.0), assume possibly error not exceeding (1.0 / scale).
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 41 commits:
>
> - review comments
> - Merge remote-tracking branch 'origin/master' into 8299753.resize
> - Merge branch 'master' into 8299753.resize
> - Merge remote-tracking branch 'origin/master' into 8299753.resize
> - Merge remote-tracking branch 'origin/master' into 8299753.resize
> - in case of hitting min max
> - Merge branch 'master' into 8299753.resize
> - Merge branch 'master' into 8299753.resize
> - Merge remote-tracking branch 'origin/master' into 8299753.resize
> - Merge branch 'master' into 8299753.resize
> - ... and 31 more: https://git.openjdk.org/jfx/compare/a1765747...5464d7ee
All my testing looks good. I left a couple more comments about using ceil for max as well as min.
modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ResizeHelper.java line 79:
> 77: min[i] = cmin;
> 78: max[i] = cmax;
> 79: pref[i] = clip(snapRound(c.getPrefWidth()), cmin, cmax);
Since `prefWidth` is a size, should it also be `snapCeil`?
modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ResizeHelper.java line 588:
> 586: }
> 587:
> 588: private double snapFloor(double x) {
This method can be removed if you change max to use `snapCeil`.
modules/javafx.controls/src/main/java/javafx/scene/control/ResizeFeaturesBase.java line 108:
> 106: width = max;
> 107: if (width < min) {
> 108: // safety check in case floor(max) < ceil(min)
This comment should now be changed to: "...in case ceil(max) < ceil(min)" (or maybe just "...max < min")
-------------
PR Review: https://git.openjdk.org/jfx/pull/1156#pullrequestreview-2582197459
PR Review Comment: https://git.openjdk.org/jfx/pull/1156#discussion_r1934664642
PR Review Comment: https://git.openjdk.org/jfx/pull/1156#discussion_r1934689930
PR Review Comment: https://git.openjdk.org/jfx/pull/1156#discussion_r1934552495
More information about the openjfx-dev
mailing list