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

Add constructor with RenderSettings to ComposePanel #1377

Merged
merged 15 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion compose/ui/ui/api/desktop/ui.api
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,8 @@ public final class androidx/compose/ui/awt/ComposeDialog : javax/swing/JDialog {
public final class androidx/compose/ui/awt/ComposePanel : javax/swing/JLayeredPane {
public static final field $stable I
public fun <init> ()V
public fun <init> (Lorg/jetbrains/skiko/SkiaLayerAnalytics;)V
public fun <init> (Lorg/jetbrains/skiko/SkiaLayerAnalytics;Landroidx/compose/ui/awt/RenderSettings;)V
public synthetic fun <init> (Lorg/jetbrains/skiko/SkiaLayerAnalytics;Landroidx/compose/ui/awt/RenderSettings;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun add (Ljava/awt/Component;)Ljava/awt/Component;
public fun addFocusListener (Ljava/awt/event/FocusListener;)V
public fun addNotify ()V
Expand Down Expand Up @@ -528,6 +529,17 @@ public final class androidx/compose/ui/awt/ComposeWindow : javax/swing/JFrame {
public fun setUndecorated (Z)V
}

public final class androidx/compose/ui/awt/RenderSettings {
public static final field $stable I
public static final field Companion Landroidx/compose/ui/awt/RenderSettings$Companion;
public fun <init> (Ljava/lang/Boolean;)V
public final fun isVsyncEnabled ()Ljava/lang/Boolean;
}

public final class androidx/compose/ui/awt/RenderSettings$Companion {
public final fun getDefault ()Landroidx/compose/ui/awt/RenderSettings;
}

public final class androidx/compose/ui/awt/SwingPanel_desktopKt {
public static final fun SwingPanel-euL9pac (JLkotlin/jvm/functions/Function0;Landroidx/compose/ui/Modifier;Lkotlin/jvm/functions/Function1;Landroidx/compose/runtime/Composer;II)V
public static final fun getNoOpUpdate ()Lkotlin/jvm/functions/Function1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@ import org.jetbrains.skiko.SkiaLayerAnalytics
* @param skiaLayerAnalytics Analytics that helps to know more about SkiaLayer behaviour.
* SkiaLayer is underlying class used internally to draw Compose content.
* Implementation usually uses third-party solution to send info to some centralized analytics gatherer.
* @param renderSettings Configuration class for rendering settings.
*/
class ComposePanel @ExperimentalComposeUiApi constructor(
private val skiaLayerAnalytics: SkiaLayerAnalytics,
private val renderSettings: RenderSettings = RenderSettings.Default
) : JLayeredPane() {
constructor() : this(SkiaLayerAnalytics.Empty)
constructor() : this(SkiaLayerAnalytics.Empty, RenderSettings.Default)

init {
check(isEventDispatchThread()) {
Expand Down Expand Up @@ -196,7 +198,8 @@ class ComposePanel @ExperimentalComposeUiApi constructor(
return ComposeContainer(
container = this,
skiaLayerAnalytics = skiaLayerAnalytics,
windowContainer = windowContainer
windowContainer = windowContainer,
renderSettings = renderSettings,
).apply {
focusManager.releaseFocus()
setBounds(0, 0, width, height)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ internal class ComposeWindowPanel(
// but it's always disabled here. Using fallback instead of [check] to support
// opening separate windows from [ComposePanel] with such layer type.
if (it == LayerType.OnComponent) LayerType.OnSameCanvas else it
}
},
// TODO: Add RenderingSettings to ComposeWindowPanel constructor
renderSettings = RenderSettings.Default
)
private val composeContainer
get() = requireNotNull(_composeContainer) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2024 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package androidx.compose.ui.awt

import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.ComposeFeatureFlags
import org.jetbrains.skiko.SkikoProperties

/**
* Configuration class for rendering settings.
*
* @property isVsyncEnabled Indicates whether presentation vsync is enabled or not
* When true, the internal implementation will attempt to synchronize drawable presentations
* with vsync. It guarantees that the frame is presented without visual artifacts like tearing in
* exchange for a latency increase.
* When false, the internal implementation will not attempt to synchronize drawable presentations
* it can reduce latency but may introduce visual artifacts like screen tearing.
* When null, the internal implementation will use the global configuration from [SkikoProperties.vsyncEnabled]
*
* This flag has no effect if [ComposeFeatureFlags.useSwingGraphics] is true. In this case Compose
* will render the content to Swing provided offscreen buffer and the presentation will be controlled
* by Swing.
*/
@ExperimentalComposeUiApi
class RenderSettings(
val isVsyncEnabled: Boolean?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

) {
companion object {
/**
* Default rendering settings
*/
val Default = RenderSettings(
isVsyncEnabled = null
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.compose.ui.LayerType
import androidx.compose.ui.awt.AwtEventFilter
import androidx.compose.ui.awt.AwtEventListener
import androidx.compose.ui.awt.AwtEventListeners
import androidx.compose.ui.awt.RenderSettings
import androidx.compose.ui.input.key.KeyEvent
import androidx.compose.ui.platform.LocalLifecycleOwner
import androidx.compose.ui.platform.LocalInternalViewModelStoreOwner
Expand Down Expand Up @@ -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 renderSettings The settings for rendering.
*/
internal class ComposeContainer(
val container: JLayeredPane,
Expand All @@ -88,6 +90,7 @@ internal class ComposeContainer(

private val useSwingGraphics: Boolean = ComposeFeatureFlags.useSwingGraphics,
private val layerType: LayerType = ComposeFeatureFlags.layerType,
private val renderSettings: RenderSettings,
) : WindowFocusListener, WindowListener, LifecycleOwner, ViewModelStoreOwner {
val windowContext = PlatformWindowContext()
var window: Window? = null
Expand Down Expand Up @@ -338,7 +341,13 @@ internal class ComposeContainer(
return if (useSwingGraphics) {
SwingSkiaLayerComponent(mediator, renderDelegate, skiaLayerAnalytics)
} else {
WindowSkiaLayerComponent(mediator, windowContext, renderDelegate, skiaLayerAnalytics)
WindowSkiaLayerComponent(
mediator,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

windowContext,
renderDelegate,
skiaLayerAnalytics,
renderSettings
)
}
}

Expand Down Expand Up @@ -381,7 +390,8 @@ internal class ComposeContainer(
density = density,
layoutDirection = layoutDirection,
focusable = focusable,
compositionContext = compositionContext
compositionContext = compositionContext,
renderSettings = renderSettings
)
LayerType.OnComponent -> SwingComposeSceneLayer(
composeContainer = this,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package androidx.compose.ui.scene

import org.jetbrains.skia.Rect as SkRect
import androidx.compose.runtime.CompositionContext
import androidx.compose.ui.awt.RenderSettings
import androidx.compose.ui.awt.getTransparentWindowBackground
import androidx.compose.ui.awt.setTransparent
import androidx.compose.ui.awt.toAwtRectangle
Expand Down Expand Up @@ -52,7 +53,8 @@ internal class WindowComposeSceneLayer(
density: Density,
layoutDirection: LayoutDirection,
focusable: Boolean,
compositionContext: CompositionContext
compositionContext: CompositionContext,
private val renderSettings: RenderSettings
) : DesktopComposeSceneLayer(composeContainer, density, layoutDirection) {
private val window get() = requireNotNull(composeContainer.window)
private val windowContext = PlatformWindowContext().also {
Expand Down Expand Up @@ -193,7 +195,8 @@ internal class WindowComposeSceneLayer(
mediator = mediator,
windowContext = windowContext,
renderDelegate = renderDelegate,
skiaLayerAnalytics = skiaLayerAnalytics
skiaLayerAnalytics = skiaLayerAnalytics,
renderSettings = renderSettings,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package androidx.compose.ui.scene.skia

import androidx.compose.ui.awt.RenderSettings
import androidx.compose.ui.platform.PlatformWindowContext
import androidx.compose.ui.scene.ComposeSceneMediator
import java.awt.Dimension
Expand All @@ -24,6 +25,7 @@ import javax.accessibility.Accessible
import org.jetbrains.skiko.GraphicsApi
import org.jetbrains.skiko.SkiaLayer
import org.jetbrains.skiko.SkiaLayerAnalytics
import org.jetbrains.skiko.SkiaLayerProperties
import org.jetbrains.skiko.SkikoRenderDelegate

/**
Expand All @@ -36,7 +38,8 @@ internal class WindowSkiaLayerComponent(
private val mediator: ComposeSceneMediator,
private val windowContext: PlatformWindowContext,
renderDelegate: SkikoRenderDelegate,
skiaLayerAnalytics: SkiaLayerAnalytics
skiaLayerAnalytics: SkiaLayerAnalytics,
private val renderSettings: RenderSettings,
) : SkiaLayerComponent {
/**
* See also backend layer for swing interop in [SwingSkiaLayerComponent]
Expand All @@ -47,6 +50,13 @@ internal class WindowSkiaLayerComponent(
// apply `checkNotNull` for "non-null" field.
checkNotNull(mediator.accessible)
},
properties = run {
val defaultProperties = SkiaLayerProperties()

SkiaLayerProperties(
isVsyncEnabled = renderSettings.isVsyncEnabled ?: defaultProperties.isVsyncEnabled,
)
},
analytics = skiaLayerAnalytics
) {
override fun paint(g: Graphics) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,10 @@ class ComposePanelTest {
}
}

val composePanel = ComposePanel(skiaLayerAnalytics = analytics)
val composePanel = ComposePanel(
skiaLayerAnalytics = analytics,
renderSettings = RenderSettings.Default
)
composePanel.size = Dimension(100, 100)

val frame = JFrame()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
package androidx.compose.ui.window

import androidx.compose.ui.assertThat
import androidx.compose.ui.awt.RenderSettings
import androidx.compose.ui.isEqualTo
import androidx.compose.ui.scene.ComposeContainer
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleEventObserver
import androidx.lifecycle.LifecycleOwner
import java.awt.Frame
import java.awt.Toolkit
import java.time.Duration
import javax.swing.JFrame
import javax.swing.JLayeredPane
Expand Down Expand Up @@ -142,7 +141,8 @@ class ComposeContainerLifecycleOwnerTest {
container = ComposeContainer(
container = pane,
skiaLayerAnalytics = SkiaLayerAnalytics.Empty,
window = window
window = window,
renderSettings = RenderSettings.Default
).also {
it.lifecycle.addObserver(allEvents)
}
Expand Down Expand Up @@ -172,7 +172,8 @@ class ComposeContainerLifecycleOwnerTest {
container = ComposeContainer(
container = pane,
skiaLayerAnalytics = SkiaLayerAnalytics.Empty,
window = window
window = window,
renderSettings = RenderSettings.Default
).also {
it.lifecycle.addObserver(allEvents)
}
Expand Down Expand Up @@ -202,7 +203,8 @@ class ComposeContainerLifecycleOwnerTest {
container = ComposeContainer(
container = pane,
skiaLayerAnalytics = SkiaLayerAnalytics.Empty,
window = window
window = window,
renderSettings = RenderSettings.Default
).also {
it.lifecycle.addObserver(allEvents)
}
Expand Down Expand Up @@ -250,7 +252,8 @@ class ComposeContainerLifecycleOwnerTest {
container = ComposeContainer(
container = this,
skiaLayerAnalytics = SkiaLayerAnalytics.Empty,
window = window
window = window,
renderSettings = RenderSettings.Default
)
container.lifecycle.addObserver(observer)
window.add(this)
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ moshi = "1.13.0"
protobuf = "3.21.8"
paparazzi = "1.0.0"
paparazziNative = "2022.1.1-canary-f5f9f71"
skiko = "0.8.4"
skiko = "0.8.8"
sqldelight = "1.3.0"
retrofit = "2.7.2"
wire = "4.5.1"
Expand Down
Loading