Merge pull request #65 from JetBrains/macos_fix_freeze

Fix macOs freeze when we render very fast on resize. Disable drawing without vsync
JetBrains · Feb 17, 2021 · b123a2f · b123a2f
2 parents b6e0411 + 529dc3f
commit b123a2f
Show file tree

Hide file tree

Showing 8 changed files with 120 additions and 31 deletions.
diff --git a/samples/SkijaInjectSample/src/test/kotlin/org/jetbrains/skiko/SkiaWindowTest.kt b/samples/SkijaInjectSample/src/test/kotlin/org/jetbrains/skiko/SkiaWindowTest.kt
@@ -1,5 +1,6 @@
 package org.jetbrains.skiko
 
+import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.delay
@@ -204,6 +205,44 @@ class SkiaWindowTest {
         delay(5000)
     }
 
+    @Test(timeout = 20000)
+    fun `render continuously empty content without vsync`() = swingTest {
+        assumeTrue(hostOs != OS.MacOS) // TODO remove when we will support drawing without vsync on macOs
+
+        val targetDrawCount = 500
+        var drawCount = 0
+        val onDrawCompleted = CompletableDeferred<Unit>()
+
+        val window = SkiaWindow(
+            properties = SkiaLayerProperties(
+                isVsyncEnabled = false
+            )
+        )
+
+        try {
+            window.setLocation(200, 200)
+            window.setSize(400, 200)
+            window.defaultCloseOperation = WindowConstants.DISPOSE_ON_CLOSE
+            window.layer.renderer = object : SkiaRenderer {
+                override fun onRender(canvas: Canvas, width: Int, height: Int, nanoTime: Long) {
+                    drawCount++
+
+                    if (drawCount < targetDrawCount) {
+                        window.layer.needRedraw()
+                    } else {
+                        onDrawCompleted.complete(Unit)
+                    }
+                }
+            }
+            window.isUndecorated = true
+            window.isVisible = true
+
+            onDrawCompleted.await()
+        } finally {
+            window.close()
+        }
+    }
+
     @Test
     fun `render text (Windows)`() {
         testRenderText(OS.Windows)

diff --git a/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/PlatformOperations.kt b/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/PlatformOperations.kt
@@ -14,7 +14,7 @@ internal interface PlatformOperations {
     fun isFullscreen(component: Component): Boolean
     fun setFullscreen(component: Component, value: Boolean)
     fun getDpiScale(component: Component): Float
-    fun createRedrawer(layer: HardwareLayer): Redrawer
+    fun createRedrawer(layer: HardwareLayer, properties: SkiaLayerProperties): Redrawer
 }
 
 internal val platformOperations: PlatformOperations by lazy {
@@ -32,9 +32,9 @@ internal val platformOperations: PlatformOperations by lazy {
                     return component.graphicsConfiguration.defaultTransform.scaleX.toFloat()
                 }
 
-                override fun createRedrawer(layer: HardwareLayer) = when(renderApi) {
+                override fun createRedrawer(layer: HardwareLayer, properties: SkiaLayerProperties) = when(renderApi) {
                     GraphicsApi.SOFTWARE -> RasterRedrawer(layer)
-                    else -> MacOsOpenGLRedrawer(layer)
+                    else -> MacOsOpenGLRedrawer(layer, properties)
                 }
         }
         OS.Windows -> {
@@ -55,9 +55,9 @@ internal val platformOperations: PlatformOperations by lazy {
                     return component.graphicsConfiguration.defaultTransform.scaleX.toFloat()
                 }
 
-                override fun createRedrawer(layer: HardwareLayer) = when(renderApi) {
+                override fun createRedrawer(layer: HardwareLayer, properties: SkiaLayerProperties) = when(renderApi) {
                     GraphicsApi.SOFTWARE -> RasterRedrawer(layer)
-                    else -> WindowsOpenGLRedrawer(layer)
+                    else -> WindowsOpenGLRedrawer(layer, properties)
                 }
             }
         }
@@ -91,9 +91,9 @@ internal val platformOperations: PlatformOperations by lazy {
 //                    return component.useDrawingSurfacePlatformInfo(::linuxGetDpiScaleNative)
                 }
 
-                override fun createRedrawer(layer: HardwareLayer) = when(renderApi) {
+                override fun createRedrawer(layer: HardwareLayer, properties: SkiaLayerProperties) = when(renderApi) {
                     GraphicsApi.SOFTWARE -> RasterRedrawer(layer)
-                    else -> LinuxOpenGLRedrawer(layer)
+                    else -> LinuxOpenGLRedrawer(layer, properties)
                 }
             }
         }

diff --git a/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/SkiaLayer.kt b/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/SkiaLayer.kt
@@ -18,7 +18,9 @@ interface SkiaRenderer {
 
 private class PictureHolder(val instance: Picture, val width: Int, val height: Int)
 
-open class SkiaLayer : HardwareLayer() {
+open class SkiaLayer(
+    private val properties: SkiaLayerProperties = SkiaLayerProperties()
+) : HardwareLayer() {
     var renderer: SkiaRenderer? = null
     val clipComponents = mutableListOf<ClipRectangle>()
 
@@ -35,7 +37,7 @@ open class SkiaLayer : HardwareLayer() {
 
     override fun init() {
         super.init()
-        redrawer = platformOperations.createRedrawer(this)
+        redrawer = platformOperations.createRedrawer(this, properties)
         redrawer?.syncSize()
         redrawer?.redrawImmediately()
     }

diff --git a/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/SkiaLayerProperties.kt b/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/SkiaLayerProperties.kt
@@ -0,0 +1,5 @@
+package org.jetbrains.skiko
+
+class SkiaLayerProperties(
+    val isVsyncEnabled: Boolean = SkikoProperties.vsyncEnabled
+)
diff --git a/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/SkiaWindow.kt b/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/SkiaWindow.kt
@@ -2,8 +2,10 @@ package org.jetbrains.skiko
 
 import javax.swing.JFrame
 
-open class SkiaWindow : JFrame() {
-    val layer = SkiaLayer()
+open class SkiaWindow(
+    properties: SkiaLayerProperties = SkiaLayerProperties()
+) : JFrame() {
+    val layer = SkiaLayer(properties)
 
     init {
         contentPane.add(layer)

diff --git a/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/redrawer/LinuxOpenGLRedrawer.kt b/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/redrawer/LinuxOpenGLRedrawer.kt
@@ -7,11 +7,12 @@ import org.jetbrains.skiko.DrawingSurface
 import org.jetbrains.skiko.FrameDispatcher
 import org.jetbrains.skiko.HardwareLayer
 import org.jetbrains.skiko.OpenGLApi
-import org.jetbrains.skiko.SkikoProperties
+import org.jetbrains.skiko.SkiaLayerProperties
 import org.jetbrains.skiko.getDrawingSurface
 
 internal class LinuxOpenGLRedrawer(
-    private val layer: HardwareLayer
+    private val layer: HardwareLayer,
+    private val properties: SkiaLayerProperties
 ) : Redrawer {
     private val context = layer.lockDrawingSurface {
         it.createContext()
@@ -70,6 +71,8 @@ internal class LinuxOpenGLRedrawer(
                 }
             }
 
+            val isVsyncEnabled = toRedrawAlive.all { it.properties.isVsyncEnabled }
+
             val drawingSurfaces = toRedrawAlive.map { lockDrawingSurface(it.layer) }.toList()
             try {
                 toRedrawAlive.forEachIndexed { index, redrawer ->
@@ -79,7 +82,7 @@ internal class LinuxOpenGLRedrawer(
 
                 toRedrawAlive.forEachIndexed { index, _ ->
                     // it is ok to set swap interval every frame, there is no performance overhead
-                    drawingSurfaces[index].setSwapInterval(if (SkikoProperties.vsyncEnabled) 1 else 0)
+                    drawingSurfaces[index].setSwapInterval(if (isVsyncEnabled) 1 else 0)
                     drawingSurfaces[index].swapBuffers()
                 }
 

diff --git a/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/redrawer/MacOsOpenGLRedrawer.kt b/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/redrawer/MacOsOpenGLRedrawer.kt
@@ -1,18 +1,28 @@
 package org.jetbrains.skiko.redrawer
 
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.swing.Swing
 import org.jetbrains.skiko.FrameDispatcher
 import org.jetbrains.skiko.HardwareLayer
 import org.jetbrains.skiko.OpenGLApi
-import org.jetbrains.skiko.SkikoProperties
-import org.jetbrains.skiko.useDrawingSurfacePlatformInfo
+import org.jetbrains.skiko.SkiaLayerProperties
 import org.jetbrains.skiko.Task
+import org.jetbrains.skiko.useDrawingSurfacePlatformInfo
 import javax.swing.SwingUtilities.convertPoint
 import javax.swing.SwingUtilities.getRootPane
+import kotlin.system.measureNanoTime
+
+// Current implementation is fragile (it works in all tested cases, but we can't test everything)
+//
+// We should investigate can we implement our own CAOpenGLLayer, without its restrictions.
+// (see, for example https://github.com/gnustep/libs-gui/blob/master/Source/NSOpenGLView.m)
+//
+// P.S. MacOsOpenGLRedrawer will not be used by default in the future, because we will support Metal.
 
 internal class MacOsOpenGLRedrawer(
-    private val layer: HardwareLayer
+    private val layer: HardwareLayer,
+    private val properties: SkiaLayerProperties
 ) : Redrawer {
     private val containerLayerPtr = layer.useDrawingSurfacePlatformInfo(::initContainer)
     private val drawLock = Any()
@@ -24,6 +34,8 @@ internal class MacOsOpenGLRedrawer(
                 layer.draw()
             }
         }
+
+        suspend fun display() = display(::setNeedsDisplay)
     }
 
     // use a separate layer for vsync, because with single layer we cannot asynchronously update layer
@@ -52,7 +64,7 @@ internal class MacOsOpenGLRedrawer(
             return canDraw
         }
 
-        override fun setNeedsDisplay() {
+        private fun requestAsyncDisplay() {
             // Use asynchronous mode instead of just setNeedsDisplay,
             // so Core Animation will wait for the next frame in vsync signal
             //
@@ -63,13 +75,13 @@ internal class MacOsOpenGLRedrawer(
             // https://chromium.googlesource.com/chromium/chromium/+/0489078bf98350b00876070cf2fdce230905f47e/content/browser/renderer_host/compositing_iosurface_layer_mac.mm#57
             if (!isAsynchronous) {
                 isAsynchronous = true
-                super.setNeedsDisplay()
+                setNeedsDisplay()
             }
         }
 
         suspend fun sync() {
             canDraw = true
-            display()
+            display(::requestAsyncDisplay)
             canDraw = false
         }
     }
@@ -78,13 +90,11 @@ internal class MacOsOpenGLRedrawer(
         synchronized(drawLock) {
             layer.update(System.nanoTime())
         }
-        if (SkikoProperties.vsyncEnabled) {
+        if (properties.isVsyncEnabled) {
             drawLayer.setNeedsDisplay()
             vsyncLayer.sync()
         } else {
-            // If vsync is disabled we should await the drawing to end.
-            // Otherwise we will call 'update' multiple times.
-            drawLayer.display()
+            error("Drawing without vsync isn't supported on macOs with OpenGL")
         }
     }
 
@@ -113,7 +123,16 @@ internal class MacOsOpenGLRedrawer(
 
     override fun redrawImmediately() {
         layer.update(System.nanoTime())
-        drawLayer.setNeedsDisplay()
+
+        // macOs will call 'draw' itself because of 'setNeedsDisplayOnBoundsChange=true'.
+        // But we schedule new frame after vsync anyway.
+        // Because 'redrawImmediately' can be called after 'draw',
+        // and we need at least one 'draw' after 'redrawImmediately'.
+        //
+        // We don't use setNeedsDisplay, because frequent calls of it are unreliable.
+        // 'setNeedsDisplayOnBoundsChange=true' with combination of 'scheduleFrame' is enough
+        // to not see the white bars on resize.
+        frameDispatcher.scheduleFrame()
     }
 }
 
@@ -134,15 +153,32 @@ private abstract class AWTGLLayer(private val containerPtr: Long, setNeedsDispla
         get() = isAsynchronous(ptr)
         set(value) = setAsynchronous(ptr, value)
 
-    open fun setNeedsDisplay() = setNeedsDisplayOnMainThread(ptr)
+    /**
+     * Schedule next [draw] as soon as possible (not waiting for vsync)
+     *
+     * WARNING!!!
+     *
+     * CAOpenGLLayer will not call [draw] if we call [setNeedsDisplay] too often.
+     *
+     * Experimentally we found out that after 15 draw's between two vsync's (900 FPS on 60 Hz display) will cause
+     * setNeedsDisplay to not schedule the next draw at all.
+     *
+     * Only after the next vsync, [setNeedsDisplay] will be working again.
+     */
+    fun setNeedsDisplay() {
+        setNeedsDisplayOnMainThread(ptr)
+    }
 
-    suspend fun display() = display.runAndAwait {
-        setNeedsDisplay()
+    protected suspend fun display(
+        startDisplay: () -> Unit
+    ) = display.runAndAwait {
+        startDisplay()
     }
 
     // Called in AppKit Thread
     protected open fun canDraw() = true
 
+    // Called in AppKit Thread
     @Suppress("unused") // called from native code
     private fun performDraw() {
         try {

diff --git a/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/redrawer/WindowsOpenGLRedrawer.kt b/skiko/src/jvmMain/kotlin/org/jetbrains/skiko/redrawer/WindowsOpenGLRedrawer.kt
@@ -7,11 +7,12 @@ import kotlinx.coroutines.withContext
 import org.jetbrains.skiko.FrameDispatcher
 import org.jetbrains.skiko.HardwareLayer
 import org.jetbrains.skiko.OpenGLApi
-import org.jetbrains.skiko.SkikoProperties
+import org.jetbrains.skiko.SkiaLayerProperties
 import org.jetbrains.skiko.useDrawingSurfacePlatformInfo
 
 internal class WindowsOpenGLRedrawer(
-    private val layer: HardwareLayer
+    private val layer: HardwareLayer,
+    private val properties: SkiaLayerProperties
 ) : Redrawer {
     private val device = layer.useDrawingSurfacePlatformInfo(::getDevice)
     private val context = createContext(device)
@@ -92,7 +93,8 @@ internal class WindowsOpenGLRedrawer(
                 OpenGLApi.instance.glFinish()
             }
 
-            if (SkikoProperties.vsyncEnabled) {
+            val isVsyncEnabled = toRedrawAlive.all { it.properties.isVsyncEnabled }
+            if (isVsyncEnabled) {
                 withContext(Dispatchers.IO) {
                     dwmFlush() // wait for vsync
                 }