Fix race condition reading layer properties in a background thread

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/AngleContextHandler.kt

@@ -23,12 +23,11 @@ internal class AngleContextHandler(layer: SkiaLayer) : ContextBasedContextHandle
         return false
     }
 
-    override fun initCanvas() {
+    override fun DrawScope.initCanvas() {
         val context = context ?: return
-        val scale = layer.contentScale
 
-        val w = (layer.width * scale).toInt().coerceAtLeast(0)
-        val h = (layer.height * scale).toInt().coerceAtLeast(0)
+        val w = scaledLayerWidth
+        val h = scaledLayerHeight
 
         if (isSizeChanged(w, h) || surface == null) {
             disposeCanvas()

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/ContextBasedContextHandler.kt

@@ -6,7 +6,7 @@ import org.jetbrains.skiko.SkiaLayer
 
 internal abstract class ContextBasedContextHandler(layer: SkiaLayer, val name: String) : JvmContextHandler(layer) {
 
-    abstract protected fun makeContext(): DirectContext
+    protected abstract fun makeContext(): DirectContext
 
     override fun initContext(): Boolean {
         try {

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/ContextFreeContextHandler.kt

@@ -3,13 +3,13 @@ package org.jetbrains.skiko.context
 import org.jetbrains.skiko.SkiaLayer
 
 internal abstract class ContextFreeContextHandler(layer: SkiaLayer) : JvmContextHandler(layer) {
-    private var isInited = false
+    private var isInitialized = false
 
     override fun initContext(): Boolean {
-        if (!isInited) {
-            isInited = true
+        if (!isInitialized) {
+            isInitialized = true
             onContextInitialized()
         }
-        return isInited
+        return isInitialized
     }
 }

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/Direct3DContextHandler.kt

@@ -30,15 +30,14 @@ internal class Direct3DContextHandler(layer: SkiaLayer) : ContextBasedContextHan
         return false
     }
 
-    override fun initCanvas() {
+    override fun DrawScope.initCanvas() {
         val context = context ?: return
-        val scale = layer.contentScale
 
         // Direct3D can't work with zero size.
         // Don't rewrite code to skipping, as we need the whole pipeline in zero case too
         // (drawing -> flushing -> swapping -> waiting for vsync)
-        val width = (layer.width * scale).toInt().coerceAtLeast(1)
-        val height = (layer.height * scale).toInt().coerceAtLeast(1)
+        val width = scaledLayerWidth.coerceAtLeast(1)
+        val height = scaledLayerHeight.coerceAtLeast(1)
 
         if (isSizeChanged(width, height) || isSurfacesNull()) {
             disposeCanvas()

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/DirectSoftwareContextHandler.kt

@@ -20,10 +20,9 @@ internal class DirectSoftwareContextHandler(layer: SkiaLayer) : ContextFreeConte
         return false
     }
 
-    override fun initCanvas() {
-        val scale = layer.contentScale
-        val w = (layer.width * scale).toInt().coerceAtLeast(0)
-        val h = (layer.height * scale).toInt().coerceAtLeast(0)
+    override fun DrawScope.initCanvas() {
+        val w = scaledLayerWidth
+        val h = scaledLayerHeight
         if (isSizeChanged(w, h) || surface == null) {
             disposeCanvas()
             if (w > 0 && h > 0) {

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/JvmContextHandler.kt

@@ -4,6 +4,13 @@ import org.jetbrains.skiko.Logger
 import org.jetbrains.skiko.SkiaLayer
 
 internal abstract class JvmContextHandler(layer: SkiaLayer) : ContextHandler(layer, layer::draw) {
+
+    override fun createDrawScope() = DrawScope(
+        layerWidth = layer.width,
+        layerHeight = layer.height,
+        scale = layer.contentScale
+    )
+
     protected fun onContextInitialized() {
         if (System.getProperty("skiko.hardwareInfo.enabled") == "true") {
             Logger.info { "Renderer info:\n ${rendererInfo()}" }

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/MetalContextHandler.kt

@@ -20,12 +20,11 @@ internal class MetalContextHandler(
     private val device: MetalDevice,
     private val adapter: MetalAdapter
 ) : ContextBasedContextHandler(layer, "Metal") {
-    override fun initCanvas() {
+    override fun DrawScope.initCanvas() {
         disposeCanvas()
 
-        val scale = layer.contentScale
-        val width = (layer.backedLayer.width * scale).toInt().coerceAtLeast(0)
-        val height = (layer.backedLayer.height * scale).toInt().coerceAtLeast(0)
+        val width = scaledLayerWidth
+        val height = scaledLayerHeight
 
         if (width > 0 && height > 0) {
             renderTarget = makeRenderTarget(width, height)

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/OpenGLContextHandler.kt

@@ -18,10 +18,9 @@ internal class OpenGLContextHandler(layer: SkiaLayer) : ContextBasedContextHandl
         return false
     }
 
-    override fun initCanvas() {
-        val scale = layer.contentScale
-        val w = (layer.width * scale).toInt().coerceAtLeast(0)
-        val h = (layer.height * scale).toInt().coerceAtLeast(0)
+    override fun DrawScope.initCanvas() {
+        val w = scaledLayerWidth
+        val h = scaledLayerHeight
 
         if (isSizeChanged(w, h) || surface == null) {
             disposeCanvas()

skiko/src/awtMain/kotlin/org/jetbrains/skiko/context/SoftwareContextHandler.kt

@@ -22,12 +22,11 @@ internal class SoftwareContextHandler(layer: SkiaLayer) : ContextFreeContextHand
     var imageData: ByteArray? = null
     var raster: WritableRaster? = null
 
-    override fun initCanvas() {
+    override fun DrawScope.initCanvas() {
         disposeCanvas()
 
-        val scale = layer.contentScale
-        val w = (layer.width * scale).toInt().coerceAtLeast(0)
-        val h = (layer.height * scale).toInt().coerceAtLeast(0)
+        val w = scaledLayerWidth
+        val h = scaledLayerHeight
 
         if (storage.width != w || storage.height != h) {
             storage.allocPixelsFlags(ImageInfo.makeS32(w, h, ColorAlphaType.PREMUL), false)

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

@@ -7,6 +7,7 @@ import org.jetbrains.skia.SurfaceProps
 import org.jetbrains.skia.impl.InteropPointer
 import org.jetbrains.skia.impl.interopScope
 import org.jetbrains.skiko.*
+import org.jetbrains.skiko.context.ContextHandler
 import org.jetbrains.skiko.context.Direct3DContextHandler
 
 internal class Direct3DRedrawer(
@@ -73,24 +74,28 @@ internal class Direct3DRedrawer(
         update()
         inDrawScope {
             if (!isDisposed) { // Redrawer may be disposed in user code, during `update`
-                drawAndSwap(withVsync = SkikoProperties.windowsWaitForVsyncOnRedrawImmediately)
+                contextHandler.inDrawScope {
+                    drawAndSwap(withVsync = SkikoProperties.windowsWaitForVsyncOnRedrawImmediately)
+                }
             }
         }
     }
 
     private suspend fun draw() {
         inDrawScope {
-            withContext(dispatcherToBlockOn) {
-                drawAndSwap(withVsync = properties.isVsyncEnabled)
+            contextHandler.inDrawScope {
+                withContext(dispatcherToBlockOn) {
+                    drawAndSwap(withVsync = properties.isVsyncEnabled)
+                }
             }
         }
     }
 
-    private fun drawAndSwap(withVsync: Boolean) = synchronized(drawLock) {
+    private fun ContextHandler.DrawScope.drawAndSwap(withVsync: Boolean) = synchronized(drawLock) {
         if (isDisposed) {
             return
         }
-        contextHandler.draw()
+        contextHandlerDraw()
         swap(withVsync)
     }
 

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

@@ -3,6 +3,7 @@ package org.jetbrains.skiko.redrawer
 import kotlinx.coroutines.*
 import kotlinx.coroutines.channels.Channel
 import org.jetbrains.skiko.*
+import org.jetbrains.skiko.context.ContextHandler
 import org.jetbrains.skiko.context.MetalContextHandler
 import java.util.concurrent.atomic.AtomicBoolean
 import javax.swing.SwingUtilities.*
@@ -104,7 +105,9 @@ internal class MetalRedrawer(
         update()
         inDrawScope {
             if (!isDisposed) { // Redrawer may be disposed in user code, during `update`
-                performDraw()
+                contextHandler.inDrawScope {
+                    performDraw()
+                }
                 // Trying to draw immediately in Metal will result in lost (undrawn)
                 // frames if there are more than two between consecutive vsync events.
                 if (SkikoProperties.macOSWaitForPreviousFrameVsyncOnRedrawImmediately) {
@@ -118,11 +121,13 @@ internal class MetalRedrawer(
 
     private suspend fun draw() {
         inDrawScope {
-            // Move drawing to another thread to free the main thread
-            // It can be expensive to run it in the main thread and FPS can become unstable.
-            // This is visible by running [SkiaLayerPerformanceTest], standard deviation is increased significantly.
-            withContext(dispatcherToBlockOn) {
-                performDraw()
+            contextHandler.inDrawScope {
+                // Move drawing to another thread to free the main thread
+                // It can be expensive to run it in the main thread and FPS can become unstable.
+                // This is visible by running [SkiaLayerPerformanceTest], standard deviation is increased significantly.
+                withContext(dispatcherToBlockOn) {
+                    performDraw()
+                }
             }
         }
         if (isDisposed) throw CancellationException()
@@ -144,10 +149,10 @@ internal class MetalRedrawer(
         windowOcclusionStateChannel.trySend(isOccluded)
     }
 
-    private fun performDraw() = synchronized(drawLock) {
+    private fun ContextHandler.DrawScope.performDraw() = synchronized(drawLock) {
         if (!isDisposed) {
             autoreleasepool {
-                contextHandler.draw()
+                contextHandlerDraw()
             }
         }
     }

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

@@ -578,7 +578,7 @@ class SkiaLayerTest {
             object : BaseTestRedrawer(layer) {
                 private val contextHandler = object : JvmContextHandler(layer) {
                     override fun initContext() = false
-                    override fun initCanvas() = Unit
+                    override fun DrawScope.initCanvas() = Unit
                 }
                 override fun renderImmediately() = layer.inDrawScope(contextHandler::draw)
             }

skiko/src/awtTest/kotlin/org/jetbrains/skiko/util/UiTest.kt

@@ -15,7 +15,7 @@ internal fun uiTest(
     block: suspend UiTestScope.() -> Unit
 ) {
     assumeFalse(GraphicsEnvironment.isHeadless())
-    assumeTrue(System.getProperty("skiko.test.ui.enabled", "false") == "true")
+//    assumeTrue(System.getProperty("skiko.test.ui.enabled", "false") == "true")
 
     val renderApiProperty = System.getProperty("skiko.test.ui.renderApi", "all")
 

skiko/src/commonMain/kotlin/org/jetbrains/skiko/context/ContextHandler.kt

@@ -13,7 +13,7 @@ internal abstract class ContextHandler(
     protected var canvas: Canvas? = null
 
     protected abstract fun initContext(): Boolean
-    protected abstract fun initCanvas()
+    protected abstract fun DrawScope.initCanvas()
 
     protected open fun flush() {
         context?.flush()
@@ -34,8 +34,30 @@ internal abstract class ContextHandler(
                 "OS: ${hostOs.id} ${hostArch.id}\n"
     }
 
-    // throws RenderException if initialization of graphic context was not successful
+    /**
+     * This function will be called only in a thread where it is valid to access layer properties.
+     */
+    protected abstract fun createDrawScope(): DrawScope
+
+    /**
+     * Reads layer properties, creating a [DrawScope] in which [DrawScope.contextHandlerDraw] can later be called on a
+     * background thread.
+     *
+     * This function should be called only in a thread where it is valid to access layer properties.
+     */
+    inline fun inDrawScope(block: DrawScope.() -> Unit) {
+        createDrawScope().block()
+    }
+
+    /**
+     * This function should be called only in a thread where it is valid to access layer properties.
+     */
     fun draw() {
+        createDrawScope().contextHandlerDraw()
+    }
+
+    // throws RenderException if initialization of graphic context was not successful
+    private fun DrawScope.drawImpl() {
         if (!initContext()) {
             throw RenderException("Cannot init graphic context")
         }
@@ -46,4 +68,22 @@ internal abstract class ContextHandler(
         }
         flush()
     }
-}
+
+    inner class DrawScope(
+        val scaledLayerWidth: Int,
+        val scaledLayerHeight: Int
+    ) {
+        constructor(layerWidth: Int, layerHeight: Int, scale: Float): this(
+            scaledLayerWidth = (layerWidth * scale).toInt().coerceAtLeast(0),
+            scaledLayerHeight = (layerHeight * scale).toInt().coerceAtLeast(0)
+        )
+        constructor(layerWidth: Double, layerHeight: Double, scale: Float): this(
+            scaledLayerWidth = (layerWidth * scale).toInt().coerceAtLeast(0),
+            scaledLayerHeight = (layerHeight * scale).toInt().coerceAtLeast(0)
+        )
+
+        fun contextHandlerDraw() {
+            drawImpl()
+        }
+    }
+}

skiko/src/macosMain/kotlin/org/jetbrains/skiko/context/MetalContextHandler.macos.kt

@@ -7,7 +7,7 @@ import org.jetbrains.skiko.SkiaLayer
 import org.jetbrains.skiko.redrawer.MacOsMetalRedrawer
 
 /**
- * Metal ContextHandler implementation for MacOs.
+ * Metal ContextHandler implementation for macOS.
  */
 internal class MacOsMetalContextHandler(layer: SkiaLayer) : ContextHandler(layer, layer::draw) {
     private val metalRedrawer: MacOsMetalRedrawer
@@ -25,12 +25,17 @@ internal class MacOsMetalContextHandler(layer: SkiaLayer) : ContextHandler(layer
         return true
     }
 
-    override fun initCanvas() {
+    override fun createDrawScope() = DrawScope(
+        layerWidth = layer.nsView.frame.useContents { size.width },
+        layerHeight = layer.nsView.frame.useContents { size.height },
+        scale = layer.contentScale
+    )
+
+    override fun DrawScope.initCanvas() {
         disposeCanvas()
 
-        val scale = layer.contentScale
-        val w = (layer.nsView.frame.useContents { size.width } * scale).toInt().coerceAtLeast(0)
-        val h = (layer.nsView.frame.useContents { size.height } * scale).toInt().coerceAtLeast(0)
+        val w = scaledLayerWidth
+        val h = scaledLayerHeight
 
         if (w > 0 && h > 0) {
             renderTarget = metalRedrawer.makeRenderTarget(w, h)