On Thu, 23 May 2024 14:01:40 GMT, Marius Hanl <mh...@openjdk.org> wrote:

>> This PR fixes the problem that maximizing/fullscreen a `Stage` or `Dialog` 
>> is broken when `sizeToScene()` was called before or after.
>> 
>> The approach here is to ignore the `sizeToScene()` request when the `Stage` 
>> is maximized or set to fullscreen. 
>> Otherwise the Window Manager of the OS will be confused and you will get 
>> weird flickering or wrong Window buttons (e.g. on Windows, the 'Maximize' 
>> button still shows the 'Restore' Icon, while we already resized the `Stage` 
>> to not be maximized).
>
> Marius Hanl has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Implement isSizeToSceneAllowed() method to determines whether the 
> sizeToScene() request is allowed. Implement much more tests
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 
> jdk-8326619-maximize-minimize-scene
>  - improve tests
>  - JDK-8326619: Improve tests
>  - JDK-8326619: Stage.sizeToScene() on maximized/fullscreen Stage breaks the 
> Window

I won't have time to do a detailed review for a while, but the updated approach 
to the fix looks promising. I note that the change in behavior will need to be 
documented another way, possibly in the base `Window::sizeToScene` method, 
since the newly added method is, correctly, not public (nor should it be).

When running the new test on macOS, I see a few test failures followed by a 
crash. The crash is clearly a bug in JavaFX glass code (I'll file a bug for 
that), but the failures point to a problem with the test.

I noticed while running it that there are no delays between various window 
operations in the tests. This will never work on Mac (and likely is a factor in 
provoking the crash), and will not be reliable on other platforms.

Here are the test failures:


SizeToSceneTest > testInitialSizeOnSizeToScene() FAILED
    org.opentest4j.AssertionFailedError: 1536.0 <= 410 ==> expected: <true> but 
was: <false>
        at 
app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
        at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
        at 
app//test.javafx.stage.SizeToSceneTest.assertStageSceneBounds(SizeToSceneTest.java:77)
        at 
app//test.javafx.stage.SizeToSceneTest.testInitialSizeOnSizeToScene(SizeToSceneTest.java:197)

SizeToSceneTest > testInitialSizeSizeToSceneFullscreenOnOff() FAILED
    org.opentest4j.AssertionFailedError: 1536.0 <= 410 ==> expected: <true> but 
was: <false>
        at 
app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
        at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
        at 
app//test.javafx.stage.SizeToSceneTest.assertStageSceneBounds(SizeToSceneTest.java:77)
        at 
app//test.javafx.stage.SizeToSceneTest.testInitialSizeSizeToSceneFullscreenOnOff(SizeToSceneTest.java:229)

SizeToSceneTest > testInitialSizeMaximizedOnOffSizeToScene() FAILED
    org.opentest4j.AssertionFailedError: 1536.0 <= 410 ==> expected: <true> but 
was: <false>
        at 
app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
        at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
        at 
app//test.javafx.stage.SizeToSceneTest.assertStageSceneBounds(SizeToSceneTest.java:77)
        at 
app//test.javafx.stage.SizeToSceneTest.testInitialSizeMaximizedOnOffSizeToScene(SizeToSceneTest.java:245)

SizeToSceneTest > testInitialSizeFullscreenOnOffSizeToScene() FAILED
    org.opentest4j.AssertionFailedError: 1536.0 <= 410 ==> expected: <true> but 
was: <false>
        at 
app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
        at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
        at 
app//test.javafx.stage.SizeToSceneTest.assertStageSceneBounds(SizeToSceneTest.java:77)
        at 
app//test.javafx.stage.SizeToSceneTest.testInitialSizeFullscreenOnOffSizeToScene(SizeToSceneTest.java:213)

SizeToSceneTest > testInitialSizeSizeToSceneMaximizedOnOff() FAILED
    org.opentest4j.AssertionFailedError: 1536.0 <= 410 ==> expected: <true> but 
was: <false>
        at 
app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
        at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210)
        at 
app//test.javafx.stage.SizeToSceneTest.assertStageSceneBounds(SizeToSceneTest.java:77)
        at 
app//test.javafx.stage.SizeToSceneTest.testInitialSizeSizeToSceneMaximizedOnOff(SizeToSceneTest.java:261)

Java has been detached already, but someone is still trying to use it at 
-[GlassWindow(Overrides) 
windowDidResignKey:]:jfx/modules/javafx.graphics/src/main/native-glass/mac/GlassWindow+Overrides.m:96
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x0000000102f2fdbe, pid=25095, tid=259
#
# JRE version: Java(TM) SE Runtime Environment (21.0.2+13) (build 
21.0.2+13-LTS-58)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (21.0.2+13-LTS-58, mixed mode, 
sharing, tiered, compressed oops, compressed class ptrs, g1 gc, bsd-amd64)
# Problematic frame:
# C  [libglass.dylib+0x29dbe]  -[GlassWindow(Overrides) 
windowDidResignKey:]+0xde
#
# No core dump will be written. Core dumps have been disabled. To enable core 
dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# jfx/tests/system/hs_err_pid25095.log
Gradle Test Executor 1 finished executing tests.
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp
#
0   libglass.dylib                      0x0000000102f2fd7b 
-[GlassWindow(Overrides) windowDidResignKey:] + 155
1   CoreFoundation                      0x00007ff807aea688 
__CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 137
2   CoreFoundation                      0x00007ff807b833fe 
___CFXRegistrationPost_block_invoke + 88
3   CoreFoundation                      0x00007ff807b83353 _CFXRegistrationPost 
+ 536
4   CoreFoundation                      0x00007ff807abd927 _CFXNotificationPost 
+ 729
5   Foundation                          0x00007ff80892ab30 
-[NSNotificationCenter postNotificationName:object:userInfo:] + 82
6   AppKit                              0x00007ff80ad2bd37 -[NSWindow 
resignKeyWindow] + 758
7   AppKit                              0x00007ff80b4d7e4b -[NSWindow 
_orderOut:calculatingKeyWithOptions:documentWindow:] + 326
8   AppKit                              0x00007ff80abdd8b7 
NSPerformVisuallyAtomicChange + 132
9   AppKit                              0x00007ff80b4da0a7 -[NSWindow 
_reallyDoOrderWindowOutRelativeTo:] + 618
10  AppKit                              0x00007ff80b4da4ca -[NSWindow 
_reallyDoOrderWindow:] + 99
11  AppKit                              0x00007ff80b4da740 -[NSWindow 
_doOrderWindow:] + 295
12  AppKit                              0x00007ff80b4d5029 -[NSWindow 
_finishClosingWindow] + 306
13  AppKit                              0x00007ff80ae92a1d -[NSWindow _close] + 
336
14  libglass.dylib                      0x0000000102f3071f -[GlassWindow_Normal 
close] + 79
15  Foundation                          0x00007ff8089a0743 
__NSThreadPerformPerform + 177
16  CoreFoundation                      0x00007ff807af4eba 
__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
17  CoreFoundation                      0x00007ff807af4e5c __CFRunLoopDoSource0 
+ 157
18  CoreFoundation                      0x00007ff807af4c93 
__CFRunLoopDoSources0 + 311
19  CoreFoundation                      0x00007ff807af38bf __CFRunLoopRun + 916
20  CoreFoundation                      0x00007ff807af2ec1 CFRunLoopRunSpecific 
+ 560
21  libjli.dylib                        0x000000010201bee2 
CreateExecutionEnvironment + 386
22  libjli.dylib                        0x00000001020178bd JLI_Launch + 1357
23  java                                0x0000000101faec17 main + 391
24  dyld                                0x00007ff8076be41f start + 1903

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1382#issuecomment-2130099872

Reply via email to