Swing interop: off-screen rendering on graphics

[Walingar]

Proposed Changes

This PR is related to JetBrains/skiko#720

• Use system property "compose.swing.render.on.graphics" that enables off-screen rendering on graphics
• Introduce ComposeSwingLayer and ComposeWindowLayer to separate using different backend layers (lightweight and heavyweight components)
• Extract common logic into ComposeLayer abstract class

Testing

Test: Since skiko PR is not merged yet, it is needed to build skiko part locally and then test it on swingExample from desktop examples passing "-Dcompose.swing.render.on.graphics=true"

Issues Fixed

It will partially fix: JetBrains/compose-multiplatform#2925
And other interop issues related to rendering

[Walingar]

The same here. I delegate some parts of SkiaLayer here, so it can be used in ComposePanel, may beyou can propose better solution...

[Walingar]

Code of these layers are kinda the same. I thought about having Wrapper JPanel and override these methods there and add SkiaLayer to it, but I think that something may go wrong in this case.

[m-sasha]

Do we really need compose.swing.render.on.graphics?

Is this not what we want:

• When the window is a compose window, use ComposeWindowLayer and regular drawing.
• When using ComposePanel in a Swing app, use ComposeSwingLayer.

[Walingar]

@m-sasha Regarding compose.swing.render.on.graphics we would like to have it for now, since this offscreen rendering is not implemented for Windows and Linux yet, so it's kinda an opt-in feature

Later, yes, we will remove it and have only SkiaSwingLayer, and some logic will be simplified

[m-sasha]

The names are too much here. Both too long and not very informative. I'm not sure I have better suggestions off the top of my head, but I'd really love some.

• ComposeLayer
• ComposeSwingLayer
• ComposeSwingSkiaLayer
• ComposeWindowLayer
• ComposeWindowSkiaLayer
• SkiaLayer
• SkiaSwingLayer
• SkiaSwingLayerComponent

🤯

[m-sasha]

I'm not sure you can trust Density.equals to work correctly. It's an interface with many different implementations.
Perhaps we shouldn't be using the Compose Density at this level at all, but a single scale: Float value. The fontScale seems to always be 1f here anyway.

[Walingar]

Sorry, I didn't get the second point, can you elaborate it? With the first one I agree, it seems we need to make density.density check here

[m-sasha]

About the 2nd point:
GraphicsConfiguration.density is currently a Density object with fontScale=1f. Maybe we should replace it with a GraphicsConfirguration.scale: Float property, and only create a Density from it where it's actually exposed to the user (in LocalDensity and such). Internally, only deal with a scale: Float.

[Walingar]

That's an interesting point. @igordmn WDYT?
I'd also keep it out of scope, but I would like to fix it in a separate PR after merging this one

[igordmn]

It is true that Density can be just some abstract object (look at its descendants).

But for simplicity, I wouldn't change it. The correct equals, or equals by reference - both work here. It is the similar to the check when we change LocalDensity.

is currently a Density object with fontScale=1f

We have an issue for supporting fontScale (macOs doesn't have it, Windows/Linux have it)

[m-sasha]

Not sure I understand how equals can work correctly here. If there are two implementations of Density that don't know about each other, comparing them for equality will likely return false. For example:

private data class MyDensity(
    override val density: Float,
    override val fontScale: Float
) : Density


fun main(){
    val density1 = Density(1.0f, 1.0f)
    val density2 = MyDensity(1.0f, 1.0f)
    println(density1 == density2)
}

prints false

[igordmn]

It is not about comparing 2 implementations (they indeed aren't equals), it is about GraphicsConfiguration.density returning the same instance if it wouln't implement equals:

val d = MyDensity(1.0f, 1.0f)

val GraphicsConfiguration.density get() = d

fun foo() {
    if (scene.density != graphicsConfiguration.density) {
        // only performs once
        println("update density")
    }
}

LocalDensity, ComposeScene.density has an impilicit performance contract that it needs to be the equal instance to avoid recompositions. Either by implemented equals, or just the same instance.

GraphicsConfiguration.density complies this contract using equals, but returning the same instance will work as well.

[m-sasha]

I think there's a problem here. platform.windowInfo.keyboardModifiers needs to be set from a listener on events on the window, not just the component.

[Walingar]

That's a really good point! But let's fix it separately

[igordmn]

True, it should listen the window.

[igordmn]

Found 2 bugs on Windows (i.e. software rendering) when we run ./gradlew runSwing:

1. When press "SOUTH/REMOVE COMPOSE", Compose panel isn't removed (but shouldn't)
2. When we press New window, we have a 2-3 second lag

They can be fixed separately I believe.

[Walingar]

Regarding the first one -- issue is inside the example, fixed it, please check.

[igordmn]

Shouldn't SkiaLayer itself call it?

panel.revalidate()
panel.repaint()

From my non-Swing perspective, I expect panel.remove to be the only function needed.

[Walingar]

It won't be called, in most of the cases, when you remove a component from a hierarchy, you need to call revalidate + repaint. I haven't found a documentation part regarding this, but this is from my experience.

[igordmn]

True, it should listen the window.

[igordmn]

It is true that Density can be just some abstract object (look at its descendants).

But for simplicity, I wouldn't change it. The correct equals, or equals by reference - both work here. It is the similar to the check when we change LocalDensity.

is currently a Density object with fontScale=1f

We have an issue for supporting fontScale (macOs doesn't have it, Windows/Linux have it)