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

Proper clipping of SwingPanel interop #915

Merged
merged 20 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion compose/ui/ui/api/desktop/ui.api
Original file line number Diff line number Diff line change
Expand Up @@ -3120,7 +3120,6 @@ public abstract interface class androidx/compose/ui/platform/PlatformContext {
public fun isWindowTransparent ()Z
public fun requestFocus ()Z
public fun setPointerIcon (Landroidx/compose/ui/input/pointer/PointerIcon;)V
public fun setWindowTransparent (Z)V
MatkovIvan marked this conversation as resolved.
Show resolved Hide resolved
}

public final class androidx/compose/ui/platform/PlatformContext$Companion {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,14 @@ internal abstract class ComposeBridge(
private var isDisposed = false

private val _invisibleComponent = InvisibleComponent()

abstract val component: JComponent
val invisibleComponent: Component get() = _invisibleComponent

abstract val component: JComponent
abstract val renderApi: GraphicsApi

abstract val interopBlendingSupported: Boolean
abstract val clipComponents: MutableList<ClipRectangle>
private val clipMap = mutableMapOf<Component, ClipComponent>()

// Needed for case when componentLayer is a wrapper for another Component that need to acquire focus events
// e.g. canvas in case of ComposeWindowLayer
Expand Down Expand Up @@ -149,6 +150,7 @@ internal abstract class ComposeBridge(
private val desktopTextInputService = DesktopTextInputService(platformComponent)
protected val platformContext = DesktopPlatformContext()
internal var rootForTestListener: PlatformContext.RootForTestListener? by DelegateRootForTestListener()
internal var isWindowTransparent by platformContext::isWindowTransparent

private val semanticsOwnerListener = DesktopSemanticsOwnerListener()
val sceneAccessible = ComposeSceneAccessible {
Expand Down Expand Up @@ -337,6 +339,18 @@ internal abstract class ComposeBridge(
initContent()
}

fun addClipComponent(component: Component) {
val clipComponent = ClipComponent(component)
clipMap[component] = clipComponent
clipComponents.add(clipComponent)
}

fun removeClipComponent(component: Component) {
clipMap.remove(component)?.let {
clipComponents.remove(it)
}
}

private var _initContent: (() -> Unit)? = null

protected fun initContent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ class ComposeDialog : JDialog {
* `true`, otherwise AWT will throw an exception.
*/
var isTransparent: Boolean
get() = delegate.isTransparent
get() = delegate.isWindowTransparent
set(value) {
delegate.isTransparent = value
delegate.isWindowTransparent = value
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ import javax.swing.JLayeredPane
import javax.swing.SwingUtilities.isEventDispatchThread
import org.jetbrains.skiko.ClipComponent
import org.jetbrains.skiko.GraphicsApi
import org.jetbrains.skiko.OS
import org.jetbrains.skiko.SkiaLayerAnalytics
import org.jetbrains.skiko.hostOs

/**
* ComposePanel is a panel for building UI using Compose for Desktop.
Expand Down Expand Up @@ -86,7 +88,6 @@ class ComposePanel @ExperimentalComposeUiApi constructor(
private var _isFocusable = true
private var _isRequestFocusEnabled = false
private var bridge: ComposeBridge? = null
private val clipMap = mutableMapOf<Component, ClipComponent>()
private var content: (@Composable () -> Unit)? = null

/**
Expand Down Expand Up @@ -171,38 +172,58 @@ class ComposePanel @ExperimentalComposeUiApi constructor(
}

override fun add(component: Component): Component {
if (bridge == null) {
return component
addToLayer(component, componentLayer)
if (!interopBlending) {
bridge?.addClipComponent(component)
}
val clipComponent = ClipComponent(component)
clipMap[component] = clipComponent
bridge!!.clipComponents.add(clipComponent)
return super.add(component, Integer.valueOf(0))
return component
}

override fun remove(component: Component) {
bridge!!.clipComponents.remove(clipMap[component]!!)
clipMap.remove(component)
bridge?.removeClipComponent(component)
super.remove(component)
}

private fun addToLayer(component: Component, layer: Int) {
if (renderApi == GraphicsApi.METAL && bridge !is SwingComposeBridge) {
igordmn marked this conversation as resolved.
Show resolved Hide resolved
// Applying layer on macOS makes our bridge non-transparent
// But it draws always on top, so we can just add it as-is
// TODO: Figure out why it makes difference in transparency
super.add(component, 0)
} else {
super.setLayer(component, layer)
super.add(component)
}
}

private val bridgeLayer: Int = 10
private val componentLayer: Int
get() = if (interopBlending) 0 else 20

private val renderOnGraphics: Boolean
get() = System.getProperty("compose.swing.render.on.graphics").toBoolean()
Copy link

Choose a reason for hiding this comment

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

Do we want to allow changing these settings at runtime? If not, I suggest reading them once and storing the values.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's existing behaviour, I don't want to introduce new restrictions here

private val _interopBlending: Boolean
get() = System.getProperty("compose.interop.blending").toBoolean()
Copy link

Choose a reason for hiding this comment

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

Here too,

private val interopBlending: Boolean
get() = _interopBlending &&
(renderOnGraphics || requireNotNull(bridge).interopBlendingSupported)

override fun addNotify() {
super.addNotify()

// After [super.addNotify] is called we can safely initialize the bridge and composable
// content.
if (bridge == null) {
bridge = createComposeBridge()
if (this.bridge == null) {
val bridge = createComposeBridge()
this.bridge = bridge
initContent()
super.add(bridge!!.invisibleComponent, Integer.valueOf(1))
super.add(bridge!!.component, Integer.valueOf(1))
addToLayer(bridge.invisibleComponent, bridgeLayer)
addToLayer(bridge.component, bridgeLayer)
}
}

private fun createComposeBridge(): ComposeBridge {
val renderOnGraphics = System.getProperty("compose.swing.render.on.graphics").toBoolean()
val bridge: ComposeBridge = if (renderOnGraphics) {
// TODO: Add window transparent info
SwingComposeBridge(skiaLayerAnalytics, layoutDirectionFor(this))
} else {
WindowComposeBridge(skiaLayerAnalytics, layoutDirectionFor(this))
Expand Down Expand Up @@ -328,5 +349,5 @@ class ComposePanel @ExperimentalComposeUiApi constructor(
* environment variable.
*/
val renderApi: GraphicsApi
get() = if (bridge != null) bridge!!.renderApi else GraphicsApi.UNKNOWN
get() = bridge?.renderApi ?: GraphicsApi.UNKNOWN
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class ComposeWindow @ExperimentalComposeUiApi constructor(
* Transparency should be set only if window is not showing and `isUndecorated` is set to
* `true`, otherwise AWT will throw an exception.
*/
var isTransparent: Boolean by delegate::isTransparent
var isTransparent: Boolean by delegate::isWindowTransparent

var placement: WindowPlacement
get() = when {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,30 +60,123 @@ internal class ComposeWindowDelegate(
}
internal val scene: ComposeScene
get() = bridge.scene

internal val windowAccessible: Accessible
get() = bridge.sceneAccessible

internal var rootForTestListener by bridge::rootForTestListener

val undecoratedWindowResizer = UndecoratedWindowResizer(window)

var fullscreen: Boolean
get() = bridge.component.fullscreen
set(value) {
bridge.component.fullscreen = value
}

var compositionLocalContext: CompositionLocalContext?
get() = bridge.compositionLocalContext
set(value) {
bridge.compositionLocalContext = value
}

@ExperimentalComposeUiApi
var exceptionHandler: WindowExceptionHandler?
get() = bridge.exceptionHandler
set(value) {
bridge.exceptionHandler = value
}

val windowHandle: Long
get() = bridge.component.windowHandle

val renderApi: GraphicsApi
get() = bridge.renderApi

private val _interopBlending: Boolean
get() = System.getProperty("compose.interop.blending").toBoolean()
Copy link

Choose a reason for hiding this comment

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

Here too.

private val interopBlending: Boolean
get() = _interopBlending && bridge.interopBlendingSupported

var isWindowTransparent: Boolean = false
set(value) {
if (field != value) {
check(isUndecorated()) { "Transparent window should be undecorated!" }
check(!window.isDisplayable) {
"Cannot change transparency if window is already displayable."
}
field = value
bridge.isWindowTransparent = value
bridge.transparency = value || interopBlending

/*
* Windows makes clicks on transparent pixels fall through, but it doesn't work
* with GPU accelerated rendering since this check requires having access to pixels from CPU.
*
* JVM doesn't allow override this behaviour with low-level windows methods, so hack this in this way.
* Based on tests, it doesn't affect resulting pixel color.
*
* Note: Do not set isOpaque = false for this container
*/
if (value && hostOs == OS.Windows) {
pane.background = Color(0, 0, 0, 1)
pane.isOpaque = true
} else {
pane.background = null
pane.isOpaque = false
}

window.background = if (value && !skikoTransparentWindowHack) Color(0, 0, 0, 0) else null
}
}

/**
* There is a hack inside skiko OpenGL and Software redrawers for Windows that makes current
* window transparent without setting `background` to JDK's window. It's done by getting native
* component parent and calling `DwmEnableBlurBehindWindow`.
*
* FIXME: Make OpenGL work inside transparent window (background == Color(0, 0, 0, 0)) without this hack.
*
* See `enableTransparentWindow` (skiko/src/awtMain/cpp/windows/window_util.cc)
*/
private val skikoTransparentWindowHack: Boolean
get() = hostOs == OS.Windows && renderApi != GraphicsApi.DIRECT3D

private val _pane = object : JLayeredPane() {
override fun setBounds(x: Int, y: Int, width: Int, height: Int) {
bridge.component.setSize(width, height)
bridge.component.setBounds(0, 0, width, height)
super.setBounds(x, y, width, height)
}

override fun add(component: Component): Component {
val clipComponent = ClipComponent(component)
clipMap[component] = clipComponent
bridge.clipComponents.add(clipComponent)
return add(component, Integer.valueOf(0))
addToLayer(component, componentLayer)
if (!interopBlending) {
bridge.addClipComponent(component)
}
return component
}

override fun remove(component: Component) {
bridge.clipComponents.remove(clipMap[component]!!)
clipMap.remove(component)
bridge.removeClipComponent(component)
super.remove(component)
}

private fun addToLayer(component: Component, layer: Int) {
if (renderApi == GraphicsApi.METAL) {
Copy link
Collaborator

@igordmn igordmn Dec 13, 2023

Choose a reason for hiding this comment

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

I agree with Alexander, we use so many hacks for transparency that we can easily break the behavior in the future.

I tested main variations on Windows/macOs - it works, but to be safe, let's write java.awt.Robot screenshot tests for different OS'es, as we wrote them in Skiko (look at SkiaLayerTest).

Disable it for now, as we should run them only on CI. I will configure it later myself.

We can merge the PR now to receive feedback earlier, but If we agree that we can/should write the tests, we should write them before the 1.6.0 release (please, create a task).

Even if Robot tests can be flaky and it is hard to run them locally, they are very useful.

To write tests for transparency, we can place an opaque window behind a transparent window.

Copy link
Collaborator

@igordmn igordmn Dec 13, 2023

Choose a reason for hiding this comment

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

// Applying layer on macOS makes our bridge non-transparent
// But it draws always on top, so we can just add it as-is
// TODO: Figure out why it makes difference in transparency
super.add(component, 0)
} else {
super.setLayer(component, layer)
super.add(component)
}
}

private val bridgeLayer: Int get() = 10
private val componentLayer: Int
get() = if (interopBlending) 0 else 20

override fun addNotify() {
super.addNotify()
bridge.component.requestFocus()
Expand All @@ -94,8 +187,8 @@ internal class ComposeWindowDelegate(

init {
layout = null
super.add(bridge.invisibleComponent, 1)
super.add(bridge.component, 1)
addToLayer(bridge.invisibleComponent, bridgeLayer)
addToLayer(bridge.component, bridgeLayer)
}

fun dispose() {
Expand All @@ -106,8 +199,6 @@ internal class ComposeWindowDelegate(

val pane get() = _pane

private val clipMap = mutableMapOf<Component, ClipComponent>()

init {
pane.focusTraversalPolicy = object : FocusTraversalPolicy() {
override fun getComponentAfter(aContainer: Container?, aComponent: Component?) = null
Expand All @@ -117,6 +208,7 @@ internal class ComposeWindowDelegate(
override fun getDefaultComponent(aContainer: Container?) = null
}
pane.isFocusCycleRoot = true
bridge.transparency = interopBlending
setContent {}
}

Expand All @@ -128,18 +220,6 @@ internal class ComposeWindowDelegate(
_pane.remove(component)
}

var fullscreen: Boolean
get() = bridge.component.fullscreen
set(value) {
bridge.component.fullscreen = value
}

var compositionLocalContext: CompositionLocalContext?
get() = bridge.compositionLocalContext
set(value) {
bridge.compositionLocalContext = value
}

fun setContent(
onPreviewKeyEvent: (KeyEvent) -> Boolean = { false },
onKeyEvent: (KeyEvent) -> Boolean = { false },
Expand Down Expand Up @@ -225,38 +305,6 @@ internal class ComposeWindowDelegate(
}
}

@ExperimentalComposeUiApi
var exceptionHandler: WindowExceptionHandler?
get() = bridge.exceptionHandler
set(value) {
bridge.exceptionHandler = value
}

val windowHandle: Long
get() = bridge.component.windowHandle

val renderApi: GraphicsApi
get() = bridge.renderApi

var isTransparent: Boolean
get() = bridge.transparency
set(value) {
if (value != bridge.transparency) {
check(isUndecorated()) { "Transparent window should be undecorated!" }
check(!window.isDisplayable) {
"Cannot change transparency if window is already displayable."
}
bridge.transparency = value
if (value) {
if (hostOs != OS.Windows) {
window.background = Color(0, 0, 0, 0)
}
} else {
window.background = null
}
}
}

fun addMouseListener(listener: MouseListener) {
bridge.component.addMouseListener(listener)
}
Expand Down