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