Skip to content

Commit 1ca83c1

Browse files
authored
Fix bounds of ComposePanel in IntelliJ on macOs (JetBrains#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
1 parent e2bf12b commit 1ca83c1

File tree

11 files changed

+102
-28
lines changed

11 files changed

+102
-28
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ jobs:
120120
./gradlew --stacktrace --info -Pskiko.native.enabled=true -Pskiko.test.onci=true :skiko:tvosX64Test
121121
# tvosSimulatorArm64Test will build the binary but the tests will be skipped due to X64 host machine
122122
./gradlew --stacktrace --info -Pskiko.native.enabled=true -Pskiko.test.onci=true :skiko:tvosSimulatorArm64Test
123-
- uses: actions/upload-artifact@v2
123+
- uses: actions/upload-artifact@v4
124124
if: always()
125125
with:
126126
name: test-reports-macos

.github/workflows/web.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
cd ./skiko
6161
source ./emsdk/emsdk_env.sh
6262
./gradlew --stacktrace --info -Pskiko.wasm.enabled=true -Pskiko.js.enabled=true -Pskiko.test.onci=true jsTest
63-
- uses: actions/upload-artifact@v2
63+
- uses: actions/upload-artifact@v4
6464
if: always()
6565
with:
6666
name: test-reports-wasm

skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ import javax.swing.JPanel
1818
import javax.swing.SwingUtilities
1919
import javax.swing.SwingUtilities.isEventDispatchThread
2020
import javax.swing.UIManager
21+
import javax.swing.event.AncestorEvent
22+
import javax.swing.event.AncestorListener
23+
import kotlin.math.ceil
24+
import kotlin.math.floor
2125

2226
actual open class SkiaLayer internal constructor(
2327
externalAccessibleFactory: ((Component) -> Accessible)? = null,
@@ -133,6 +137,16 @@ actual open class SkiaLayer internal constructor(
133137
@Suppress("LeakingThis")
134138
add(backedLayer)
135139

140+
addAncestorListener(object : AncestorListener {
141+
override fun ancestorAdded(event: AncestorEvent?) = Unit
142+
143+
override fun ancestorRemoved(event: AncestorEvent?) = Unit
144+
145+
override fun ancestorMoved(event: AncestorEvent?) {
146+
revalidate()
147+
}
148+
})
149+
136150
backedLayer.addHierarchyListener {
137151
if (it.changeFlags and HierarchyEvent.SHOWING_CHANGED.toLong() != 0L) {
138152
checkShowing()
@@ -143,7 +157,7 @@ actual open class SkiaLayer internal constructor(
143157
addPropertyChangeListener("graphicsContextScaleTransform") {
144158
Logger.debug { "graphicsContextScaleTransform changed for $this" }
145159
latestReceivedGraphicsContextScaleTransform = it.newValue as AffineTransform
146-
redrawer?.syncSize()
160+
revalidate()
147161
notifyChange(PropertyKind.ContentScale)
148162

149163
// Workaround for JBR-5259
@@ -191,7 +205,7 @@ actual open class SkiaLayer internal constructor(
191205
redrawer?.setVisible(isShowing)
192206
}
193207
if (isShowing) {
194-
redrawer?.syncSize()
208+
redrawer?.syncBounds()
195209
repaint()
196210
}
197211
}
@@ -252,7 +266,7 @@ actual open class SkiaLayer internal constructor(
252266
private val redrawerManager = RedrawerManager<Redrawer>(properties.renderApi) { renderApi, oldRedrawer ->
253267
oldRedrawer?.dispose()
254268
val newRedrawer = renderFactory.createRedrawer(this, renderApi, analytics, properties)
255-
newRedrawer.syncSize()
269+
newRedrawer.syncBounds()
256270
newRedrawer
257271
}
258272

@@ -316,12 +330,16 @@ actual open class SkiaLayer internal constructor(
316330

317331
override fun doLayout() {
318332
Logger.debug { "doLayout on $this" }
319-
backedLayer.setBounds(0, 0, roundSize(width), roundSize(height))
333+
backedLayer.setBounds(
334+
0,
335+
0,
336+
adjustSizeToContentScale(contentScale, width),
337+
adjustSizeToContentScale(contentScale, height)
338+
)
320339
backedLayer.validate()
321-
redrawer?.syncSize()
340+
redrawer?.syncBounds()
322341
}
323342

324-
325343
override fun paint(g: java.awt.Graphics) {
326344
Logger.debug { "Paint called on: $this" }
327345
checkContentScale()
@@ -338,7 +356,7 @@ actual open class SkiaLayer internal constructor(
338356
// Please note that calling redraw during layout might break software renderers,
339357
// so applying this fix only for Direct3D case.
340358
if (renderApi == GraphicsApi.DIRECT3D && isShowing) {
341-
redrawer?.syncSize()
359+
redrawer?.syncBounds()
342360
tryRedrawImmediately()
343361
}
344362
}
@@ -579,18 +597,6 @@ actual open class SkiaLayer internal constructor(
579597
}
580598
}
581599

582-
private fun roundSize(value: Int): Int {
583-
var rounded = value * contentScale
584-
val diff = rounded - rounded.toInt()
585-
// We check values close to 0.5 and edit the size to avoid white lines glitch
586-
if (diff > 0.4f && diff < 0.6f) {
587-
rounded = value + 1f
588-
} else {
589-
rounded = value.toFloat()
590-
}
591-
return rounded.toInt()
592-
}
593-
594600
fun requestNativeFocusOnAccessible(accessible: Accessible?) {
595601
backedLayer.requestNativeFocusOnAccessible(accessible)
596602
}
@@ -637,3 +643,28 @@ internal fun Canvas.clipRectBy(rectangle: ClipRectangle, scale: Float) {
637643
true
638644
)
639645
}
646+
647+
// TODO Recheck this method validity in 2 cases - full Window content, and a Panel content
648+
// issue: https://youtrack.jetbrains.com/issue/CMP-5447/Window-white-line-on-the-bottom-before-resizing
649+
// suggestions: https://github.com/JetBrains/skiko/pull/988#discussion_r1763219300
650+
// possible issues:
651+
// - isn't obvious why 0.4/0.6 is used
652+
// - increasing it by one, we avoid 1px white line, but we cut the content by 1px
653+
// - it probably doesn't work correctly in a Panel content case - we don't need to adjust in this case
654+
/**
655+
* Increases the value of width/height by one if necessary,
656+
* to avoid 1px white line between Canvas and the bounding window.
657+
* 1px white line appears when users resizes the window:
658+
* - it is resized in Px (125, 126, 127,...)
659+
* - the canvas is resized in Points (with 1.25 scale, it will be 100, 100, 101)
660+
* during converting Int AWT points to actual screen pixels.
661+
*/
662+
private fun adjustSizeToContentScale(contentScale: Float, value: Int): Int {
663+
val scaled = value * contentScale
664+
val diff = scaled - floor(scaled)
665+
return if (diff > 0.4f && diff < 0.6f) {
666+
value + 1
667+
} else {
668+
value
669+
}
670+
}

skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ internal class MetalRedrawer(
169169
}
170170
}
171171

172-
override fun syncSize() = synchronized(drawLock) {
172+
override fun syncBounds() = synchronized(drawLock) {
173173
check(isEventDispatchThread()) { "Method should be called from AWT event dispatch thread" }
174174
val rootPane = getRootPane(layer)
175175
val globalPosition = convertPoint(layer.backedLayer, 0, 0, rootPane)

skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import java.awt.BorderLayout
2828
import java.awt.Color
2929
import java.awt.Dimension
3030
import java.awt.event.*
31+
import javax.swing.Box
3132
import javax.swing.JFrame
3233
import javax.swing.JLayeredPane
3334
import javax.swing.JPanel
@@ -261,6 +262,48 @@ class SkiaLayerTest {
261262
}
262263
}
263264

265+
@Test
266+
fun `move without redrawing`() = uiTest {
267+
val window = JFrame()
268+
val layer = SkiaLayer(
269+
properties = SkiaLayerProperties(renderApi = renderApi)
270+
)
271+
272+
layer.renderDelegate = object : SkikoRenderDelegate {
273+
override fun onRender(canvas: Canvas, width: Int, height: Int, nanoTime: Long) {
274+
canvas.drawRect(Rect(0f, 0f, width.toFloat(), height.toFloat()), Paint().apply {
275+
color = Color.RED.rgb
276+
})
277+
}
278+
}
279+
layer.size = Dimension(100, 100)
280+
val box = Box.createVerticalBox().apply {
281+
add(layer)
282+
}
283+
box.setBounds(0, 0, 100, 100)
284+
285+
try {
286+
val panel = JLayeredPane()
287+
panel.add(box)
288+
window.contentPane.add(panel)
289+
window.setLocation(200, 200)
290+
window.size = Dimension(200, 200)
291+
window.defaultCloseOperation = WindowConstants.DISPOSE_ON_CLOSE
292+
window.isUndecorated = true
293+
window.isVisible = true
294+
295+
delay(1000)
296+
screenshots.assert(window.bounds, "frame1")
297+
298+
box.setBounds(100, 0, 100, 100)
299+
delay(1000)
300+
screenshots.assert(window.bounds, "frame2")
301+
} finally {
302+
layer.dispose()
303+
window.close()
304+
}
305+
}
306+
264307
@Test
265308
fun `resize window`() = uiTest {
266309
val window = UiTestWindow()

skiko/src/commonMain/kotlin/org/jetbrains/skiko/redrawer/Redrawer.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ internal interface Redrawer {
44
fun dispose()
55
fun needRedraw()
66
fun redrawImmediately()
7-
fun syncSize() = Unit
7+
fun syncBounds() = Unit
88
fun setVisible(isVisible: Boolean) = Unit
99
val renderInfo: String
1010
}
511 Bytes
Loading
514 Bytes
Loading

skiko/src/macosMain/kotlin/org/jetbrains/skiko/SkiaLayer.macos.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,13 @@ actual open class SkiaLayer {
8484
private val nsViewObserver = object : NSObject() {
8585
@ObjCAction
8686
fun frameDidChange(notification: NSNotification) {
87-
redrawer?.syncSize()
87+
redrawer?.syncBounds()
8888
redrawer?.redrawImmediately()
8989
}
9090

9191
@ObjCAction
9292
fun windowDidChangeBackingProperties(notification: NSNotification) {
93-
redrawer?.syncSize()
93+
redrawer?.syncBounds()
9494
redrawer?.redrawImmediately()
9595
}
9696

@@ -126,7 +126,7 @@ actual open class SkiaLayer {
126126
nsView.postsFrameChangedNotifications = true
127127
nsViewObserver.addObserver()
128128
redrawer = createNativeRedrawer(this, renderApi).apply {
129-
syncSize()
129+
syncBounds()
130130
needRedraw()
131131
}
132132
}

skiko/src/macosMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.macos.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ internal class MacOsMetalRedrawer(
7979
/**
8080
* Synchronizes the [metalLayer] size with the size of underlying nsView
8181
*/
82-
override fun syncSize() {
82+
override fun syncBounds() {
8383
syncContentScale()
8484
val osFrame = skiaLayer.nsView.frame
8585
val (w, h) = osFrame.useContents {

0 commit comments

Comments
 (0)