Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Window doesn't apply placement and size after start in 1.4.0-RC1 #3003

Closed
Nourepide opened this issue Apr 7, 2023 · 5 comments · Fixed by JetBrains/skiko#689 or JetBrains/compose-multiplatform-core#528
Assignees
Labels
bug Something isn't working p:high High priority reproduced

Comments

@Nourepide
Copy link

Nourepide commented Apr 7, 2023

Describe the bug
After switching to version 1.4.0, the window stopped applying the placement and size parameters at the start of the application. If some parameter has been changed, the window immediately applies all the parameters that were passed before.

Affected platforms
Select one of the platforms below:

  • Desktop

Versions

  • Kotlin version: 1.8.10
  • Compose Multiplatform version: 1.4.0-rc01
  • OS version(s): Arch Linux KDE
  • OS architecture (x86 or arm64): x86
  • JDK (for desktop issues): Corretto 18

To Reproduce
Just run this code.

import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.material3.Button
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.window.WindowPlacement
import androidx.compose.ui.window.singleWindowApplication
import java.awt.Dimension

data class WindowState(
    val windowPlacement: MutableState<WindowPlacement> = mutableStateOf(WindowPlacement.Fullscreen),
    val windowSize: MutableState<Dimension> = mutableStateOf(Dimension(1366, 768))
)

fun main() {
    val windowState = WindowState()

    singleWindowApplication {
        window.placement = windowState.windowPlacement.value
        window.preferredSize = windowState.windowSize.value
        window.size = windowState.windowSize.value

        Screen(windowState)
    }
}

@Composable
fun Screen(windowState: WindowState) {
    Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
        Row {
            Button(onClick = { windowState.windowPlacement.value = WindowPlacement.Floating }) {
                Text("Set floating.")
            }

            Button(onClick = { windowState.windowSize.value = Dimension(1280,720 ) }) {
                Text("Change window size.")
            }
        }
    }
}

Expected behavior
As in 1.3.1

@Nourepide Nourepide added bug Something isn't working submitted labels Apr 7, 2023
@Nourepide
Copy link
Author

Nourepide commented Apr 7, 2023

Reproduced in Windows 10.

I suspect it's this merge request.
JetBrains/compose-multiplatform-core#442

Appears in v1.4.0-alpha01-dev985
https://github.com/JetBrains/compose-multiplatform/releases/tag/v1.4.0-alpha01-dev985

@igordmn
Copy link
Collaborator

igordmn commented Apr 7, 2023

Thanks for the investigation!

    singleWindowApplication {
        window.placement = windowState.windowPlacement.value
        window.preferredSize = windowState.windowSize.value
        window.size = windowState.windowSize.value

        Screen(windowState)
    }

It isn't correct to change these properties in the composition, because:

  1. Composable's can be called multiple time at any time, and the size/position of the window will be overridden on every recomposition.
  2. singleApplicationWindow owns these properties, and there can be concurrent changes, which can lead to unpredictable behavior.

The correct way should be to use built-in WindowState:

import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.material.Button
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.*

fun main() {
    val windowState = WindowState(WindowPlacement.Fullscreen, size = DpSize(1280.dp, 720.dp))

    singleWindowApplication(windowState) {
        Screen(windowState)
    }
}

@Composable
fun Screen(windowState: WindowState) {
    Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
        Row {
            Button(onClick = {
                windowState.placement = WindowPlacement.Floating
                windowState.size = DpSize(1280.dp, 720.dp)
            }) {
                Text("Set floating.")
            }

            Button(onClick = { windowState.size = DpSize(1280.dp, 720.dp) }) {
                Text("Change window size.")
            }
        }
    }
}

But it doesn't work unfortenutely, even in Compose 1.2.0/1.3.1. So, Compose has a long standing bug.

Until this is fixed, the workaroround is to use ComposeWindow directly:

import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.material.Button
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.awt.ComposeWindow
import androidx.compose.ui.window.AwtWindow
import androidx.compose.ui.window.WindowPlacement
import androidx.compose.ui.window.application
import java.awt.Dimension
import java.awt.event.WindowAdapter
import java.awt.event.WindowEvent
import javax.swing.JFrame

fun main() {
    var isOpen by mutableStateOf(true)

    application {
        if (isOpen) {
            AwtWindow(
                create = {
                    ComposeWindow().apply {
                        setContent {
                            Screen(this@apply)
                        }
                        defaultCloseOperation = JFrame.DO_NOTHING_ON_CLOSE
                        addWindowListener(object : WindowAdapter() {
                            override fun windowClosing(e: WindowEvent?) {
                                isOpen = false
                            }
                        })
                        placement = WindowPlacement.Fullscreen
                    }
                },
                dispose = ComposeWindow::dispose
            )
        }
    }
}

@Composable
fun Screen(window: ComposeWindow) {
    Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
        Row {
            Button(onClick = {
                if (window.placement != WindowPlacement.Fullscreen) {
                    window.placement = WindowPlacement.Fullscreen
                } else {
                    window.placement = WindowPlacement.Floating
                    window.size = Dimension(1366, 768)
                }
            }
            ) {
                Text("Toggle fullscreen")
            }
        }
    }
}

@igordmn igordmn added p:high High priority reproduced and removed submitted labels Apr 7, 2023
@m-sasha
Copy link
Member

m-sasha commented Apr 14, 2023

The problem here is in FullscreenAdapter in Skiko. Its componentResized is called before componentShown, which causes _isFullscreen to be false. This is then picked up by the ComponentAdapter in Window.desktop:Window:Window(create) which re-sets WindowState.placement to Floating. So after opening the window in fullscreen, WindowState.placement is actually Floating. Then, when the button sets it to Floating, it doesn't actually do anything.

@m-sasha
Copy link
Member

m-sasha commented Apr 14, 2023

Reproducer/test:

    @Test
    fun `WindowState placement after showing fullscreen window`() = runApplicationTest(useDelay = isLinux) {
        val state = WindowState(placement = WindowPlacement.Fullscreen)

        launchTestApplication {
            Window(onCloseRequest = {}, state) { }
        }

        awaitIdle()

        assertThat(state.placement).isEqualTo(WindowPlacement.Fullscreen)
    }

@okushnikov
Copy link
Collaborator

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p:high High priority reproduced
Projects
None yet
4 participants