JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
15.92k stars 1.16k forks source link

Linux Window Resizing Issue #1040

Closed ruckustboom closed 2 years ago

ruckustboom commented 3 years ago

Sorry for the non-descriptive title, but it's hard to explain. Resizing a window for me is super buggy. It stutters and glitches, and resizing on one axis causes the other to randomly grow. Here's a video demonstrating the issue:

https://user-images.githubusercontent.com/1735659/129054830-2c420a54-fd8a-4ff9-8f05-04db358717fb.mp4

Running on Fedora 34


Code:

class SplitState(initialSize: Dp = 100.dp, grabWidth: Dp = 8.dp) {
    var size by mutableStateOf(initialSize)
    var isResizing by mutableStateOf(false)
    var grabWidth by mutableStateOf(grabWidth)
}

@Composable
fun VerticalSplittable(
    state: SplitState,
    modifier: Modifier = Modifier,
    children: @Composable () -> Unit,
) = Layout({
    children()
    VerticalSplitter(state)
}, modifier, measurePolicy = { measurables, constraints ->
    require(measurables.size == 3)

    val firstPlaceable = measurables[0].measure(constraints.copy(minWidth = 0))
    val secondWidth = constraints.maxWidth - firstPlaceable.width
    val secondPlaceable = measurables[1].measure(
        Constraints(
            minWidth = secondWidth,
            maxWidth = secondWidth,
            minHeight = constraints.maxHeight,
            maxHeight = constraints.maxHeight
        )
    )
    val splitterPlaceable = measurables[2].measure(constraints)
    layout(constraints.maxWidth, constraints.maxHeight) {
        firstPlaceable.placeRelative(0, 0)
        secondPlaceable.placeRelative(firstPlaceable.width, 0)
        splitterPlaceable.placeRelative(firstPlaceable.width - (state.grabWidth / 2F).roundToPx(), 0)
        if (!state.isResizing) {
            val expected = state.size.coerceIn(0.dp, constraints.maxWidth.toDp())
            if (expected != state.size) state.size = expected
        }
    }
})

@Composable
private fun VerticalSplitter(state: SplitState) = Box {
    val density = LocalDensity.current
    Box(
        Modifier
            .width(state.grabWidth)
            .fillMaxHeight()
            .background(Color(0.5F, 0.5F, 0.5F, 0.5F))
            .draggable(
                state = rememberDraggableState { with(density) { state.size += it.toDp() } },
                orientation = Orientation.Horizontal,
                startDragImmediately = true,
                onDragStarted = { state.isResizing = true },
                onDragStopped = { state.isResizing = false },
            )
            .cursorForHorizontalResize()
    )
}

@OptIn(ExperimentalComposeUiApi::class)
fun Modifier.cursorForHorizontalResize(): Modifier = pointerHoverIcon(PointerIcon(Cursor(Cursor.E_RESIZE_CURSOR)))

// Demo

fun main() = application {
    Window(onCloseRequest = ::exitApplication) {
        val state = remember { SplitState() }

        VerticalSplittable(state, modifier = Modifier.fillMaxSize()) {
            Box(Modifier.width(state.size).fillMaxHeight().background(Color.Blue)) {}
            Box(Modifier.fillMaxHeight().background(Color.Green)) {}
        }
    }
}
akurasov commented 3 years ago

Hi! Is it just simple window like singleWindowApplication or you applied some modifiers to it? Could you share your code, please?

igordmn commented 3 years ago

It looks like, you run ./gradlew SplitPane:demo:run in components folder?

ruckustboom commented 3 years ago

Not quite. I was trying to make my own minimal split pane (based on the one in the widgets gallery example).

ruckustboom commented 2 years ago

This is still an issue with today's beta1 build.

igordmn commented 2 years ago

Can you check 1.0.0-beta3? There was a fix for this issue.

I am partially reproduced it on Windows, but was not able to reproduce it on Linux, so I am not sure if it is fully fixed.

ruckustboom commented 2 years ago

Yes, with beta3 the glitching is completely gone (at least in this quick test). Awesome!

There does appear to be a different (hopefully minor) issue. The initial 100dp width of the blue box is no longer respected. Resizing it works fine, but it starts initially with no width. Is there a chance I'm doing the measuring wrong?

igordmn commented 2 years ago

It seems now we have an necessary layout pass at the start, with a zero container size. Thanks for noticing!

It isn't a real bug, but we will get rid of this in the future versions, as it affects performance, and it is a weird behavior.

Anyway, no matter how many layouts are happen, ideally your code shouldn't rely on it. You coerces expected to the initial size (in our case to zero) in the first pass, so it will never be greater after that.

P.S. You code also has a side effect inside a layout pass:

        if (!state.isResizing) {
            val expected = state.size.coerceIn(0.dp, constraints.maxWidth.toDp())
            if (expected != state.size) state.size = expected
        }

Better to write code in Composable/layout code in a way, that it doesn't change state that is read before that code, as it will cause recomposition/relayout.

ruckustboom commented 2 years ago

Ah, makes sense. Good to know.

Yeah, the code isn't great. I'm still new to reactive programming in general (I primarily use JavaFX for desktop currently). Thanks for the tip!

ruckustboom commented 2 years ago

Should I close the issue, or will it be closed as part of the beta3 "release"?

igordmn commented 2 years ago

let's close it, as beta3 is released. there is no github release because we have few irrelevant test failures

okushnikov commented 2 months ago

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