On Wed, 7 May 2025 09:30:05 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org> wrote:
>> This is a continuation to >> [JDK-8236651](https://bugs.openjdk.org/browse/JDK-8236651) and it aims to >> stabilize the linux glass gtk backend. >> >> >> Overall, it has been made more robust within its scope, particularly in >> terms of sizing, positioning, and state management. >> >> List of changes: >> 1. Use GDK for creating Windows. Since we paint directly to the underlying >> XWindow, creating a GtkWidget for the window is unnecessary and results in >> unused GTK resources. Additionally, avoiding the use of a GtkWidget >> eliminates the need for workarounds caused by conflicting GTK behavior—such >> as setting the initial window state, position, or size. >> 2. It aligns with X11's asynchronous behavior by reporting geometry changes >> only upon receiving a configure event, since requests to the window manager >> aren't guaranteed to be honored. However, changes are still reported >> immediately in special cases, such as before the window is mapped. For >> example, a request to move the window to (0, 0) might be overridden by the >> window manager, placing it in the top-right corner below the panels instead. >> 3. States (fullscreen, maximized and iconify) are now reported back to Java >> when it actually happens rather than immediately (except when not mapped); >> 4. When a window is maximized, it will ignore geometry changes and restore >> to the geometry it had prior to being maximized. After some testing, I >> believe this is the best behavior for platform compatibility; >> 5. Unifies the WindowContext class: previously, there were three separate >> classes—two of which (for applets and Java Web Start) were removed, leaving >> only one. However, the supporting infrastructure was still there partially. >> [Unify WindowContext in >> glass-gtk](https://bugs.openjdk.org/browse/JDK-8305768) >> 6. Tests were added and re-enabled to ensure everything works correctly. The >> stage tests now cover various StageStyle configurations, as I found that >> `DECORATED` stages often behave differently from `UNDECORATED` or `UTILITY` >> stages; >> 7. Added Logs for debugging. Enable it with ` -PCONF=DebugNative`; >> 8. Old work-arounds dating back to Ubuntu 16.04 with Compiz were removed. >> >> A simple manual test is provided: >> `java @build/run.args tests/manual/stage/TestStage.java ` >> >> >> List of fixed issues: >> >> 1. [[Linux] Stage.setMaximized() before show() does not >> persist](https://bugs.openjdk.org/browse/JDK-8316425) >> 3. [[Linux] Intermittent test failure in >> IconifyTest.canIconifyDecoratedStage](https://bugs.openjdk.org/brow... > > Thiago Milczarek Sayao has updated the pull request incrementally with one > additional commit since the last revision: > > Fix non-debug build I think the GTK changes are fine, however the tests need some addressing. On Linux (Ubuntu 24.04.2 VM) I see consistently failing two tests: StageOwnershipTest. testClosingAppModalShouldFocusParent(StageStyle) [1] DECORATED StageOwnershipTest. testClosingAppModalShouldFocusParent(StageStyle) [2] UNDECORATED Not sure for the reason for these, but it is a similar failure to Windows ones listed below - line 436 `assertTrue(stage1.isFocused())` fails. Occasionally I also see `StageMixedSizeTest. testSetHeightOnlyAfterShownOnContentSizeWindow(StageStyle) [1] DECORATED` fail - it might be something timing related. As @beldenfox mentioned, on Windows 11 the situation with the new tests is a bit worse. I see consistent failures in: SizingTest. testMinSize(StageStyle) [1] DECORATED SizingTest. testMinSize(StageStyle) [2] UNDECORATED SizingTest. testMinSize(StageStyle) [3] TRANSPARENT SizingTest. testMinSize(StageStyle) [4] UTILITY StageLocationTest. testMove(StageStyle) [1] DECORATED StageOwnershipTest. testChildStageWithoutModality(StageStyle) [1] DECORATED StageOwnershipTest. testChildStageWithoutModality(StageStyle) [2] UNDECORATED StageOwnershipTest. testClosingAppModalShouldFocusParent(StageStyle) [1] DECORATED StageOwnershipTest. testClosingAppModalShouldFocusParent(StageStyle) [2] UNDECORATED StageOwnershipTest. testIconfyRestoreChildren(StageStyle) [1] DECORATED StageOwnershipTest. testIconfyRestoreChildren(StageStyle) [2] UNDECORATED StageOwnershipTest. testLayeredModality(StageStyle) [1] DECORATED StageOwnershipTest. testLayeredModality(StageStyle) [2] UNDECORATED Re: `SizingTest.testMinSize` - these tests all fail for the same reason that seems unrelated to your changes. Stage has min size limit set properly and it is respected by the OS, however JFX still returns smaller width/height. This is _despite_ the window having physically the same dimensions - in other words, Stage does not physically shrink, but JFX returns smaller width/height regardless, causing the Stage state to desync. This should be filed as a new JBS bug and fixed separately, I would set these tests to ignored/disabled on Windows for the time being with comment containing related JDK issue number. Re: `StageLocationTest` - I added a comment in code about this. Re: `StageOwnershipTest` - those fail when questioning APIs like `isIconified` and `isFocused`. We've already had some issues with focus on Windows 11 that I was trying to resolve ([JDK-8351357](https://bugs.openjdk.org/browse/JDK-8351357)) but it seems unrelated (my local hacky-fix-patch did not help it). I think this requires some more analysis to fully determine what is up. tests/system/src/test/java/test/robot/javafx/stage/StageLocationTest.java line 91: > 89: stage.setY(Y); > 90: }, > 91: stage::show, On my Windows 11 machine this consistently fails for DECORATED stage, which is the very first test to be done from the collection. My guess is that this misses a `CountDownLatch` or some other way to ensure the Stage is fully shown. I would also add a similar latch wait to other tests in this file. A common `showStage` method of sorts that calls `stage::show` and then waits for the latch seems to me like the best route. Plenty of other system tests use this pattern if you need to source the solution from somewhere. ------------- PR Review: https://git.openjdk.org/jfx/pull/1789#pullrequestreview-2821812695 PR Review Comment: https://git.openjdk.org/jfx/pull/1789#discussion_r2077664569
