JetBrains / skiko

Kotlin Multiplatform bindings to Skia
Apache License 2.0
1.83k stars 118 forks source link

Get rid of input lag in Compose for Desktop #64

Closed igordmn closed 1 month ago

igordmn commented 3 years ago

Currently after we move/click mouse, press key on the keyboard, the content will change not on the next display vsync signal, but on the second next vsync signal.

So, the actual time between input and picture change can be 16.6 + 14ms (for example).

Single 30ms lag isn't noticeable, but if we constantly draw behind mouse input, we can notice this.

For example, check this code in Compose for Desktop (dragging a box with mouse):

import androidx.compose.desktop.Window
import androidx.compose.foundation.background
import androidx.compose.foundation.gestures.awaitFirstDown
import androidx.compose.foundation.gestures.drag
import androidx.compose.foundation.gestures.forEachGesture
import androidx.compose.foundation.layout.*
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.round

fun main() = Window {
    var offset by remember { mutableStateOf(Offset.Zero) }

    Box(Modifier.fillMaxSize()) {
        Box(
            Modifier.offset { offset.round() }
                .size(100.dp)
                .background(Color.Red)
                .pointerInput(Unit) {
                    forEachGesture {
                        awaitPointerEventScope {
                            val down = awaitFirstDown()
                            drag(down.id) {
                                offset += (it.position - it.previousPosition)
                            }
                        }
                    }
                }
        )
    }
}

When we drag the box, we can notice that the box is always behind the cursor. It is because when we move cursor, it will update its position on the next vsync, and the box will update its position on the second next vsync.

When we disable systemProperty("skiko.vsync.enabled", "false"), then we can see that the lag is much lower (but still exist though, don't know why yet).

One possible solution - provide two functions needsRedrawImmediately/needsRedraw instead of single needsRedraw. needsRedraw will be called in compose if there are any running animations (frameClock.hasAwaiters), needsRedrawImmediately - if there are any changes after any input event or after frame (list.any(DesktopOwner::needsRender))

igordmn commented 3 years ago

There is also AWTDebounceEventQueue

Probably we don't need it after this fix (not sure)

tonsky commented 3 years ago

but still exist though, don't know why yet

AFAIK there’s a special path in all OSes to render mouse cursor as fast as possible. No matter what you do, software cursor will always lag behind the hardware one. Good read here https://www.vsynctester.com/manual.html (scroll to “3. Mouse 'input lag' detector”)

tonsky commented 3 years ago

BTW Safari does 1 frame for me, while Firefox does two. So 1 frame is possible (and worth fighting for), 0 probably not

pusolito commented 3 years ago

Any concrete plans for this yet?

MohamedRejeb commented 6 months ago

Any updates on this, I'm having a similar issue with drawing. Here's a comparison between a Skiko and Swing implementations:

https://github.com/JetBrains/skiko/assets/41842296/395f4f8a-ee41-4fd7-9ef5-00eadbf4693d

Compose: image

Swing: image

Disabling vsync makes things better, but still having some lag.

tonsky commented 6 months ago

@MohamedRejeb can you share this drawing demo app?

MohamedRejeb commented 6 months ago

@tonsky Sure, here's the demo. I'm using Compose not skiko in this demo, but the results are the same. You can notice the difference clearly when vsync is enabled. Disabling vsync makes things better, but the latency is still there.

MohamedRejeb commented 6 months ago

I was planning to check it with skija, but I didn't have the chance yet.

tonsky commented 6 months ago

I don’t see a link, maybe you forgot to add it? I was planning to do Skija/JWM test myself

MohamedRejeb commented 6 months ago

Sorry, here is the demo

tonsky commented 6 months ago

No permission

MohamedRejeb commented 6 months ago

Oh sorry again haha it should be fine now

tonsky commented 6 months ago

So Skija + JWM seem to be doing okay

https://github.com/JetBrains/skiko/assets/285292/4cb9303a-c183-41c5-8f39-8cd4f598bb72

Red square: software mouse cursor Red circle: 1-frame area around software mouse cursor with the radius of current cursor speed. Useful to detect a lag between software cursor and hardware cursor

Graph at the bottom:

If you want to try, check out this commit https://github.com/HumbleUI/HumbleUI/commit/4a994b9a9d1790d38e1c0fd17da74d184f41f017 and run it with ./script/repl.py.

m-sasha commented 6 months ago

Skiko, on macOS, appears to have a two-frame delay between mouse-move and its result being visible on screen. Here's a reproducer:

fun main() {
    val path = Path()
    val skiaLayer = SkiaLayer()
    skiaLayer.renderDelegate = object : SkikoRenderDelegate {
        val paint = Paint().apply {
            color = org.jetbrains.skia.Color.BLACK
            strokeWidth = 5f
            strokeCap = PaintStrokeCap.ROUND
            strokeJoin = PaintStrokeJoin.ROUND
            mode = PaintMode.STROKE
        }
        override fun onRender(canvas: Canvas, width: Int, height: Int, nanoTime: Long) {
            val contentScale = skiaLayer.contentScale
            canvas.scale(contentScale, contentScale)
            canvas.drawPath(
                path = path,
                paint = paint
            )
            val lastPt = path.lastPt
            if (lastPt.x != 0f) {
                canvas.drawCircle(x = lastPt.x, y = lastPt.y, radius = 6f, paint = paint)
            }
        }
    }

    SwingUtilities.invokeLater {
        val window = JFrame("Skiko example").apply {
            defaultCloseOperation = WindowConstants.EXIT_ON_CLOSE
            preferredSize = Dimension(800, 600)
        }
        skiaLayer.attachTo(window.contentPane)
        skiaLayer.needRedraw()
        skiaLayer.addMouseListener(object: MouseAdapter() {
            override fun mousePressed(e: MouseEvent) {
                path.reset()
                path.moveTo(e.x.toFloat(), e.y.toFloat())
                skiaLayer.needRedraw()
            }

            override fun mouseReleased(e: MouseEvent) {
                path.reset()
                skiaLayer.needRedraw()
            }
        })
        skiaLayer.addMouseMotionListener(object: MouseAdapter() {
            override fun mouseDragged(e: MouseEvent) {
                path.lineTo(e.x.toFloat(), e.y.toFloat())
                skiaLayer.needRedraw()
            }
        })
        window.pack()
        window.isVisible = true
    }
}

https://github.com/JetBrains/skiko/assets/5230206/78b8543c-73b1-4b7c-9047-4292b2fd8260

The delay appears to be significantly reduced when using skiko.vsync.enabled=false:

https://github.com/JetBrains/skiko/assets/5230206/8baa09d1-c815-44dc-8bc2-c3d20d03305b

m-sasha commented 6 months ago

The culprit appears to be the setting of MetalDevice.layer.displaySyncEnabled to true in MetalRedrawer.mm. With only this set to false (while keeping vsyncEnabled=true in skiko), the delay is reduced. This is consistent with the documentation: https://developer.apple.com/documentation/quartzcore/cametallayer/2887087-displaysyncenabled?language=objc

@igordmn @elijah-semyonov

elijah-semyonov commented 6 months ago

@m-sasha Frankly speaking, I can assume as much as you do here judging by the documentation. This flag is unique to macOS and is poorly documented/explained.

So I guess, the answers to your question are:

Could we always use MetalDevice.layer.displaySyncEnabled=false.

Yes, we could. Perhaps we need to add desktop specific API to make this behavior optional in case it's undesirable due to tearing? (make it window-wise, dynamically configurable based on the app needs?)

Will we have tearing if we do?

According to documentation we will occasionally have it.

If not, could we make this a separate setting from skiko.vsync.enabled (with the default being set to the value of skiko.vsync.enabled)?

That's kind of a thing we discussed with @igordmn almost a year ago, that vsync is an extremely overloaded term. What do we want to separate exactly? In this context this flag is a hint toward the macOS internals on how to schedule the drawable presentation. It's seems to be the only thing (can you verify this? I didn't find any other usage of this flag) that is controlled by this property on MacOS.

m-sasha commented 6 months ago

That's kind of a thing we discussed with @igordmn almost a year ago, that vsync is an extremely overloaded term. What do we want to separate exactly? In this context this flag is a hint toward the macOS internals on how to schedule the drawable presentation. It's seems to be the only thing (can you verify this? I didn't find any other usage of this flag) that is controlled by this property on MacOS.

I assumed skiko.vsync.enabled controlled two things:

  1. The setting of MetalDevice.layer.displaySyncEnabled
  2. Whether we throttle drawing to the refresh rate.

It turns out I was wrong about (2). In MetalRedrawer we throttle drawing to the refresh rate even when skiko.vsync.enabled=false, via

            displayLinkThrottler.waitVSync(windowPtr = layer.windowHandle)

@igordm, is that what we want? I believe on other platforms, setting skiko.vsync.enabled=false allows drawing to happen faster than the refresh rate.

But given this, could we still have tearing if we set MetalDevice.layer.displaySyncEnabled=false?

m-sasha commented 6 months ago

Perhaps we could implement double-buffering ourselves, to prevent tearing.

okushnikov commented 1 month ago

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