On Mon, 27 Jan 2025 20:22:30 GMT, Andy Goryachev <ango...@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
