On Sun, 25 Oct 2020 19:27:30 GMT, Jose Pereda <jper...@openjdk.org> wrote:
>> curious: it this expected to fix the ellipsed checkBox texts? Can verify >> that the test fails/passes before/after the fix, but the example in the >> report looks still is eclipsed: same for 1.5, slightly better (in that only >> the first text is eclipsed compared to the first 3 without) for 1.75 >> scaling. What am I missing? > > @kleopatra Thanks for checking. > > As discussed in the JBS issue, there is a call in > `CheckBoxSkin::layoutChildren` that, probably by mistake, calls `snapSizeX` > twice, and that gives different results when a different UI scale is used. > > This PR applies to the snapSizeXX methods, so it is guaranteed that > `snapSizeXX(snapSizeXX(a)) == snapSizeXX(a)`. With the included test, _this_ > issue should now be fixed. > > However, you are totally right, the CheckBox test from the JBS issue only > works fine with this PR either if `Stage::sizeToScene` is called or if the > Scene is created with a defined width, so we still have another issue laying > around. > > Running the CheckBox test on Windows with 1.75% UI scale and without calling > `sizeToScene`, produces this log of width and height values: > > Scene::resizeRootOnSceneSizeChange 256.0, 33.0 > ... > Scene::resizeRootOnSceneSizeChange 256.0, 33.0 > ... > // WindowEvent.RESIZE event UI scale 1.75 > ... > Scene::resizeRootOnSceneSizeChange 256.0, 33.14285659790039 > > Both values should be snapped, as these are generated with snap methods in > HBox. However, this is not the case for height, as there are some conversions > in between: This 33.14285659790039 value comes from an int (`View`: height = > (int) (33.0 x 1.75) = 58) to float (`TKSceneListener`: h = 33.142857) to > double (`Scene` width = 33.14285659790039). However, the proper snapped value > should be 33.14285714285714. > > However, if we add a call to `stage.sizeToScene()`, then the log is: > > Scene::resizeRootOnSceneSizeChange 256.0, 33.0 > ... > Scene::resizeRootOnSceneSizeChange 256.0, 33.0 > ... > Scene::resizeRootOnSceneSizeChange 256.0, 33.14285659790039 > ... > Scene::resizeRootOnSceneSizeChange 257.1428571428571, 33.14285714285714 > ... > Scene::resizeRootOnSceneSizeChange 257.1428571428571, 33.14285714285714 > ... > both width and height have stable snapped values, and the checkbox layout is > correct. > > Back to the 256.0 value of width, this value comes from margin (8.0 * 2) + > controls width(54.0 * 4) + spacing (8.0 * 3), but the value 54.0 is not > properly computed, as at this point the UI scale is still 1.0. > Once the UI scale is set to 1.75 the checkBox width is 54.285714285714285, > and the scene's width is 257.14285714285714. > > So the issue can be related to the UI scale value taken into account too > late, and how this change doesn't trigger an update of the scene. > > Does it make sense to address this issue with the current PR? thanks for the additional info - good dig 👍 As you noted, there are two issues: - the implementation of the scaled snaps to make them stable (against nested calls) BTW, any reason snappedXXInset are not scaled? - a timing/sequence issue with applying the scale So might separate them out in different issues, the first with a new one with this PR fixing it. Depends a bit on granularity of issues - personally, would prefer it, though, if only to make snapping issues more prominent. ------------- PR: https://git.openjdk.java.net/jfx/pull/336
