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

Make device scaling reactivity more reliable and fix layout glitch #661

Merged
merged 7 commits into from
Feb 23, 2023
7 changes: 3 additions & 4 deletions samples/SkiaAwtSample/src/main/kotlin/SkiaAwtSample/App.kt
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ fun createWindow(title: String, exitOnClose: Boolean) = SwingUtilities.invokeLat
val window = JFrame(title)
window.defaultCloseOperation =
if (exitOnClose) WindowConstants.EXIT_ON_CLOSE else WindowConstants.DISPOSE_ON_CLOSE
window.title = title

skiaLayer.attachTo(window.contentPane)
window.background = Color.GREEN
window.contentPane = skiaLayer

// Create menu.
val menuBar = JMenuBar()
Expand Down Expand Up @@ -126,7 +125,7 @@ fun createWindow(title: String, exitOnClose: Boolean) = SwingUtilities.invokeLat
}
skiaLayer.transparency = true
} else {
skiaLayer.background = Color.WHITE
skiaLayer.background = Color.LIGHT_GRAY
}

// MANDATORY: set window preferred size before calling pack()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class ClocksAwt(private val layer: SkiaLayer) : SkikoView {
}
val paragraph = ParagraphBuilder(style, fontCollection)
.pushStyle(TextStyle().setColor(0xFF000000.toInt()))
.addText("Graphics API: ${layer.renderApi} ✿゚ $currentSystemTheme")
.addText("JRE: ${System.getProperty("java.vendor")}, ${System.getProperty("java.runtime.version")}, Graphics API: ${layer.renderApi} ✿゚ $currentSystemTheme")
.popStyle()
.build()
paragraph.layout(Float.POSITIVE_INFINITY)
Expand Down
6 changes: 0 additions & 6 deletions skiko/src/awtMain/cpp/linux/drawlayer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,6 @@ extern "C"
}
}

JNIEXPORT jfloat JNICALL Java_org_jetbrains_skiko_PlatformOperationsKt_linuxGetDpiScaleNative(JNIEnv *env, jobject properties, jlong platformInfoPtr)
{
JAWT_X11DrawingSurfaceInfo *dsi_x11 = fromJavaPointer<JAWT_X11DrawingSurfaceInfo *>(platformInfoPtr);
return (float) getDpiScaleByDisplay(dsi_x11->display);
}

JNIEXPORT jfloat JNICALL Java_org_jetbrains_skiko_SetupKt_linuxGetSystemDpiScale(JNIEnv *env, jobject layer)
{
return (float) getDpiScale();
Expand Down
26 changes: 0 additions & 26 deletions skiko/src/awtMain/kotlin/org/jetbrains/skiko/HardwareLayer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ internal open class HardwareLayer(
}
}

// getDpiScale is expensive operation on some platforms, so we cache it
private var _contentScale: Float? = null

fun defineContentScale() {
_contentScale = getDpiScale()
}

override fun paint(g: Graphics) {}

open fun init() {
Expand All @@ -43,22 +36,6 @@ internal open class HardwareLayer(
private external fun nativeInit(platformInfo: Long)
private external fun nativeDispose()

// TODO checkContentScale is called before init. it is ok, but when we fix getDpiScale on Linux we should check [isInit]
fun checkContentScale(): Boolean {
val contentScale = getDpiScale()
if (contentScale != _contentScale) {
_contentScale = contentScale
return true
}
return false
}

private fun getDpiScale(): Float {
val scale = platformOperations.getDpiScale(this)
check(scale > 0) { "HardwareLayer.contentScale isn't positive: $contentScale"}
return scale
}

val contentHandle: Long
get() = useDrawingSurfacePlatformInfo(::getContentHandle)

Expand All @@ -68,9 +45,6 @@ internal open class HardwareLayer(
val currentDPI: Int
get() = useDrawingSurfacePlatformInfo(::getCurrentDPI)

val contentScale: Float
get() = _contentScale!!

var fullscreen: Boolean
get() = platformOperations.isFullscreen(this)
set(value) = platformOperations.setFullscreen(this, value)
Expand Down
16 changes: 0 additions & 16 deletions skiko/src/awtMain/kotlin/org/jetbrains/skiko/PlatformOperations.kt
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ internal interface PlatformOperations {
fun setFullscreen(component: Component, value: Boolean)
fun disableTitleBar(component: Component, headerHeight: Float)
fun orderEmojiAndSymbolsPopup()
fun getDpiScale(component: Component): Float
}

internal val platformOperations: PlatformOperations by lazy {
Expand All @@ -56,10 +55,6 @@ internal val platformOperations: PlatformOperations by lazy {
osxSetFullscreenNative(component, value)
}

override fun getDpiScale(component: Component): Float {
return component.graphicsConfiguration.defaultTransform.scaleX.toFloat()
}

override fun disableTitleBar(component: Component, headerHeight: Float) {
osxDisableTitleBar(component, headerHeight)
}
Expand Down Expand Up @@ -88,10 +83,6 @@ internal val platformOperations: PlatformOperations by lazy {

override fun orderEmojiAndSymbolsPopup() {
}

override fun getDpiScale(component: Component): Float {
return component.graphicsConfiguration.defaultTransform.scaleX.toFloat()
}
}
}
OS.Linux -> {
Expand All @@ -113,10 +104,6 @@ internal val platformOperations: PlatformOperations by lazy {

override fun orderEmojiAndSymbolsPopup() {
}

override fun getDpiScale(component: Component): Float {
return component.graphicsConfiguration.defaultTransform.scaleX.toFloat()
}
}
}
OS.Android -> TODO()
Expand All @@ -131,6 +118,3 @@ external private fun osxIsFullscreenNative(component: Component): Boolean
external private fun osxSetFullscreenNative(component: Component, value: Boolean)
external private fun osxDisableTitleBar(component: Component, headerHeight: Float)
external private fun osxOrderEmojiAndSymbolsPopup()

// Linux
external private fun linuxGetDpiScaleNative(platformInfo: Long): Float
64 changes: 39 additions & 25 deletions skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package org.jetbrains.skiko

import org.jetbrains.skia.*
import org.jetbrains.skia.Canvas
import org.jetbrains.skiko.redrawer.Redrawer
import java.awt.Color
import java.awt.Component
import java.awt.Window
import java.awt.event.*
import java.awt.geom.AffineTransform
import java.awt.im.InputMethodRequests
import java.util.concurrent.CancellationException
import javax.accessibility.Accessible
Expand All @@ -15,6 +15,9 @@ import javax.swing.JPanel
import javax.swing.SwingUtilities
import javax.swing.SwingUtilities.isEventDispatchThread
import javax.swing.UIManager
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch

actual open class SkiaLayer internal constructor(
externalAccessibleFactory: ((Component) -> Accessible)? = null,
Expand Down Expand Up @@ -71,19 +74,22 @@ actual open class SkiaLayer internal constructor(
val canvas: java.awt.Canvas
get() = backedLayer

private var peerBufferSizeFixJob: Job? = null

init {
isOpaque = false
layout = null
backedLayer = object : HardwareLayer(externalAccessibleFactory) {
override fun paint(g: java.awt.Graphics) {
checkContentScale()

// 1. JPanel.paint is not always called (in rare cases).
// For example if we call 'jframe.isResizable = false` on Ubuntu
//
// 2. HardwareLayer.paint is also not always called.
// For example, on macOs when we resize window or change DPI
//
// 3. to avoid double paint in one single frame, use needRedraw instead of redrawImmediately
[email protected]()
redrawer?.needRedraw()
}

Expand All @@ -109,11 +115,26 @@ actual open class SkiaLayer internal constructor(
}
@Suppress("LeakingThis")
add(backedLayer)

backedLayer.addHierarchyListener {
if (it.changeFlags and HierarchyEvent.SHOWING_CHANGED.toLong() != 0L) {
checkShowing()
}
}

addPropertyChangeListener("graphicsContextScaleTransform") {
redrawer?.syncSize()
notifyChange(PropertyKind.ContentScale)

// Workaround for JBR-5259
if (hostOs == OS.Windows) {
peerBufferSizeFixJob?.cancel()
peerBufferSizeFixJob = GlobalScope.launch(MainUIDispatcher) {
backedLayer.setLocation(1, 0)
backedLayer.setLocation(0, 0)
}
}
}
}

private var fullscreenAdapter = FullscreenAdapter(backedLayer)
Expand All @@ -129,8 +150,6 @@ actual open class SkiaLayer internal constructor(
super.addNotify()
val window = SwingUtilities.getRoot(this) as Window
window.addComponentListener(fullscreenAdapter)
backedLayer.defineContentScale()
checkContentScale()
checkShowing()
init(isInited)
}
Expand Down Expand Up @@ -158,7 +177,7 @@ actual open class SkiaLayer internal constructor(
}

actual val contentScale: Float
get() = backedLayer.contentScale
manu-unter marked this conversation as resolved.
Show resolved Hide resolved
get() = graphicsConfiguration.defaultTransform.scaleX.toFloat()

/**
* Returns the pointer to an OS specific handle (native resource) of the [SkiaLayer].
Expand Down Expand Up @@ -336,25 +355,21 @@ actual open class SkiaLayer internal constructor(
pictureRecorder?.close()
pictureRecorder = null
backedLayer.dispose()
peerBufferSizeFixJob?.cancel()
isDisposed = true
}
}

override fun setBounds(x: Int, y: Int, width: Int, height: Int) {
var roundedWidth = width
var roundedHeight = height
if (isInited) {
roundedWidth = roundSize(width)
roundedHeight = roundSize(height)
}
super.setBounds(x, y, roundedWidth, roundedHeight)
backedLayer.setSize(roundedWidth, roundedHeight)
override fun doLayout() {
backedLayer.setBounds(0, 0, roundSize(width), roundSize(height))
backedLayer.validate()
redrawer?.syncSize()
}


override fun paint(g: java.awt.Graphics) {
super.paint(g)
checkContentScale()

// `paint` can be called when we already inside `draw` method.
//
// For example if we call some AWT function inside renderer.onRender,
Expand All @@ -368,14 +383,15 @@ actual open class SkiaLayer internal constructor(
}
}

/*
In AWT there is no a change DPI event; so we should call this function when we expect that DPI maybe changed
We hope that call it on AWT/SWING `paint` and our update is enough
*/
private fun checkContentScale() {
if (backedLayer.checkContentScale()) {
notifyChange(PropertyKind.ContentScale)
redrawer?.syncSize()
private var latestCheckedDefaultTransform: AffineTransform? = null

// Workaround for JBR-5274 and JBR-5305
fun checkContentScale() {
graphicsConfiguration.defaultTransform.let {
if (it != latestCheckedDefaultTransform) {
firePropertyChange("graphicsContextScaleTransform", latestCheckedDefaultTransform, it)
latestCheckedDefaultTransform = it
}
}
}

Expand Down Expand Up @@ -504,8 +520,6 @@ actual open class SkiaLayer internal constructor(
check(isEventDispatchThread()) { "Method should be called from AWT event dispatch thread" }
check(!isDisposed) { "SkiaLayer is disposed" }

checkContentScale()

FrameWatcher.nextFrame()
fpsCounter?.tick()

Expand Down
6 changes: 0 additions & 6 deletions skiko/src/jvmMain/cpp/common/stubs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ void skikoUnimplemented(const char* message) {
// we put here stubs for all OS specific native methods.

#ifndef SK_BUILD_FOR_LINUX
JNIEXPORT jfloat JNICALL Java_org_jetbrains_skiko_PlatformOperationsKt_linuxGetDpiScaleNative(
JNIEnv *env, jobject properties, jlong platformInfoPtr) {
skikoUnimplemented("Java_org_jetbrains_skiko_PlatformOperationsKt_linuxGetDpiScaleNative");
return 0;
}

JNIEXPORT jfloat JNICALL Java_org_jetbrains_skiko_SetupKt_linuxGetSystemDpiScale(JNIEnv *env, jobject layer) {
skikoUnimplemented("Java_org_jetbrains_skiko_SetupKt_linuxGetSystemDpiScale");
return 0;
Expand Down