[Desktop] Fix changing WindowPlacement from Fullscreen to Maximized

[MohamedRejeb]

Proposed Changes

When the WindowPlacement is set to Maximized, I changed the isFullscreen value to false to make sure that we exit fullscreen.
It's fixing the issue in mac, I still didn't try it on windows to make sure that it's not causing any issues.

Testing

Test: Describe how you tested your changes. Note that this line (with Test:) is required, your PR will not build without it!

I wasn't able to test this change since isFullscreen is private.

Issues Fixed

Fixes: JetBrains/compose-multiplatform#4380

Google CLA

You need to sign the Google Contributor’s License Agreement at https://cla.developers.google.com/.
This is needed since we synchronise most of the code with Google’s AOSP repository. Signing this agreement allows us to synchronise code from your Pull Requests as well.

[MohamedRejeb]

This fixes the issue, but it looks like it causes a different issue.
Now if you set window placement to floating -> fullscreen -> maximized, the placement is going to be changed to floating instead of maximized. It looks like the isFullscreen = false is going to return the window to the latest state before opening fullscreen.
I will add this condition to the test.
Any ideas how to fix this edge case?

[MohamedRejeb]

I was able to use this workaround, the first set placement to Maximized will exit the fullscreen, then I wait for the full screen to be exited and then I set the placement to Maximized again.

scope.launch {
    window.placement = WindowPlacement.Maximized
    while (window.placement == WindowPlacement.Fullscreen) {
        withFrameMillis {  }
    }
    window.placement = WindowPlacement.Maximized
}

So for this to work correctly, we need to add a check when the window is fullscreen and you change the placement to Maximized exit the fullscreen first and wait for it to be exited successfully then maximize the window.

[MohamedRejeb]

Using something like this can fix the issue, but I'm not sure if you are ok with using runBlocking here. We need to wait for fullscreen to exit before changing the isMaximized value.
There's also another possible solution by relying on the listeners in Window.desktop.kt and correct the isMaximized value after exiting fullscreen.

[igordmn]

Thanks!

I still didn't try it on windows

It seems work on Windows, but it fails on Linux. See the failed tests.

It's fixing the issue in mac

The mac fullscreen logic is overriden in Skiko.

First, it violates the contract: setting isFullscreen = false should immediately cause isFullscreen = false. When this contract is fixed, your workaround won't work.

Second, it performs setting fullscreen asynchronously calling performSelectorOnMainThread. that is why isMaximized = true didn't work.

We probably can't wait for AppKit main thread from Swing thread, as it can cause deadlocks (main thread can wait for Swing thread as well).

I don't have an easy solution in mind

Maybe to fix the mac issue we need to provide skialayer.placement, so it can schedule fullscreen = false and maximized = true on the main AppKit thread at once.

To fix the Linux issue, we need to experiment and look at the Swing sources. If Swing itself doesn't support this use case, we have to write a native code instead.

[igordmn]

Tests fail on Linux in these lines:

expected: Maximized
but was : Floating