On Thu, 9 Mar 2023 14:56:36 GMT, Lukasz Kostyra <lkost...@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
