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

Avoid calling SkiaLayer.revalidate() when the window itself is moved. #1010

Merged
merged 1 commit into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.jetbrains.skiko.redrawer.Redrawer
import org.jetbrains.skiko.redrawer.RedrawerManager
import java.awt.Color
import java.awt.Component
import java.awt.Point
import java.awt.event.*
import java.awt.geom.AffineTransform
import java.awt.im.InputMethodRequests
Expand All @@ -20,7 +21,6 @@ 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(
Expand Down Expand Up @@ -138,12 +138,34 @@ actual open class SkiaLayer internal constructor(
add(backedLayer)

addAncestorListener(object : AncestorListener {
override fun ancestorAdded(event: AncestorEvent?) = Unit

override fun ancestorRemoved(event: AncestorEvent?) = Unit
private var positionInWindow: Point? = null

private val zeroPoint = Point(0, 0)

private fun computePositionInWindow(): Point? {
val window = SwingUtilities.getWindowAncestor(this@SkiaLayer)
return if (window == null) {
null
} else {
SwingUtilities.convertPoint(this@SkiaLayer, zeroPoint, window)
}
}

override fun ancestorAdded(event: AncestorEvent?) {
positionInWindow = computePositionInWindow()
}

override fun ancestorRemoved(event: AncestorEvent?) {
positionInWindow = null
}

override fun ancestorMoved(event: AncestorEvent?) {
revalidate()
val newPosition = computePositionInWindow()
if ((positionInWindow != null) && (positionInWindow != newPosition)) {
revalidate()
}
positionInWindow = newPosition
}
})

Expand Down Expand Up @@ -190,7 +212,6 @@ actual open class SkiaLayer internal constructor(
init(isInited)
}


actual fun detach() {
dispose()
}
Expand Down
42 changes: 34 additions & 8 deletions skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package org.jetbrains.skiko

import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.yield
import kotlinx.coroutines.*
import org.jetbrains.skia.*
import org.jetbrains.skia.Canvas
import org.jetbrains.skia.FontMgr
import org.jetbrains.skia.Paint
import org.jetbrains.skia.Rect
import org.jetbrains.skia.paragraph.FontCollection
import org.jetbrains.skia.paragraph.ParagraphBuilder
import org.jetbrains.skia.paragraph.ParagraphStyle
Expand All @@ -24,9 +20,8 @@ import org.junit.Assume.assumeTrue
import org.junit.Ignore
import org.junit.Rule
import org.junit.Test
import java.awt.BorderLayout
import java.awt.*
import java.awt.Color
import java.awt.Dimension
import java.awt.event.*
import javax.swing.Box
import javax.swing.JFrame
Expand Down Expand Up @@ -941,6 +936,37 @@ class SkiaLayerTest {
}
}

@Test
fun `content not relaid out on window move`() = uiTest {
var layoutCount = 0

val window = UiTestWindow {
contentPane.layout = object: BorderLayout() {
override fun layoutContainer(parent: Container?) {
super.layoutContainer(parent)
layoutCount++
}
}
contentPane.add(layer)
}
window.size = Dimension(400, 400)
window.isVisible = true

repeat(20) {
window.location = window.location.let {
java.awt.Point(it.x + 10, it.y + 10)
}
delay(50)
}

// Ideally, layoutCount would be just 1, but Swing appears to call layout one extra time, so it ends up being 2.
// Compare to 3 just to avoid a false-failure if there's another layout for whatever reason.
// What we're interested to validate is that there's no layout occurring on every window move.
assert(layoutCount <= 3) {
"Layout count: $layoutCount"
}
}

private class RectRenderer(
private val layer: SkiaLayer,
var rectWidth: Int,
Expand Down
Loading