RFR: 8299968: Second call to Stage.setScene() create sizing issue with uiScale > 1.0 [v2]
Kevin Rushforth
kcr at openjdk.org
Wed Mar 8 14:50:14 UTC 2023
On Tue, 7 Mar 2023 15:20:50 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 one additional commit since the last revision:
>
> Move SetSceneScalingTest to robot test directory
The fix looks good to me, although I want to test on a dual screen setup. I doubt there will be any difference in behavior, since the fix looks independent of the screen. I did leave one minor suggestion on a comment you added.
The test fails for me intermittently, which suggests that there is a timing issue. You will likely need to use a latch to wait for the button to be clicked (or a sleep of around 1/2 sec) rather than immediately testing `wasClicked`.
modules/javafx.graphics/src/main/java/com/sun/glass/ui/Window.java line 374:
> 372: this.view = view;
> 373: this.view.setWindow(this);
> 374: // JDK-8299968: View size update (especially notifyResize event) have to happen
Minor: we generally don't put a bug ID in comments in the implementation when fixing a bug.
-------------
PR: https://git.openjdk.org/jfx/pull/1054
More information about the openjfx-dev
mailing list