Unverified Commit b53f5d4a authored by Manuel Unterhofer's avatar Manuel Unterhofer Committed by GitHub

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

…on Windows

First off, I don't think there was any need for the repeated checks for device pixel ratio changes via checkContentScale because we should be able to rely on the graphicsContextScaleTransform property from AWT.

I removed the corresponding code and changed contentScale to directly return the platform-specific implementations from the toolkit/device. There's also no need to cache the value because the device and the graphics configuration already does it for us.

Secondly, there was a layout glitch on Windows when moving windows between monitors with different device pixel ratios. A fix has to happen directly in the JBR, but for now I created a semi-cheap workaround. See this issue and the linked ones for details: https://youtrack.jetbrains.com/issue/JBR-5259/Canvas-is-painted-at-the-wrong-position-after-dragging-JFrame-to-a-monitor-with-different-scale
parent 3192de0d
......@@ -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()
......@@ -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()
......
......@@ -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)
......
......@@ -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();
......
......@@ -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() {
......@@ -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)
......@@ -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)
......
......@@ -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 {
......@@ -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)
}
......@@ -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 -> {
......@@ -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()
......@@ -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
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
......@@ -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,
......@@ -71,11 +74,15 @@ 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
//
......@@ -83,7 +90,6 @@ actual open class SkiaLayer internal constructor(
// 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
this@SkiaLayer.checkContentScale()
redrawer?.needRedraw()
}
......@@ -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)
......@@ -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)
}
......@@ -158,7 +177,7 @@ actual open class SkiaLayer internal constructor(
}
actual val contentScale: Float
get() = backedLayer.contentScale
get() = graphicsConfiguration.defaultTransform.scaleX.toFloat()
/**
* Returns the pointer to an OS specific handle (native resource) of the [SkiaLayer].
......@@ -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,
......@@ -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
}
}
}
......@@ -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()
......
......@@ -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;
......
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