diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e6ba78c183..6b32e4327b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,7 +120,7 @@ jobs: ./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 ./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() with: name: test-reports-macos diff --git a/.github/workflows/web.yml b/.github/workflows/web.yml index 3d0785a1a6..cf026f177e 100644 --- a/.github/workflows/web.yml +++ b/.github/workflows/web.yml @@ -60,7 +60,7 @@ jobs: cd ./skiko source ./emsdk/emsdk_env.sh ./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() with: name: test-reports-wasm diff --git a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt index 8340fcaf0d..c6920b9b85 100644 --- a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt +++ b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt @@ -18,6 +18,10 @@ import javax.swing.JPanel import javax.swing.SwingUtilities import javax.swing.SwingUtilities.isEventDispatchThread 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( externalAccessibleFactory: ((Component) -> Accessible)? = null, @@ -133,6 +137,16 @@ actual open class SkiaLayer internal constructor( @Suppress("LeakingThis") 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 { if (it.changeFlags and HierarchyEvent.SHOWING_CHANGED.toLong() != 0L) { checkShowing() @@ -143,7 +157,7 @@ actual open class SkiaLayer internal constructor( addPropertyChangeListener("graphicsContextScaleTransform") { Logger.debug { "graphicsContextScaleTransform changed for $this" } latestReceivedGraphicsContextScaleTransform = it.newValue as AffineTransform - redrawer?.syncSize() + revalidate() notifyChange(PropertyKind.ContentScale) // Workaround for JBR-5259 @@ -191,7 +205,7 @@ actual open class SkiaLayer internal constructor( redrawer?.setVisible(isShowing) } if (isShowing) { - redrawer?.syncSize() + redrawer?.syncBounds() repaint() } } @@ -252,7 +266,7 @@ actual open class SkiaLayer internal constructor( private val redrawerManager = RedrawerManager(properties.renderApi) { renderApi, oldRedrawer -> oldRedrawer?.dispose() val newRedrawer = renderFactory.createRedrawer(this, renderApi, analytics, properties) - newRedrawer.syncSize() + newRedrawer.syncBounds() newRedrawer } @@ -316,12 +330,16 @@ actual open class SkiaLayer internal constructor( override fun doLayout() { 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() - redrawer?.syncSize() + redrawer?.syncBounds() } - override fun paint(g: java.awt.Graphics) { Logger.debug { "Paint called on: $this" } checkContentScale() @@ -338,7 +356,7 @@ actual open class SkiaLayer internal constructor( // Please note that calling redraw during layout might break software renderers, // so applying this fix only for Direct3D case. if (renderApi == GraphicsApi.DIRECT3D && isShowing) { - redrawer?.syncSize() + redrawer?.syncBounds() tryRedrawImmediately() } } @@ -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?) { backedLayer.requestNativeFocusOnAccessible(accessible) } @@ -637,3 +643,28 @@ internal fun Canvas.clipRectBy(rectangle: ClipRectangle, scale: Float) { 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 diff --git a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt index 8f6e6a8b8f..f9be7609ec 100644 --- a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt +++ b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt @@ -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" } val rootPane = getRootPane(layer) val globalPosition = convertPoint(layer.backedLayer, 0, 0, rootPane) diff --git a/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt b/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt index 49200494fd..deb8bb0bcb 100644 --- a/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt +++ b/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt @@ -28,6 +28,7 @@ import java.awt.BorderLayout import java.awt.Color import java.awt.Dimension import java.awt.event.* +import javax.swing.Box import javax.swing.JFrame import javax.swing.JLayeredPane import javax.swing.JPanel @@ -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 fun `resize window`() = uiTest { val window = UiTestWindow() diff --git a/skiko/src/commonMain/kotlin/org/jetbrains/skiko/redrawer/Redrawer.kt b/skiko/src/commonMain/kotlin/org/jetbrains/skiko/redrawer/Redrawer.kt index d072f45d9f..a071017773 100644 --- a/skiko/src/commonMain/kotlin/org/jetbrains/skiko/redrawer/Redrawer.kt +++ b/skiko/src/commonMain/kotlin/org/jetbrains/skiko/redrawer/Redrawer.kt @@ -4,7 +4,7 @@ internal interface Redrawer { fun dispose() fun needRedraw() fun redrawImmediately() - fun syncSize() = Unit + fun syncBounds() = Unit fun setVisible(isVisible: Boolean) = Unit val renderInfo: String } \ No newline at end of file diff --git a/skiko/src/jvmTest/screenshots/org_jetbrains_skiko_SkiaLayerTest_move_without_redrawing_frame1.png b/skiko/src/jvmTest/screenshots/org_jetbrains_skiko_SkiaLayerTest_move_without_redrawing_frame1.png new file mode 100644 index 0000000000..0c6674d600 Binary files /dev/null and b/skiko/src/jvmTest/screenshots/org_jetbrains_skiko_SkiaLayerTest_move_without_redrawing_frame1.png differ diff --git a/skiko/src/jvmTest/screenshots/org_jetbrains_skiko_SkiaLayerTest_move_without_redrawing_frame2.png b/skiko/src/jvmTest/screenshots/org_jetbrains_skiko_SkiaLayerTest_move_without_redrawing_frame2.png new file mode 100644 index 0000000000..291c97806c Binary files /dev/null and b/skiko/src/jvmTest/screenshots/org_jetbrains_skiko_SkiaLayerTest_move_without_redrawing_frame2.png differ diff --git a/skiko/src/macosMain/kotlin/org/jetbrains/skiko/SkiaLayer.macos.kt b/skiko/src/macosMain/kotlin/org/jetbrains/skiko/SkiaLayer.macos.kt index 070a9afd14..cea8954c4e 100644 --- a/skiko/src/macosMain/kotlin/org/jetbrains/skiko/SkiaLayer.macos.kt +++ b/skiko/src/macosMain/kotlin/org/jetbrains/skiko/SkiaLayer.macos.kt @@ -84,13 +84,13 @@ actual open class SkiaLayer { private val nsViewObserver = object : NSObject() { @ObjCAction fun frameDidChange(notification: NSNotification) { - redrawer?.syncSize() + redrawer?.syncBounds() redrawer?.redrawImmediately() } @ObjCAction fun windowDidChangeBackingProperties(notification: NSNotification) { - redrawer?.syncSize() + redrawer?.syncBounds() redrawer?.redrawImmediately() } @@ -126,7 +126,7 @@ actual open class SkiaLayer { nsView.postsFrameChangedNotifications = true nsViewObserver.addObserver() redrawer = createNativeRedrawer(this, renderApi).apply { - syncSize() + syncBounds() needRedraw() } } diff --git a/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.macos.kt b/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.macos.kt index 55d60952f5..3648d0f0e6 100644 --- a/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.macos.kt +++ b/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.macos.kt @@ -79,7 +79,7 @@ internal class MacOsMetalRedrawer( /** * Synchronizes the [metalLayer] size with the size of underlying nsView */ - override fun syncSize() { + override fun syncBounds() { syncContentScale() val osFrame = skiaLayer.nsView.frame val (w, h) = osFrame.useContents { diff --git a/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/OpenGLRedrawer.macos.kt b/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/OpenGLRedrawer.macos.kt index 4c2b002ba6..fe83630906 100644 --- a/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/OpenGLRedrawer.macos.kt +++ b/skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/OpenGLRedrawer.macos.kt @@ -43,7 +43,7 @@ internal class MacOsOpenGLRedrawer( glLayer.dispose() } - override fun syncSize() { + override fun syncBounds() { syncContentScale() skiaLayer.nsView.frame.useContents { glLayer.setFrame(