Unverified Commit 1ca83c19 authored by Igor Demin's avatar Igor Demin Committed by GitHub

Fix bounds of ComposePanel in IntelliJ on macOs (#988)

Fixes
https://youtrack.jetbrains.com/issue/CMP-5856/Desktop-ComposePanel-size-breaks-with-.fillMax-modifiers#focus=Comments-27-10632441.0-0

Fixes
https://youtrack.jetbrains.com/issue/CMP-5968/Compose-content-is-rendered-in-the-wrong-place-in-IJ-when-using-AWT-compositing

Regression after
https://github.com/JetBrains/skiko/pull/661/files#diff-910a6e28fda20a00bc98c6a8a04f74ab701d79e841b9baddf146b810e610572fR363
(`setBounds` is called more often, but not enough as `ancestorMoved`)

When a panel changes its position without changing its size, `doLayout`
isn't called because the content itself wasn't changed. But we still
need to update the bounds of the underlying layer.

## Testing
-
https://youtrack.jetbrains.com/issue/CMP-5856/Desktop-ComposePanel-size-breaks-with-.fillMax-modifiers#focus=Comments-27-10632441.0-0
isn't reproducible after the fix
- there are no resize glitches

## Release Notes
### Fixes
- Fix bounds of ComposePanel in IntelliJ on macOs
parent e2bf12b9
...@@ -120,7 +120,7 @@ jobs: ...@@ -120,7 +120,7 @@ jobs:
./gradlew --stacktrace --info -Pskiko.native.enabled=true -Pskiko.test.onci=true :skiko:tvosX64Test ./gradlew --stacktrace --info -Pskiko.native.enabled=true -Pskiko.test.onci=true :skiko:tvosX64Test
# tvosSimulatorArm64Test will build the binary but the tests will be skipped due to X64 host machine # tvosSimulatorArm64Test will build the binary but the tests will be skipped due to X64 host machine
./gradlew --stacktrace --info -Pskiko.native.enabled=true -Pskiko.test.onci=true :skiko:tvosSimulatorArm64Test ./gradlew --stacktrace --info -Pskiko.native.enabled=true -Pskiko.test.onci=true :skiko:tvosSimulatorArm64Test
- uses: actions/upload-artifact@v2 - uses: actions/upload-artifact@v4
if: always() if: always()
with: with:
name: test-reports-macos name: test-reports-macos
......
...@@ -60,7 +60,7 @@ jobs: ...@@ -60,7 +60,7 @@ jobs:
cd ./skiko cd ./skiko
source ./emsdk/emsdk_env.sh source ./emsdk/emsdk_env.sh
./gradlew --stacktrace --info -Pskiko.wasm.enabled=true -Pskiko.js.enabled=true -Pskiko.test.onci=true jsTest ./gradlew --stacktrace --info -Pskiko.wasm.enabled=true -Pskiko.js.enabled=true -Pskiko.test.onci=true jsTest
- uses: actions/upload-artifact@v2 - uses: actions/upload-artifact@v4
if: always() if: always()
with: with:
name: test-reports-wasm name: test-reports-wasm
......
...@@ -18,6 +18,10 @@ import javax.swing.JPanel ...@@ -18,6 +18,10 @@ import javax.swing.JPanel
import javax.swing.SwingUtilities import javax.swing.SwingUtilities
import javax.swing.SwingUtilities.isEventDispatchThread import javax.swing.SwingUtilities.isEventDispatchThread
import javax.swing.UIManager import javax.swing.UIManager
import javax.swing.event.AncestorEvent
import javax.swing.event.AncestorListener
import kotlin.math.ceil
import kotlin.math.floor
actual open class SkiaLayer internal constructor( actual open class SkiaLayer internal constructor(
externalAccessibleFactory: ((Component) -> Accessible)? = null, externalAccessibleFactory: ((Component) -> Accessible)? = null,
...@@ -133,6 +137,16 @@ actual open class SkiaLayer internal constructor( ...@@ -133,6 +137,16 @@ actual open class SkiaLayer internal constructor(
@Suppress("LeakingThis") @Suppress("LeakingThis")
add(backedLayer) add(backedLayer)
addAncestorListener(object : AncestorListener {
override fun ancestorAdded(event: AncestorEvent?) = Unit
override fun ancestorRemoved(event: AncestorEvent?) = Unit
override fun ancestorMoved(event: AncestorEvent?) {
revalidate()
}
})
backedLayer.addHierarchyListener { backedLayer.addHierarchyListener {
if (it.changeFlags and HierarchyEvent.SHOWING_CHANGED.toLong() != 0L) { if (it.changeFlags and HierarchyEvent.SHOWING_CHANGED.toLong() != 0L) {
checkShowing() checkShowing()
...@@ -143,7 +157,7 @@ actual open class SkiaLayer internal constructor( ...@@ -143,7 +157,7 @@ actual open class SkiaLayer internal constructor(
addPropertyChangeListener("graphicsContextScaleTransform") { addPropertyChangeListener("graphicsContextScaleTransform") {
Logger.debug { "graphicsContextScaleTransform changed for $this" } Logger.debug { "graphicsContextScaleTransform changed for $this" }
latestReceivedGraphicsContextScaleTransform = it.newValue as AffineTransform latestReceivedGraphicsContextScaleTransform = it.newValue as AffineTransform
redrawer?.syncSize() revalidate()
notifyChange(PropertyKind.ContentScale) notifyChange(PropertyKind.ContentScale)
// Workaround for JBR-5259 // Workaround for JBR-5259
...@@ -191,7 +205,7 @@ actual open class SkiaLayer internal constructor( ...@@ -191,7 +205,7 @@ actual open class SkiaLayer internal constructor(
redrawer?.setVisible(isShowing) redrawer?.setVisible(isShowing)
} }
if (isShowing) { if (isShowing) {
redrawer?.syncSize() redrawer?.syncBounds()
repaint() repaint()
} }
} }
...@@ -252,7 +266,7 @@ actual open class SkiaLayer internal constructor( ...@@ -252,7 +266,7 @@ actual open class SkiaLayer internal constructor(
private val redrawerManager = RedrawerManager<Redrawer>(properties.renderApi) { renderApi, oldRedrawer -> private val redrawerManager = RedrawerManager<Redrawer>(properties.renderApi) { renderApi, oldRedrawer ->
oldRedrawer?.dispose() oldRedrawer?.dispose()
val newRedrawer = renderFactory.createRedrawer(this, renderApi, analytics, properties) val newRedrawer = renderFactory.createRedrawer(this, renderApi, analytics, properties)
newRedrawer.syncSize() newRedrawer.syncBounds()
newRedrawer newRedrawer
} }
...@@ -316,12 +330,16 @@ actual open class SkiaLayer internal constructor( ...@@ -316,12 +330,16 @@ actual open class SkiaLayer internal constructor(
override fun doLayout() { override fun doLayout() {
Logger.debug { "doLayout on $this" } Logger.debug { "doLayout on $this" }
backedLayer.setBounds(0, 0, roundSize(width), roundSize(height)) backedLayer.setBounds(
0,
0,
adjustSizeToContentScale(contentScale, width),
adjustSizeToContentScale(contentScale, height)
)
backedLayer.validate() backedLayer.validate()
redrawer?.syncSize() redrawer?.syncBounds()
} }
override fun paint(g: java.awt.Graphics) { override fun paint(g: java.awt.Graphics) {
Logger.debug { "Paint called on: $this" } Logger.debug { "Paint called on: $this" }
checkContentScale() checkContentScale()
...@@ -338,7 +356,7 @@ actual open class SkiaLayer internal constructor( ...@@ -338,7 +356,7 @@ actual open class SkiaLayer internal constructor(
// Please note that calling redraw during layout might break software renderers, // Please note that calling redraw during layout might break software renderers,
// so applying this fix only for Direct3D case. // so applying this fix only for Direct3D case.
if (renderApi == GraphicsApi.DIRECT3D && isShowing) { if (renderApi == GraphicsApi.DIRECT3D && isShowing) {
redrawer?.syncSize() redrawer?.syncBounds()
tryRedrawImmediately() tryRedrawImmediately()
} }
} }
...@@ -579,18 +597,6 @@ actual open class SkiaLayer internal constructor( ...@@ -579,18 +597,6 @@ actual open class SkiaLayer internal constructor(
} }
} }
private fun roundSize(value: Int): Int {
var rounded = value * contentScale
val diff = rounded - rounded.toInt()
// We check values close to 0.5 and edit the size to avoid white lines glitch
if (diff > 0.4f && diff < 0.6f) {
rounded = value + 1f
} else {
rounded = value.toFloat()
}
return rounded.toInt()
}
fun requestNativeFocusOnAccessible(accessible: Accessible?) { fun requestNativeFocusOnAccessible(accessible: Accessible?) {
backedLayer.requestNativeFocusOnAccessible(accessible) backedLayer.requestNativeFocusOnAccessible(accessible)
} }
...@@ -637,3 +643,28 @@ internal fun Canvas.clipRectBy(rectangle: ClipRectangle, scale: Float) { ...@@ -637,3 +643,28 @@ internal fun Canvas.clipRectBy(rectangle: ClipRectangle, scale: Float) {
true true
) )
} }
// TODO Recheck this method validity in 2 cases - full Window content, and a Panel content
// issue: https://youtrack.jetbrains.com/issue/CMP-5447/Window-white-line-on-the-bottom-before-resizing
// suggestions: https://github.com/JetBrains/skiko/pull/988#discussion_r1763219300
// possible issues:
// - isn't obvious why 0.4/0.6 is used
// - increasing it by one, we avoid 1px white line, but we cut the content by 1px
// - it probably doesn't work correctly in a Panel content case - we don't need to adjust in this case
/**
* Increases the value of width/height by one if necessary,
* to avoid 1px white line between Canvas and the bounding window.
* 1px white line appears when users resizes the window:
* - it is resized in Px (125, 126, 127,...)
* - the canvas is resized in Points (with 1.25 scale, it will be 100, 100, 101)
* during converting Int AWT points to actual screen pixels.
*/
private fun adjustSizeToContentScale(contentScale: Float, value: Int): Int {
val scaled = value * contentScale
val diff = scaled - floor(scaled)
return if (diff > 0.4f && diff < 0.6f) {
value + 1
} else {
value
}
}
\ No newline at end of file
...@@ -169,7 +169,7 @@ internal class MetalRedrawer( ...@@ -169,7 +169,7 @@ internal class MetalRedrawer(
} }
} }
override fun syncSize() = synchronized(drawLock) { override fun syncBounds() = synchronized(drawLock) {
check(isEventDispatchThread()) { "Method should be called from AWT event dispatch thread" } check(isEventDispatchThread()) { "Method should be called from AWT event dispatch thread" }
val rootPane = getRootPane(layer) val rootPane = getRootPane(layer)
val globalPosition = convertPoint(layer.backedLayer, 0, 0, rootPane) val globalPosition = convertPoint(layer.backedLayer, 0, 0, rootPane)
......
...@@ -28,6 +28,7 @@ import java.awt.BorderLayout ...@@ -28,6 +28,7 @@ import java.awt.BorderLayout
import java.awt.Color import java.awt.Color
import java.awt.Dimension import java.awt.Dimension
import java.awt.event.* import java.awt.event.*
import javax.swing.Box
import javax.swing.JFrame import javax.swing.JFrame
import javax.swing.JLayeredPane import javax.swing.JLayeredPane
import javax.swing.JPanel import javax.swing.JPanel
...@@ -261,6 +262,48 @@ class SkiaLayerTest { ...@@ -261,6 +262,48 @@ class SkiaLayerTest {
} }
} }
@Test
fun `move without redrawing`() = uiTest {
val window = JFrame()
val layer = SkiaLayer(
properties = SkiaLayerProperties(renderApi = renderApi)
)
layer.renderDelegate = object : SkikoRenderDelegate {
override fun onRender(canvas: Canvas, width: Int, height: Int, nanoTime: Long) {
canvas.drawRect(Rect(0f, 0f, width.toFloat(), height.toFloat()), Paint().apply {
color = Color.RED.rgb
})
}
}
layer.size = Dimension(100, 100)
val box = Box.createVerticalBox().apply {
add(layer)
}
box.setBounds(0, 0, 100, 100)
try {
val panel = JLayeredPane()
panel.add(box)
window.contentPane.add(panel)
window.setLocation(200, 200)
window.size = Dimension(200, 200)
window.defaultCloseOperation = WindowConstants.DISPOSE_ON_CLOSE
window.isUndecorated = true
window.isVisible = true
delay(1000)
screenshots.assert(window.bounds, "frame1")
box.setBounds(100, 0, 100, 100)
delay(1000)
screenshots.assert(window.bounds, "frame2")
} finally {
layer.dispose()
window.close()
}
}
@Test @Test
fun `resize window`() = uiTest { fun `resize window`() = uiTest {
val window = UiTestWindow() val window = UiTestWindow()
......
...@@ -4,7 +4,7 @@ internal interface Redrawer { ...@@ -4,7 +4,7 @@ internal interface Redrawer {
fun dispose() fun dispose()
fun needRedraw() fun needRedraw()
fun redrawImmediately() fun redrawImmediately()
fun syncSize() = Unit fun syncBounds() = Unit
fun setVisible(isVisible: Boolean) = Unit fun setVisible(isVisible: Boolean) = Unit
val renderInfo: String val renderInfo: String
} }
\ No newline at end of file
...@@ -84,13 +84,13 @@ actual open class SkiaLayer { ...@@ -84,13 +84,13 @@ actual open class SkiaLayer {
private val nsViewObserver = object : NSObject() { private val nsViewObserver = object : NSObject() {
@ObjCAction @ObjCAction
fun frameDidChange(notification: NSNotification) { fun frameDidChange(notification: NSNotification) {
redrawer?.syncSize() redrawer?.syncBounds()
redrawer?.redrawImmediately() redrawer?.redrawImmediately()
} }
@ObjCAction @ObjCAction
fun windowDidChangeBackingProperties(notification: NSNotification) { fun windowDidChangeBackingProperties(notification: NSNotification) {
redrawer?.syncSize() redrawer?.syncBounds()
redrawer?.redrawImmediately() redrawer?.redrawImmediately()
} }
...@@ -126,7 +126,7 @@ actual open class SkiaLayer { ...@@ -126,7 +126,7 @@ actual open class SkiaLayer {
nsView.postsFrameChangedNotifications = true nsView.postsFrameChangedNotifications = true
nsViewObserver.addObserver() nsViewObserver.addObserver()
redrawer = createNativeRedrawer(this, renderApi).apply { redrawer = createNativeRedrawer(this, renderApi).apply {
syncSize() syncBounds()
needRedraw() needRedraw()
} }
} }
......
...@@ -79,7 +79,7 @@ internal class MacOsMetalRedrawer( ...@@ -79,7 +79,7 @@ internal class MacOsMetalRedrawer(
/** /**
* Synchronizes the [metalLayer] size with the size of underlying nsView * Synchronizes the [metalLayer] size with the size of underlying nsView
*/ */
override fun syncSize() { override fun syncBounds() {
syncContentScale() syncContentScale()
val osFrame = skiaLayer.nsView.frame val osFrame = skiaLayer.nsView.frame
val (w, h) = osFrame.useContents { val (w, h) = osFrame.useContents {
......
...@@ -43,7 +43,7 @@ internal class MacOsOpenGLRedrawer( ...@@ -43,7 +43,7 @@ internal class MacOsOpenGLRedrawer(
glLayer.dispose() glLayer.dispose()
} }
override fun syncSize() { override fun syncBounds() {
syncContentScale() syncContentScale()
skiaLayer.nsView.frame.useContents { skiaLayer.nsView.frame.useContents {
glLayer.setFrame( glLayer.setFrame(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment