RFR: 8299968: Second call to Stage.setScene() create sizing issue with uiScale > 1.0 [v3]
Ambarish Rapte
arapte at openjdk.org
Mon Mar 13 11:39:36 UTC 2023
On Thu, 9 Mar 2023 14:56:36 GMT, Lukasz Kostyra <lkostyra at openjdk.org> wrote:
>> Issue happened during setting a new Scene - updating a new View was done while the Window reference it kept was null. This caused it to default scaling values to 1.0f (or 100%) while processing a resize notification, which for high DPI screens with scaling different than 100% caused UI issues.
>>
>> Resolved by splitting `_setView()` native call into two parts - first one sets the view, then back in JVM side we set a correct Window reference, then we trigger the notification. It has to be triggered from native side, because Windows backend of Glass sends back new width/height pulled from WinAPI `::GetClientRect()` call.
>>
>> In process of working on this issue I also found another scenario causing the same problem - calling `Stage.setScene()` after `Stage.show()`. The patch fixed that case as well.
>>
>> Added a system test which is supposed to check for above issues. I didn't limit it to run only on platforms with UI scaling enabled because it also serves as a good sanity check in case there are some other changes to code that might move/scale the UI unwantingly. I tested this patch both on macOS Ventura and Windows 11, with `d9c091f` all tests pass while without `d9c091f` on Windows tests `testShowAndSetScene` and `testSecondSetScene` fail as expected.
>
> Lukasz Kostyra has updated the pull request incrementally with two additional commits since the last revision:
>
> - SetSceneScalingTest: Use latch to check if button was pressed
> - Window: Remove bug ID from comment
The fix seems good to me. Suggesting few changes in test and a minor correction in source.
modules/javafx.graphics/src/main/native-glass/win/GlassWindow.cpp line 1327:
> 1325: }
> 1326: }
> 1327: GlassView * view;
The variable `view` is not needed, must be a copy-paste leftover.
tests/system/src/test/java/test/robot/javafx/stage/SetSceneScalingTest.java line 64:
> 62:
> 63: protected void testButtonClick() {
> 64: robot.mouseMove(400, 400);
In general, this is not hard coded in our tests.
I would recommend to change as:
1. Add two variables width and height in TestApp
private final int WIDTH = 400;
private final int HEIGHT = 400;
2. Change this line to:
robot.mouseMove((int)(stage.getX() + stage.getScene().getX() + WIDTH/2),
(int)(stage.getY() + stage.getScene().getY() + HEIGHT/2));
3. Remove line 97, 98
4. Change line 99 and 100 as:
stage.setWidth(WIDTH);
stage.setHeight(HEIGHT);
tests/system/src/test/java/test/robot/javafx/stage/SetSceneScalingTest.java line 96:
> 94: stage = new Stage(StageStyle.UNDECORATED);
> 95: stage.addEventHandler(WindowEvent.WINDOW_SHOWN, e ->
> 96: Platform.runLater(shownLatch::countDown));
Can be changed to :
stage.setOnShown(l -> {
Platform.runLater(() -> startupLatch.countDown());
});
-------------
Changes requested by arapte (Reviewer).
PR: https://git.openjdk.org/jfx/pull/1054
More information about the openjfx-dev
mailing list