Add constructor with `RenderSettings` to `ComposePanel` #1377

elijah-semyonov · 2024-05-28T08:58:10Z

Fixes JetBrains/compose-multiplatform#4870

Release Notes

Features - Desktop

Add constructor with RenderSettings to ComposePanel. Added a class RenderSettings with val isVsyncEnabled: Boolean?. When set to true gives a hint to renderer implementation of the particular ComposePanel to reduce the latency between the input and visual changes in exchange for possible screen tearing.

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt

m-sasha · 2024-05-28T09:13:09Z

While I don't think this change is bad in and of itself (it's good to give more granularity to the vsync flag), I don't like how this is being used as a workaround for the presentation latency issue. It solves a very small aspect of the problem, and in a very inconvenient way.

It doesn't solve the issue for fully Compose (no Swing) apps.
It doesn't solve the issue for regular UI, where tearing looks bad.

I'd much rather we solved the underlying issue; if Swing/AWT can do it, why not us?

m-sasha · 2024-05-28T09:15:47Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt

@@ -46,8 +47,29 @@ import org.jetbrains.skiko.SkiaLayerAnalytics
 class ComposePanel @ExperimentalComposeUiApi constructor(
    private val skiaLayerAnalytics: SkiaLayerAnalytics,
 ) : JLayeredPane() {
+    private var reducePresentationLatency: Boolean? = null


Agree with Ivan that vsyncEnabled is better. reducePresentationLatency doesn't hint at any downsides - why would anyone not want to reduce presentation latency?

But maybe consider going even farther and give it a (fake) enum, e.g. PresentationMode.VSYNC, PresentationMode.IMMEDIATE etc, if there's a chance we could have more modes...

igordmn · 2024-05-28T09:24:04Z

I'd much rather we solved the underlying issue; if Swing/AWT can do it, why not us?

I would too, but as we discussed, it is much harder, and I am not sure we are able to solve it in the foreseeble future. In the meantime, we can provide a property, that even makes sense outside of this issue.

igordmn · 2024-05-28T09:35:42Z

if Swing/AWT can do it, why not us?

If we just disable vsync, we will have:

tearing (depends on the system, the system window compositor can add additional synchronization)
animation issues
- frame drops
- inconsistent animation frame times (animations won't be smooth)
- unlimited FPS (consumes CPU)
too early recomposition, that results the frame after the next to drop

Swing can have the same issues.

m-sasha · 2024-05-28T09:44:29Z

If we just disable vsync, we will have:

Are there no other solutions?

Swing can have the same issues.

But in practice, it doesn't. Swing has low-latency presentation and no tearing out-of-the-box.

igordmn · 2024-05-28T10:09:23Z

Are there no other solutions?

This is the nature of vsync. If we sync with it, we have 1-frame latency. We sync with it to solve the issues I mentioned.

Animations should always be in sync with v-sync, to avoid them. The solution might be to move input reaction changes out of vsync, but it can be not trivial. We might even need a new API that user should call explicitly, not sure.

igordmn · 2024-05-28T10:11:44Z

We also might have unnecessary frame lags in other places. Most probably it is easier to fix them than the vsync latency. But we need to profile the issue to understand if we have them.

m-sasha · 2024-05-28T10:17:16Z

This is the nature of vsync.

But, again, why is this problem not present in Swing, and (I assume) native frameworks?

Can we double buffer ourselves to avoid tearing?

We also might have unnecessary frame lags in other places.

The problem reproduces in pure Skiko, and it appears that most (possibly all) the delay comes from vsync.

igordmn · 2024-05-28T10:33:37Z

But, again, why is this problem not present in Swing, and (I assume) native frameworks?

Besides tearing I mentioned the animation issue, which Swing didn't solve. It doesn't sync it with vsync. I haven't investigated it properly in the past though. I discovered this when I tried to implement smooth animations on Swing, and didn't find a way.

I am not saying that there is no proper way, I am saying that a proper way needs time to investigate/design/implement. And I am all for doing that.

The problem reproduces in pure Skiko

What I meant that we can have issues in Compose, and solving them might also help. Skiko also can have additional lags besides that caused by vsync.

igordmn · 2024-05-28T10:43:16Z

But, again, why is this problem not present in Swing, and (I assume) native frameworks?

I would look at the Android sources (Choreographer). From my experience, it solves the latency and the animation issues. As I remember, it was introduced as Project Butter.

m-sasha · 2024-05-28T10:45:05Z

I am not saying that there is no proper way, I am saying that a proper way needs time to investigate/design/implement. And I am all for doing that.

Ok, understood.

What I meant that we can have issues in Compose, and solving them might also help. Skiko also can have additional lags besides that caused by vsync.

The experiments I've done show that this isn't the case. Or at least the contribution of other factors (in Skiko or Compose) to the latency are minimal.

The latency in a pure Skiko reproducer is very similar to the latency of a Compose reproducer.
The latency in pure Skiko with vsync turned off is negligible/acceptable - probably just the remaining, expected 1-frame delay.

m-sasha · 2024-05-28T11:00:58Z

Note that merely setting

device.layer.displaySyncEnabled = NO;

while keeping everything else related to vsync (e.g. calling DisplayLinkThrottler.waitVSync) also causes the problem to go away (but causes tearing).

elijah-semyonov · 2024-05-28T13:03:51Z

Note that merely setting
device.layer.displaySyncEnabled = NO;
while keeping everything else related to vsync (e.g. calling DisplayLinkThrottler.waitVSync) also causes the problem to go away (but causes tearing).

Yes, that's true. The introduced flag doesn't affect throttler behavior.

elijah-semyonov · 2024-05-28T13:04:59Z

Thank you for feedback and discussion, I'll do deeper investigation of other graphics backends and see if I can define a common denominator here.

MohamedRejeb · 2024-05-31T22:44:32Z

I noticed that even with vsync disabled, you can get better results with requesting redraw only 1 time per frame.
I'm on 60 fps so the frame duration is around 16ms. Calling skiaLayer.needRedraw() on each skiko onRender() call results calling onRender() each 8ms (2 times per frame). I did it manually and I called skiaLayer.needRedraw() each 16 ms and the results was better.
I'm not sure if there's a way to prevent calling draw method more than one time on each frame even before reaching the native calls, maybe it can be done on the FrameDispatcher.

elijah-semyonov · 2024-06-11T09:14:31Z

I noticed that even with vsync disabled, you can get better results with requesting redraw only 1 time per frame. I'm on 60 fps so the frame duration is around 16ms. Calling skiaLayer.needRedraw() on each skiko onRender() call results calling onRender() each 8ms (2 times per frame). I did it manually and I called skiaLayer.needRedraw() each 16 ms and the results was better. I'm not sure if there's a way to prevent calling draw method more than one time on each frame even before reaching the native calls, maybe it can be done on the FrameDispatcher.

Which platform did you test it on?
It doesn't seem a valid behavior on macOS. We already use framerate throttling disregarding the vsyncEnabled (which control the presentation, not scheduling)

.../ui/src/desktopMain/kotlin/androidx/compose/ui/scene/skia/SwingSkiaLayerComponent.desktop.kt

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt

elijah-semyonov · 2024-06-12T10:37:43Z

There is quite a lot of effects for fully exposing SkikoLayerProperties, so @igordmn and me decided to start small and fix the immediate issue with latency on macOS being a blocker for some users.

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/RenderingSettings.desktop.kt

MatkovIvan · 2024-06-12T10:59:59Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/RenderingSettings.desktop.kt

+ * by Swing.
+ */
+@ExperimentalComposeUiApi
+class RenderingSettings(


Should we add equals/hashCode to it?

Nop, we don't do any comparisons/caching/using it as a key, etc.

MatkovIvan · 2024-06-12T11:00:35Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeContainer.desktop.kt

@@ -78,6 +79,7 @@ import org.jetbrains.skiko.SkiaLayerAnalytics
 *  for window coordinate space.
 * @property useSwingGraphics Flag indicating if offscreen rendering to Swing graphics is used.
 * @property layerType The type of layer used for Popup/Dialog.
+ * @property renderingSettings The settings for rendering.


Add a note that it will work only with useSwingGraphics == true

It's in the doc of RenderSettings.

In general it's not true, because we can add settings, that will work with swing

MatkovIvan · 2024-06-12T11:01:14Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeContainer.desktop.kt

@@ -338,7 +341,13 @@ internal class ComposeContainer(
        return if (useSwingGraphics) {
            SwingSkiaLayerComponent(mediator, renderDelegate, skiaLayerAnalytics)
        } else {
-            WindowSkiaLayerComponent(mediator, windowContext, renderDelegate, skiaLayerAnalytics)
+            WindowSkiaLayerComponent(
+                mediator,


Add explicit parameter names if it's not one-liner

Do we add them when variables match parameter names or are obvious?

It's better to add it to be on the safe side during possible refactoring

MatkovIvan · 2024-06-12T11:02:54Z

...ui/src/desktopMain/kotlin/androidx/compose/ui/scene/skia/WindowSkiaLayerComponent.desktop.kt

+        properties = run {
+            val defaultProperties = SkiaLayerProperties()
+
+            SkiaLayerProperties(
+                isVsyncEnabled = renderingSettings.isVsyncEnabled ?: defaultProperties.isVsyncEnabled,
+            )
+        },


Suggested change

properties = run {

val defaultProperties = SkiaLayerProperties()

SkiaLayerProperties(

isVsyncEnabled = renderingSettings.isVsyncEnabled ?: defaultProperties.isVsyncEnabled,

)

},

properties = SkiaLayerProperties(

isVsyncEnabled = renderingSettings.isVsyncEnabled ?: SkiaLayerProperties.Default.isVsyncEnabled,

),

There can be multiple properties in future, this approach will reconstruct SkiaLayerProperties every single time. There is no Default in SkiaLayerProperties.Companion

…enderingSettings.desktop.kt Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>

This reverts commit d242e85.

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt

…omposePanel.desktop.kt Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>

m-sasha · 2024-06-13T09:48:50Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/RenderingSettings.desktop.kt

+ */
+@ExperimentalComposeUiApi
+class RenderingSettings(
+    val isVsyncEnabled: Boolean?


It's better/cleaner to put this in a (fake) enum, e.g. VSyncMode.Enabled, VSyncMode.Disabled, and VsyncMode.Default. Although maybe it's overkill here; not sure.

I'd leave it as it is now (ExperimentalAPI after all), I haven't yet performed an investigation about different presentation modes on non-Metal rendering backends.

m-sasha · 2024-06-13T09:57:03Z

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/RenderingSettings.desktop.kt

+ * by Swing.
+ */
+@ExperimentalComposeUiApi
+class RenderingSettings(


Maybe RenderSettings instead? A bit shorter and doesn't lose any meaning.

Yeah, sounds fine

MohamedRejeb · 2024-06-13T10:35:49Z

Btw the latency issue exists on windows as well (didn't try linux) and disabling vsync reduces the latency significantly there. So it's not only a macos thing.

igordmn · 2024-07-01T14:06:43Z

@elijah-semyonov, please actualise Release Notes

elijah-semyonov requested review from MatkovIvan and igordmn May 28, 2024 08:58

elijah-semyonov self-assigned this May 28, 2024

MatkovIvan added the changes in API label May 28, 2024

MatkovIvan requested a review from m-sasha May 28, 2024 09:00

MatkovIvan reviewed May 28, 2024

View reviewed changes

m-sasha reviewed May 28, 2024

View reviewed changes

elijah-semyonov marked this pull request as draft May 28, 2024 13:04

elijah-semyonov force-pushed the es/desktop-reduce-latency branch from 5e5f3f2 to f17214c Compare June 11, 2024 10:28

MatkovIvan requested changes Jun 11, 2024

View reviewed changes

.../ui/src/desktopMain/kotlin/androidx/compose/ui/scene/skia/SwingSkiaLayerComponent.desktop.kt Outdated Show resolved Hide resolved

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt Outdated Show resolved Hide resolved

elijah-semyonov added 2 commits June 12, 2024 11:38

Add RenderingSettings

dee558d

Wire rendering settings

22022fa

elijah-semyonov force-pushed the es/desktop-reduce-latency branch from e9ec093 to 22022fa Compare June 12, 2024 10:28

Fix compilation error

fc70078

elijah-semyonov requested a review from MatkovIvan June 12, 2024 10:36

elijah-semyonov requested a review from m-sasha June 12, 2024 10:36

elijah-semyonov marked this pull request as ready for review June 12, 2024 10:36

elijah-semyonov added 2 commits June 12, 2024 12:43

Bring the question mark back

1b08c88

Remove redundant experimental constructor

eaa47d4

MatkovIvan reviewed Jun 12, 2024

View reviewed changes

elijah-semyonov and others added 4 commits June 12, 2024 13:04

Update compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/R…

6fce56e

…enderingSettings.desktop.kt Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>

Fix AccessibleContext

d242e85

Revert "Fix AccessibleContext"

96182de

This reverts commit d242e85.

Bump skiko

b25b1a8

elijah-semyonov changed the title ~~Add constructor with reducePresentationLatency to ComposePanel~~ Add constructor with RenderSettings to ComposePanel Jun 12, 2024

Dump api

12b8b9e

igordmn approved these changes Jun 12, 2024

View reviewed changes

Fix test compilation

6decc01

MatkovIvan reviewed Jun 13, 2024

View reviewed changes

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposePanel.desktop.kt Outdated Show resolved Hide resolved

Update compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/C…

b76273e

…omposePanel.desktop.kt Co-authored-by: Ivan Matkov <ivan.matkov@jetbrains.com>

m-sasha approved these changes Jun 13, 2024

View reviewed changes

m-sasha reviewed Jun 13, 2024

View reviewed changes

elijah-semyonov added 2 commits June 13, 2024 13:52

Rename 'RenderingSettings' to 'RenderSettings'

68d5c9f

Dump API

561d418

MatkovIvan approved these changes Jun 14, 2024

View reviewed changes

Bump Skiko

66445e7

elijah-semyonov merged commit 03d732f into jb-main Jun 14, 2024
8 checks passed

elijah-semyonov deleted the es/desktop-reduce-latency branch June 14, 2024 10:37

igordmn mentioned this pull request Jul 1, 2024

Add 1.7.0-alpha01 to changelog JetBrains/compose-multiplatform#5052

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add constructor with `RenderSettings` to `ComposePanel` #1377

Add constructor with `RenderSettings` to `ComposePanel` #1377

elijah-semyonov commented May 28, 2024 •

edited

Loading

m-sasha commented May 28, 2024

m-sasha May 28, 2024

igordmn commented May 28, 2024

igordmn commented May 28, 2024 •

edited

Loading

m-sasha commented May 28, 2024 •

edited

Loading

igordmn commented May 28, 2024

igordmn commented May 28, 2024

m-sasha commented May 28, 2024

igordmn commented May 28, 2024

igordmn commented May 28, 2024 •

edited

Loading

m-sasha commented May 28, 2024

m-sasha commented May 28, 2024

elijah-semyonov commented May 28, 2024

elijah-semyonov commented May 28, 2024

MohamedRejeb commented May 31, 2024 •

edited

Loading

elijah-semyonov commented Jun 11, 2024 •

edited

Loading

elijah-semyonov commented Jun 12, 2024

MatkovIvan Jun 12, 2024

elijah-semyonov Jun 12, 2024

MatkovIvan Jun 12, 2024

elijah-semyonov Jun 12, 2024 •

edited

Loading

elijah-semyonov Jun 12, 2024

MatkovIvan Jun 12, 2024

elijah-semyonov Jun 12, 2024

MatkovIvan Jun 12, 2024

MatkovIvan Jun 12, 2024

elijah-semyonov Jun 12, 2024

m-sasha Jun 13, 2024

elijah-semyonov Jun 13, 2024

m-sasha Jun 13, 2024

elijah-semyonov Jun 13, 2024

MohamedRejeb commented Jun 13, 2024

igordmn commented Jul 1, 2024

Add constructor with RenderSettings to ComposePanel #1377

Add constructor with RenderSettings to ComposePanel #1377

Conversation

elijah-semyonov commented May 28, 2024 • edited Loading

Release Notes

Features - Desktop

m-sasha commented May 28, 2024

Choose a reason for hiding this comment

igordmn commented May 28, 2024

igordmn commented May 28, 2024 • edited Loading

m-sasha commented May 28, 2024 • edited Loading

igordmn commented May 28, 2024

igordmn commented May 28, 2024

m-sasha commented May 28, 2024

igordmn commented May 28, 2024

igordmn commented May 28, 2024 • edited Loading

m-sasha commented May 28, 2024

m-sasha commented May 28, 2024

elijah-semyonov commented May 28, 2024

elijah-semyonov commented May 28, 2024

MohamedRejeb commented May 31, 2024 • edited Loading

elijah-semyonov commented Jun 11, 2024 • edited Loading

elijah-semyonov commented Jun 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elijah-semyonov Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MohamedRejeb commented Jun 13, 2024

igordmn commented Jul 1, 2024

Add constructor with `RenderSettings` to `ComposePanel` #1377

Add constructor with `RenderSettings` to `ComposePanel` #1377

elijah-semyonov commented May 28, 2024 •

edited

Loading

igordmn commented May 28, 2024 •

edited

Loading

m-sasha commented May 28, 2024 •

edited

Loading

igordmn commented May 28, 2024 •

edited

Loading

MohamedRejeb commented May 31, 2024 •

edited

Loading

elijah-semyonov commented Jun 11, 2024 •

edited

Loading

elijah-semyonov Jun 12, 2024 •

edited

Loading